Auto merge of #116089 - estebank:issue-115992-2, r=compiler-errors

When suggesting `self.x` for `S { x }`, use `S { x: self.x }`

Fix #115992.

r? `@compiler-errors`

Follow up to #116086.

2 files changed, 88 insertions, 22 deletions

diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index c3c43346ed8..15ec727e4c9 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -4140,6 +4140,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         });
     }
 
+    fn resolve_expr_field(&mut self, f: &'ast ExprField, e: &'ast Expr) {
+        self.resolve_expr(&f.expr, Some(e));
+        self.visit_ident(f.ident);
+        walk_list!(self, visit_attribute, f.attrs.iter());
+    }
+
     fn resolve_expr(&mut self, expr: &'ast Expr, parent: Option<&'ast Expr>) {
         // First, record candidate traits for this expression if it could
         // result in the invocation of a method call.
@@ -4155,7 +4161,19 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
 
             ExprKind::Struct(ref se) => {
                 self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct);
-                visit::walk_expr(self, expr);
+                // This is the same as `visit::walk_expr(self, expr);`, but we want to pass the
+                // parent in for accurate suggestions when encountering `Foo { bar }` that should
+                // have been `Foo { bar: self.bar }`.
+                if let Some(qself) = &se.qself {
+                    self.visit_ty(&qself.ty);
+                }
+                self.visit_path(&se.path, expr.id);
+                walk_list!(self, resolve_expr_field, &se.fields, expr);
+                match &se.rest {
+                    StructRest::Base(expr) => self.visit_expr(expr),
+                    StructRest::Rest(_span) => {}
+                    StructRest::None => {}
+                }
             }
 
             ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => {
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
index adddc419daf..bc5f8a37b0f 100644
--- a/compiler/rustc_resolve/src/late/diagnostics.rs
+++ b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -41,7 +41,7 @@ type Res = def::Res<ast::NodeId>;
 
 /// A field or associated item from self type suggested in case of resolution failure.
 enum AssocSuggestion {
-    Field,
+    Field(Span),
     MethodWithSelf { called: bool },
     AssocFn { called: bool },
     AssocType,
@@ -51,7 +51,7 @@ enum AssocSuggestion {
 impl AssocSuggestion {
     fn action(&self) -> &'static str {
         match self {
-            AssocSuggestion::Field => "use the available field",
+            AssocSuggestion::Field(_) => "use the available field",
             AssocSuggestion::MethodWithSelf { called: true } => {
                 "call the method with the fully-qualified path"
             }
@@ -215,7 +215,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
             }
         } else {
             let mut span_label = None;
-            let item_span = path.last().unwrap().ident.span;
+            let item_ident = path.last().unwrap().ident;
+            let item_span = item_ident.span;
             let (mod_prefix, mod_str, module, suggestion) = if path.len() == 1 {
                 debug!(?self.diagnostic_metadata.current_impl_items);
                 debug!(?self.diagnostic_metadata.current_function);
@@ -231,9 +232,35 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                     })
                 {
                     let sp = item_span.shrink_to_lo();
+
+                    // Account for `Foo { field }` when suggesting `self.field` so we result on
+                    // `Foo { field: self.field }`.
+                    let field = match source {
+                        PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. })) => {
+                            expr.fields.iter().find(|f| f.ident == item_ident)
+                        }
+                        _ => None,
+                    };
+                    let pre = if let Some(field) = field && field.is_shorthand {
+                        format!("{item_ident}: ")
+                    } else {
+                        String::new()
+                    };
+                    // Ensure we provide a structured suggestion for an assoc fn only for
+                    // expressions that are actually a fn call.
+                    let is_call = match field {
+                        Some(ast::ExprField { expr, .. }) => {
+                            matches!(expr.kind, ExprKind::Call(..))
+                        }
+                        _ => matches!(
+                            source,
+                            PathSource::Expr(Some(Expr { kind: ExprKind::Call(..), ..})),
+                        ),
+                    };
+
                     match &item.kind {
                         AssocItemKind::Fn(fn_)
-                        if !sig.decl.has_self() && fn_.sig.decl.has_self() => {
+                        if (!sig.decl.has_self() || !is_call) && fn_.sig.decl.has_self() => {
                             // Ensure that we only suggest `self.` if `self` is available,
                             // you can't call `fn foo(&self)` from `fn bar()` (#115992).
                             // We also want to mention that the method exists.
@@ -243,20 +270,28 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                             ));
                             None
                         }
+                        AssocItemKind::Fn(fn_)
+                        if !fn_.sig.decl.has_self() && !is_call => {
+                            span_label = Some((
+                                item.ident.span,
+                                "an associated function by that name is available on `Self` here",
+                            ));
+                            None
+                        }
                         AssocItemKind::Fn(fn_) if fn_.sig.decl.has_self() => Some((
                             sp,
                             "consider using the method on `Self`",
-                            "self.".to_string(),
+                            format!("{pre}self."),
                         )),
                         AssocItemKind::Fn(_) => Some((
                             sp,
                             "consider using the associated function on `Self`",
-                            "Self::".to_string(),
+                            format!("{pre}Self::"),
                         )),
                         AssocItemKind::Const(..) => Some((
                             sp,
                             "consider using the associated constant on `Self`",
-                            "Self::".to_string(),
+                            format!("{pre}Self::"),
                         )),
                         _ => None
                     }
@@ -621,17 +656,30 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                 self.lookup_assoc_candidate(ident, ns, is_expected, source.is_call())
             {
                 let self_is_available = self.self_value_is_available(path[0].ident.span);
+                // Account for `Foo { field }` when suggesting `self.field` so we result on
+                // `Foo { field: self.field }`.
+                let pre = match source {
+                    PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. }))
+                        if expr
+                            .fields
+                            .iter()
+                            .any(|f| f.ident == path[0].ident && f.is_shorthand) =>
+                    {
+                        format!("{path_str}: ")
+                    }
+                    _ => String::new(),
+                };
                 match candidate {
-                    AssocSuggestion::Field => {
+                    AssocSuggestion::Field(field_span) => {
                         if self_is_available {
-                            err.span_suggestion(
-                                span,
+                            err.span_suggestion_verbose(
+                                span.shrink_to_lo(),
                                 "you might have meant to use the available field",
-                                format!("self.{path_str}"),
+                                format!("{pre}self."),
                                 Applicability::MachineApplicable,
                             );
                         } else {
-                            err.span_label(span, "a field by this name exists in `Self`");
+                            err.span_label(field_span, "a field by that name exists in `Self`");
                         }
                     }
                     AssocSuggestion::MethodWithSelf { called } if self_is_available => {
@@ -640,10 +688,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                         } else {
                             "you might have meant to refer to the method"
                         };
-                        err.span_suggestion(
-                            span,
+                        err.span_suggestion_verbose(
+                            span.shrink_to_lo(),
                             msg,
-                            format!("self.{path_str}"),
+                            "self.".to_string(),
                             Applicability::MachineApplicable,
                         );
                     }
@@ -651,10 +699,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                     | AssocSuggestion::AssocFn { .. }
                     | AssocSuggestion::AssocConst
                     | AssocSuggestion::AssocType => {
-                        err.span_suggestion(
-                            span,
+                        err.span_suggestion_verbose(
+                            span.shrink_to_lo(),
                             format!("you might have meant to {}", candidate.action()),
-                            format!("Self::{path_str}"),
+                            "Self::".to_string(),
                             Applicability::MachineApplicable,
                         );
                     }
@@ -1667,11 +1715,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                         resolution.full_res()
                     {
                         if let Some(field_ids) = self.r.field_def_ids(did) {
-                            if field_ids
+                            if let Some(field_id) = field_ids
                                 .iter()
-                                .any(|&field_id| ident.name == self.r.tcx.item_name(field_id))
+                                .find(|&&field_id| ident.name == self.r.tcx.item_name(field_id))
                             {
-                                return Some(AssocSuggestion::Field);
+                                return Some(AssocSuggestion::Field(self.r.def_span(*field_id)));
                             }
                         }
                     }