Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve tip for inaccessible traits #125852

Merged
merged 1 commit into from
Jun 19, 2024
Merged

improve tip for inaccessible traits #125852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 140 additions & 45 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,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 @@ -3069,49 +3112,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 @@ -3135,21 +3198,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
Open in desktop
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
Loading