Rollup merge of #103560 - zbyrn:issue-103358-fix, r=cjgillot

Point only to the identifiers in the typo suggestions of shadowed names instead of the entire struct

Fixes #103358.

As discussed in the issue, the `Span` of the candidate `Ident` for a typo replacement is stored alongside its `Symbol` in `TypoSuggestion`. Then, the span of the identifier is what the "you might have meant to refer to" note is pointed at, rather than the entire struct definition.

Comments in #103111 and the issue both suggest that it is desirable to:
1. include names defined in the same crate as the typo,
2. ignore names defined elsewhere such as in `std`, _and_
3. include names introduced indirectly via `use`.

Since a name from another crate but introduced via `use` has non-local `def_id`, to achieve this, a suggestion is displayed if either the `def_id` of the suggested name is local, or the `span` of the suggested name is in the same file as the typo itself.

Some UI tests have also been modified to reflect this change.

r? `@cjgillot`

Author: Dylan-DPC

compiler/rustc_resolve/src/diagnostics.rs

@@ -58,16 +58,32 @@ pub(crate) enum SuggestionTarget {
 #[derive(Debug)]
 pub(crate) struct TypoSuggestion {
     pub candidate: Symbol,
+    /// The source location where the name is defined; None if the name is not defined
+    /// in source e.g. primitives
+    pub span: Option<Span>,
     pub res: Res,
     pub target: SuggestionTarget,
 }
 
 impl TypoSuggestion {
-    pub(crate) fn typo_from_res(candidate: Symbol, res: Res) -> TypoSuggestion {
-        Self { candidate, res, target: SuggestionTarget::SimilarlyNamed }
+    pub(crate) fn typo_from_ident(ident: Ident, res: Res) -> TypoSuggestion {
+        Self {
+            candidate: ident.name,
+            span: Some(ident.span),
+            res,
+            target: SuggestionTarget::SimilarlyNamed,
+        }
+    }
+    pub(crate) fn typo_from_name(candidate: Symbol, res: Res) -> TypoSuggestion {
+        Self { candidate, span: None, res, target: SuggestionTarget::SimilarlyNamed }
     }
-    pub(crate) fn single_item_from_res(candidate: Symbol, res: Res) -> TypoSuggestion {
-        Self { candidate, res, target: SuggestionTarget::SingleItem }
+    pub(crate) fn single_item_from_ident(ident: Ident, res: Res) -> TypoSuggestion {
+        Self {
+            candidate: ident.name,
+            span: Some(ident.span),
+            res,
+            target: SuggestionTarget::SingleItem,
+        }
     }
 }
 
@@ -490,7 +506,7 @@ impl<'a> Resolver<'a> {
             if let Some(binding) = resolution.borrow().binding {
                 let res = binding.res();
                 if filter_fn(res) && ctxt.map_or(true, |ctxt| ctxt == key.ident.span.ctxt()) {
-                    names.push(TypoSuggestion::typo_from_res(key.ident.name, res));
+                    names.push(TypoSuggestion::typo_from_ident(key.ident, res));
                 }
             }
         }
@@ -1145,7 +1161,7 @@ impl<'a> Resolver<'a> {
                                 .get(&expn_id)
                                 .into_iter()
                                 .flatten()
-                                .map(|ident| TypoSuggestion::typo_from_res(ident.name, res)),
+                                .map(|ident| TypoSuggestion::typo_from_ident(*ident, res)),
                         );
                     }
                 }
@@ -1164,7 +1180,7 @@ impl<'a> Resolver<'a> {
                                 suggestions.extend(
                                     ext.helper_attrs
                                         .iter()
-                                        .map(|name| TypoSuggestion::typo_from_res(*name, res)),
+                                        .map(|name| TypoSuggestion::typo_from_name(*name, res)),
                                 );
                             }
                         }
@@ -1174,8 +1190,8 @@ impl<'a> Resolver<'a> {
                     if let MacroRulesScope::Binding(macro_rules_binding) = macro_rules_scope.get() {
                         let res = macro_rules_binding.binding.res();
                         if filter_fn(res) {
-                            suggestions.push(TypoSuggestion::typo_from_res(
-                                macro_rules_binding.ident.name,
+                            suggestions.push(TypoSuggestion::typo_from_ident(
+                                macro_rules_binding.ident,
                                 res,
                             ))
                         }
@@ -1193,7 +1209,7 @@ impl<'a> Resolver<'a> {
                     suggestions.extend(this.macro_use_prelude.iter().filter_map(
                         |(name, binding)| {
                             let res = binding.res();
-                            filter_fn(res).then_some(TypoSuggestion::typo_from_res(*name, res))
+                            filter_fn(res).then_some(TypoSuggestion::typo_from_name(*name, res))
                         },
                     ));
                 }
@@ -1203,22 +1219,22 @@ impl<'a> Resolver<'a> {
                         suggestions.extend(
                             BUILTIN_ATTRIBUTES
                                 .iter()
-                                .map(|attr| TypoSuggestion::typo_from_res(attr.name, res)),
+                                .map(|attr| TypoSuggestion::typo_from_name(attr.name, res)),
                         );
                     }
                 }
                 Scope::ExternPrelude => {
                     suggestions.extend(this.extern_prelude.iter().filter_map(|(ident, _)| {
                         let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id());
-                        filter_fn(res).then_some(TypoSuggestion::typo_from_res(ident.name, res))
+                        filter_fn(res).then_some(TypoSuggestion::typo_from_ident(*ident, res))
                     }));
                 }
                 Scope::ToolPrelude => {
                     let res = Res::NonMacroAttr(NonMacroAttrKind::Tool);
                     suggestions.extend(
                         this.registered_tools
                             .iter()
-                            .map(|ident| TypoSuggestion::typo_from_res(ident.name, res)),
+                            .map(|ident| TypoSuggestion::typo_from_ident(*ident, res)),
                     );
                 }
                 Scope::StdLibPrelude => {
@@ -1235,7 +1251,8 @@ impl<'a> Resolver<'a> {
                 Scope::BuiltinTypes => {
                     suggestions.extend(PrimTy::ALL.iter().filter_map(|prim_ty| {
                         let res = Res::PrimTy(*prim_ty);
-                        filter_fn(res).then_some(TypoSuggestion::typo_from_res(prim_ty.name(), res))
+                        filter_fn(res)
+                            .then_some(TypoSuggestion::typo_from_name(prim_ty.name(), res))
                     }))
                 }
             }

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -150,15 +150,15 @@ struct BaseError {
 #[derive(Debug)]
 enum TypoCandidate {
     Typo(TypoSuggestion),
-    Shadowed(Res),
+    Shadowed(Res, Option<Span>),
     None,
 }
 
 impl TypoCandidate {
     fn to_opt_suggestion(self) -> Option<TypoSuggestion> {
         match self {
             TypoCandidate::Typo(sugg) => Some(sugg),
-            TypoCandidate::Shadowed(_) | TypoCandidate::None => None,
+            TypoCandidate::Shadowed(_, _) | TypoCandidate::None => None,
         }
     }
 }
@@ -691,9 +691,20 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
         let is_expected = &|res| source.is_expected(res);
         let ident_span = path.last().map_or(span, |ident| ident.ident.span);
         let typo_sugg = self.lookup_typo_candidate(path, source.namespace(), is_expected);
-        if let TypoCandidate::Shadowed(res) = typo_sugg
-            && let Some(id) = res.opt_def_id()
-            && let Some(sugg_span) = self.r.opt_span(id)
+        let is_in_same_file = &|sp1, sp2| {
+            let source_map = self.r.session.source_map();
+            let file1 = source_map.span_to_filename(sp1);
+            let file2 = source_map.span_to_filename(sp2);
+            file1 == file2
+        };
+        // print 'you might have meant' if the candidate is (1) is a shadowed name with
+        // accessible definition and (2) either defined in the same crate as the typo
+        // (could be in a different file) or introduced in the same file as the typo
+        // (could belong to a different crate)
+        if let TypoCandidate::Shadowed(res, Some(sugg_span)) = typo_sugg
+            && res
+                .opt_def_id()
+                .map_or(false, |id| id.is_local() || is_in_same_file(span, sugg_span))
         {
             err.span_label(
                 sugg_span,
@@ -970,10 +981,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                         .collect();
                 if targets.len() == 1 {
                     let target = targets[0];
-                    return Some(TypoSuggestion::single_item_from_res(
-                        target.0.ident.name,
-                        target.1,
-                    ));
+                    return Some(TypoSuggestion::single_item_from_ident(target.0.ident, target.1));
                 }
             }
         }
@@ -1615,7 +1623,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                 // Locals and type parameters
                 for (ident, &res) in &rib.bindings {
                     if filter_fn(res) && ident.span.ctxt() == rib_ctxt {
-                        names.push(TypoSuggestion::typo_from_res(ident.name, res));
+                        names.push(TypoSuggestion::typo_from_ident(*ident, res));
                     }
                 }
 
@@ -1644,9 +1652,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                                             Res::Def(DefKind::Mod, crate_id.as_def_id());
 
                                         if filter_fn(crate_mod) {
-                                            Some(TypoSuggestion::typo_from_res(
-                                                ident.name, crate_mod,
-                                            ))
+                                            Some(TypoSuggestion::typo_from_ident(*ident, crate_mod))
                                         } else {
                                             None
                                         }
@@ -1665,7 +1671,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
             // Add primitive types to the mix
             if filter_fn(Res::PrimTy(PrimTy::Bool)) {
                 names.extend(PrimTy::ALL.iter().map(|prim_ty| {
-                    TypoSuggestion::typo_from_res(prim_ty.name(), Res::PrimTy(*prim_ty))
+                    TypoSuggestion::typo_from_name(prim_ty.name(), Res::PrimTy(*prim_ty))
                 }))
             }
         } else {
@@ -1692,7 +1698,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                     return TypoCandidate::None;
                 };
                 if found == name {
-                    TypoCandidate::Shadowed(sugg.res)
+                    TypoCandidate::Shadowed(sugg.res, sugg.span)
                 } else {
                     TypoCandidate::Typo(sugg)
                 }

src/test/ui/lexical-scopes.stderr

@@ -2,7 +2,7 @@ error[E0574]: expected struct, variant or union type, found type parameter `T`
   --> $DIR/lexical-scopes.rs:3:13
    |
 LL | struct T { i: i32 }
-   | ------------------- you might have meant to refer to this struct
+   |        - you might have meant to refer to this struct
 LL | fn f<T>() {
    |      - found this type parameter
 LL |     let t = T { i: 0 };

src/test/ui/resolve/point-at-type-parameter-shadowing-another-type.stderr

@@ -1,16 +1,14 @@
 error[E0574]: expected struct, variant or union type, found type parameter `Baz`
   --> $DIR/point-at-type-parameter-shadowing-another-type.rs:16:13
    |
-LL | / struct Baz {
-LL | |     num: usize,
-LL | | }
-   | |_- you might have meant to refer to this struct
-LL |
-LL |   impl<Baz> Foo<Baz> for Bar {
-   |        --- found this type parameter
+LL | struct Baz {
+   |        --- you might have meant to refer to this struct
 ...
-LL |               Baz { num } => num,
-   |               ^^^ not a struct, variant or union type
+LL | impl<Baz> Foo<Baz> for Bar {
+   |      --- found this type parameter
+...
+LL |             Baz { num } => num,
+   |             ^^^ not a struct, variant or union type
 
 error: aborting due to previous error
 

src/test/ui/span/issue-35987.stderr

@@ -1,6 +1,9 @@
 error[E0404]: expected trait, found type parameter `Add`
   --> $DIR/issue-35987.rs:5:21
    |
+LL | use std::ops::Add;
+   |               --- you might have meant to refer to this trait
+LL |
 LL | impl<T: Clone, Add> Add for Foo<T> {
    |                ---  ^^^ not a trait
    |                |