Skip to content

Commit

Permalink
Rollup merge of rust-lang#125852 - bvanjoi:improve-tip-for-invisible-…
Browse files Browse the repository at this point in the history 
…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;
   |
```
  • Loading branch information
@jieyouxu
jieyouxu authored Jun 19, 2024
2 parents 03032b8 + 586821e commit aad1f30
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 57 deletions.
185 changes: 140 additions & 45 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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",
Expand Down
15 changes: 3 additions & 12 deletions tests/ui/traits/item-privacy.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit aad1f30

Please sign in to comment.