Auto merge of rust-lang#125852 - bvanjoi:improve-tip-for-invisible-trait, r=compiler-errors

improve tip for inaccessible traits

Improve the tips when the candidate method is from an inaccessible trait.

For example:

```rs
mod m {
  trait Trait {
    fn f() {}
  }
  impl<T> Trait for T {}
}

fn main() {
  struct S;
  S::f();
}
```

The difference between before and now is:

```diff
error[E0599]: no function or associated item named `f` found for struct `S` in the current scope
  --> ./src/main.rs:88:6
   |
LL |   struct S;
   |   -------- function or associated item `f` not found for this struct
LL |   S::f();
   |      ^ function or associated item not found in `S`
   |
   = help: items from traits can only be used if the trait is implemented and in scope
- help: trait `Trait` which provides `f` is implemented but not in scope; perhaps you want to import it
+ help: trait `crate::m::Trait` which provides `f` is implemented but not reachable
   |
- LL + use crate::m::Trait;
   |
```

Author: bors

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -303,14 +303,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 }
                 if !candidates.is_empty() {
                     let help = format!(
-                        "{an}other candidate{s} {were} found in the following trait{s}, perhaps \
-                         add a `use` for {one_of_them}:",
+                        "{an}other candidate{s} {were} found in the following trait{s}",
                         an = if candidates.len() == 1 { "an" } else { "" },
                         s = pluralize!(candidates.len()),
                         were = pluralize!("was", candidates.len()),
-                        one_of_them = if candidates.len() == 1 { "it" } else { "one_of_them" },
                     );
-                    self.suggest_use_candidates(&mut err, help, candidates);
+                    self.suggest_use_candidates(
+                        candidates,
+                        |accessible_sugg, inaccessible_sugg, span| {
+                            let suggest_for_access =
+                                |err: &mut Diag<'_>, mut msg: String, sugg: Vec<_>| {
+                                    msg += &format!(
+                                        ", perhaps add a `use` for {one_of_them}:",
+                                        one_of_them =
+                                            if sugg.len() == 1 { "it" } else { "one_of_them" },
+                                    );
+                                    err.span_suggestions(
+                                        span,
+                                        msg,
+                                        sugg,
+                                        Applicability::MaybeIncorrect,
+                                    );
+                                };
+                            let suggest_for_privacy =
+                                |err: &mut Diag<'_>, mut msg: String, sugg: Vec<String>| {
+                                    if sugg.len() == 1 {
+                                        let msg = format!("\
+                                            trait `{}` provides `{item_name}` is implemented but not reachable",
+                                            sugg[0].trim()
+                                        );
+                                        err.help(msg);
+                                    } else {
+                                        msg += &format!(" but {} not reachable", pluralize!("is", sugg.len()));
+                                        err.span_suggestions(
+                                            span,
+                                            msg,
+                                            sugg,
+                                            Applicability::MaybeIncorrect,
+                                        );
+                                    }
+                                };
+                            if accessible_sugg.is_empty() {
+                                // `inaccessible_sugg` must not be empty
+                                suggest_for_privacy(&mut err, help, inaccessible_sugg);
+                            } else if inaccessible_sugg.is_empty() {
+                                suggest_for_access(&mut err, help, accessible_sugg);
+                            } else {
+                                suggest_for_access(&mut err, help.clone(), accessible_sugg);
+                                suggest_for_privacy(&mut err, help, inaccessible_sugg);
+                            }
+                        },
+                    );
                 }
                 if let ty::Ref(region, t_type, mutability) = rcvr_ty.kind() {
                     if needs_mut {
@@ -3089,49 +3132,69 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
-    fn suggest_use_candidates(&self, err: &mut Diag<'_>, msg: String, candidates: Vec<DefId>) {
+    fn suggest_use_candidates<F>(&self, candidates: Vec<DefId>, handle_candidates: F)
+    where
+        F: FnOnce(Vec<String>, Vec<String>, Span),
+    {
         let parent_map = self.tcx.visible_parent_map(());
 
-        // Separate out candidates that must be imported with a glob, because they are named `_`
-        // and cannot be referred with their identifier.
-        let (candidates, globs): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|trait_did| {
-            if let Some(parent_did) = parent_map.get(trait_did) {
-                // If the item is re-exported as `_`, we should suggest a glob-import instead.
-                if *parent_did != self.tcx.parent(*trait_did)
-                    && self
-                        .tcx
-                        .module_children(*parent_did)
-                        .iter()
-                        .filter(|child| child.res.opt_def_id() == Some(*trait_did))
-                        .all(|child| child.ident.name == kw::Underscore)
-                {
-                    return false;
-                }
-            }
+        let scope = self.tcx.parent_module_from_def_id(self.body_id);
+        let (accessible_candidates, inaccessible_candidates): (Vec<_>, Vec<_>) =
+            candidates.into_iter().partition(|id| {
+                let vis = self.tcx.visibility(*id);
+                vis.is_accessible_from(scope, self.tcx)
+            });
 
-            true
-        });
+        let sugg = |candidates: Vec<_>, visible| {
+            // Separate out candidates that must be imported with a glob, because they are named `_`
+            // and cannot be referred with their identifier.
+            let (candidates, globs): (Vec<_>, Vec<_>) =
+                candidates.into_iter().partition(|trait_did| {
+                    if let Some(parent_did) = parent_map.get(trait_did) {
+                        // If the item is re-exported as `_`, we should suggest a glob-import instead.
+                        if *parent_did != self.tcx.parent(*trait_did)
+                            && self
+                                .tcx
+                                .module_children(*parent_did)
+                                .iter()
+                                .filter(|child| child.res.opt_def_id() == Some(*trait_did))
+                                .all(|child| child.ident.name == kw::Underscore)
+                        {
+                            return false;
+                        }
+                    }
 
-        let module_did = self.tcx.parent_module_from_def_id(self.body_id);
-        let (module, _, _) = self.tcx.hir().get_module(module_did);
-        let span = module.spans.inject_use_span;
+                    true
+                });
 
-        let path_strings = candidates.iter().map(|trait_did| {
-            format!("use {};\n", with_crate_prefix!(self.tcx.def_path_str(*trait_did)),)
-        });
+            let prefix = if visible { "use " } else { "" };
+            let postfix = if visible { ";" } else { "" };
+            let path_strings = candidates.iter().map(|trait_did| {
+                format!(
+                    "{prefix}{}{postfix}\n",
+                    with_crate_prefix!(self.tcx.def_path_str(*trait_did)),
+                )
+            });
 
-        let glob_path_strings = globs.iter().map(|trait_did| {
-            let parent_did = parent_map.get(trait_did).unwrap();
-            format!(
-                "use {}::*; // trait {}\n",
-                with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
-                self.tcx.item_name(*trait_did),
-            )
-        });
-        let mut sugg: Vec<_> = path_strings.chain(glob_path_strings).collect();
-        sugg.sort();
+            let glob_path_strings = globs.iter().map(|trait_did| {
+                let parent_did = parent_map.get(trait_did).unwrap();
+                format!(
+                    "{prefix}{}::*{postfix} // trait {}\n",
+                    with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
+                    self.tcx.item_name(*trait_did),
+                )
+            });
+            let mut sugg: Vec<_> = path_strings.chain(glob_path_strings).collect();
+            sugg.sort();
+            sugg
+        };
 
-        err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect);
+        let accessible_sugg = sugg(accessible_candidates, true);
+        let inaccessible_sugg = sugg(inaccessible_candidates, false);
+
+        let (module, _, _) = self.tcx.hir().get_module(scope);
+        let span = module.spans.inject_use_span;
+        handle_candidates(accessible_sugg, inaccessible_sugg, span);
     }
 
     fn suggest_valid_traits(
@@ -3155,21 +3218,53 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             if explain {
                 err.help("items from traits can only be used if the trait is in scope");
             }
+
             let msg = format!(
-                "{this_trait_is} implemented but not in scope; perhaps you want to import \
-                 {one_of_them}",
+                "{this_trait_is} implemented but not in scope",
                 this_trait_is = if candidates.len() == 1 {
                     format!(
                         "trait `{}` which provides `{item_name}` is",
                         self.tcx.item_name(candidates[0]),
                     )
                 } else {
                     format!("the following traits which provide `{item_name}` are")
-                },
-                one_of_them = if candidates.len() == 1 { "it" } else { "one of them" },
+                }
             );
 
-            self.suggest_use_candidates(err, msg, candidates);
+            self.suggest_use_candidates(candidates, |accessible_sugg, inaccessible_sugg, span| {
+                let suggest_for_access = |err: &mut Diag<'_>, mut msg: String, sugg: Vec<_>| {
+                    msg += &format!(
+                        "; perhaps you want to import {one_of}",
+                        one_of = if sugg.len() == 1 { "it" } else { "one of them" },
+                    );
+                    err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect);
+                };
+                let suggest_for_privacy = |err: &mut Diag<'_>, sugg: Vec<String>| {
+                    let msg = format!(
+                        "{this_trait_is} implemented but not reachable",
+                        this_trait_is = if sugg.len() == 1 {
+                            format!("trait `{}` which provides `{item_name}` is", sugg[0].trim())
+                        } else {
+                            format!("the following traits which provide `{item_name}` are")
+                        }
+                    );
+                    if sugg.len() == 1 {
+                        err.help(msg);
+                    } else {
+                        err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect);
+                    }
+                };
+                if accessible_sugg.is_empty() {
+                    // `inaccessible_sugg` must not be empty
+                    suggest_for_privacy(err, inaccessible_sugg);
+                } else if inaccessible_sugg.is_empty() {
+                    suggest_for_access(err, msg, accessible_sugg);
+                } else {
+                    suggest_for_access(err, msg, accessible_sugg);
+                    suggest_for_privacy(err, inaccessible_sugg);
+                }
+            });
+
             if let Some(did) = edition_fix {
                 err.note(format!(
                     "'{}' is included in the prelude starting in Edition 2021",

tests/ui/traits/item-privacy.stderr

@@ -8,10 +8,7 @@ LL |     S.a();
    |       ^
    |
    = help: items from traits can only be used if the trait is implemented and in scope
-help: trait `A` which provides `a` is implemented but not in scope; perhaps you want to import it
-   |
-LL + use method::A;
-   |
+   = help: trait `method::A` which provides `a` is implemented but not reachable
 help: there is a method `b` with a similar name
    |
 LL |     S.b();
@@ -58,15 +55,12 @@ LL |     S::a(&S);
    |        ^ function or associated item not found in `S`
    |
    = help: items from traits can only be used if the trait is implemented and in scope
+   = help: trait `method::A` which provides `a` is implemented but not reachable
 help: there is an associated constant `B` with a similar name
   --> $DIR/item-privacy.rs:29:9
    |
 LL |         const B: u8 = 0;
    |         ^^^^^^^^^^^
-help: trait `A` which provides `a` is implemented but not in scope; perhaps you want to import it
-   |
-LL + use method::A;
-   |
 
 error[E0599]: no function or associated item named `b` found for struct `S` in the current scope
   --> $DIR/item-privacy.rs:80:8
@@ -107,10 +101,7 @@ LL |     S::A;
    |        ^ associated item not found in `S`
    |
    = help: items from traits can only be used if the trait is implemented and in scope
-help: trait `A` which provides `A` is implemented but not in scope; perhaps you want to import it
-   |
-LL + use assoc_const::A;
-   |
+   = help: trait `assoc_const::A` which provides `A` is implemented but not reachable
 help: there is an associated constant `B` with a similar name
    |
 LL |     S::B;