Structured `use` suggestion on privacy error

When encoutering a privacy error on an item through a re-export that is
accessible in an alternative path, provide a structured suggestion with
that path.

```
error[E0603]: module import `mem` is private
  --> $DIR/private-std-reexport-suggest-public.rs:4:14
   |
LL |     use foo::mem;
   |              ^^^ private module import
   |
note: the module import `mem` is defined here...
  --> $DIR/private-std-reexport-suggest-public.rs:8:9
   |
LL |     use std::mem;
   |         ^^^^^^^^
note: ...and refers to the module `mem` which is defined here
  --> $SRC_DIR/std/src/lib.rs:LL:COL
   |
   = note: you could import this
help: import `mem` through the re-export
   |
LL |     use std::mem;
   |         ~~~~~~~~
```

Fix #42909.
This commit is contained in:
Esteban Küber 2023-11-19 17:46:44 +00:00
parent 0e2dac8375
commit beaf31581a
7 changed files with 140 additions and 6 deletions

View File

@ -1697,6 +1697,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
struct_span_err!(self.tcx.sess, ident.span, E0603, "{} `{}` is private", descr, ident);
err.span_label(ident.span, format!("private {descr}"));
let mut not_publicly_reexported = false;
if let Some((this_res, outer_ident)) = outermost_res {
let import_suggestions = self.lookup_import_candidates(
outer_ident,
@ -1717,6 +1718,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
);
// If we suggest importing a public re-export, don't point at the definition.
if point_to_def && ident.span != outer_ident.span {
not_publicly_reexported = true;
err.span_label(
outer_ident.span,
format!("{} `{outer_ident}` is not publicly re-exported", this_res.descr()),
@ -1749,10 +1751,51 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
let mut sugg_paths = vec![];
if let Some(mut def_id) = res.opt_def_id() {
// We can't use `def_path_str` in resolve.
let mut path = vec![def_id];
while let Some(parent) = self.tcx.opt_parent(def_id) {
def_id = parent;
if !def_id.is_top_level_module() {
path.push(def_id);
} else {
break;
}
}
// We will only suggest importing directly if it is accessible through that path.
let path_names: Option<Vec<String>> = path
.iter()
.rev()
.map(|def_id| {
self.tcx.opt_item_name(*def_id).map(|n| {
if def_id.is_top_level_module() {
"crate".to_string()
} else {
n.to_string()
}
})
})
.collect();
if let Some(def_id) = path.get(0)
&& let Some(path) = path_names
{
if let Some(def_id) = def_id.as_local() {
if self.effective_visibilities.is_directly_public(def_id) {
sugg_paths.push((path, false));
}
} else if self.is_accessible_from(self.tcx.visibility(def_id), parent_scope.module)
{
sugg_paths.push((path, false));
}
}
}
// Print the whole import chain to make it easier to see what happens.
let first_binding = binding;
let mut next_binding = Some(binding);
let mut next_ident = ident;
let mut path = vec![];
while let Some(binding) = next_binding {
let name = next_ident;
next_binding = match binding.kind {
@ -1771,6 +1814,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
_ => None,
};
match binding.kind {
NameBindingKind::Import { import, .. } => {
for segment in import.module_path.iter().skip(1) {
path.push(segment.ident.to_string());
}
sugg_paths.push((
path.iter()
.cloned()
.chain(vec![ident.to_string()].into_iter())
.collect::<Vec<_>>(),
true, // re-export
));
}
NameBindingKind::Res(_) | NameBindingKind::Module(_) => {}
}
let first = binding == first_binding;
let msg = format!(
"{and_refers_to}the {item} `{name}`{which} is defined here{dots}",
@ -1782,7 +1840,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let def_span = self.tcx.sess.source_map().guess_head_span(binding.span);
let mut note_span = MultiSpan::from_span(def_span);
if !first && binding.vis.is_public() {
note_span.push_span_label(def_span, "consider importing it directly");
let desc = match binding.kind {
NameBindingKind::Import { .. } => "re-export",
_ => "directly",
};
note_span.push_span_label(def_span, format!("you could import this {desc}"));
}
// Final step in the import chain, point out if the ADT is `non_exhaustive`
// which is probably why this privacy violation occurred.
@ -1796,6 +1858,29 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
err.span_note(note_span, msg);
}
// We prioritize shorter paths, non-core imports and direct imports over the alternatives.
sugg_paths.sort_by_key(|(p, reexport)| (p.len(), p[0] == "core", *reexport));
for (sugg, reexport) in sugg_paths {
if not_publicly_reexported {
break;
}
if sugg.len() <= 1 {
// A single path segment suggestion is wrong. This happens on circular imports.
// `tests/ui/imports/issue-55884-2.rs`
continue;
}
let path = sugg.join("::");
err.span_suggestion_verbose(
dedup_span,
format!(
"import `{ident}` {}",
if reexport { "through the re-export" } else { "directly" }
),
path,
Applicability::MachineApplicable,
);
break;
}
err.emit();
}

View File

@ -13,17 +13,21 @@ note: ...and refers to the struct import `ParseOptions` which is defined here...
--> $DIR/issue-55884-2.rs:13:9
|
LL | pub use parser::ParseOptions;
| ^^^^^^^^^^^^^^^^^^^^ consider importing it directly
| ^^^^^^^^^^^^^^^^^^^^ you could import this re-export
note: ...and refers to the struct import `ParseOptions` which is defined here...
--> $DIR/issue-55884-2.rs:6:13
|
LL | pub use options::*;
| ^^^^^^^^^^ consider importing it directly
| ^^^^^^^^^^ you could import this re-export
note: ...and refers to the struct `ParseOptions` which is defined here
--> $DIR/issue-55884-2.rs:2:5
|
LL | pub struct ParseOptions {}
| ^^^^^^^^^^^^^^^^^^^^^^^ consider importing it directly
| ^^^^^^^^^^^^^^^^^^^^^^^ you could import this directly
help: import `ParseOptions` through the re-export
|
LL | pub use parser::ParseOptions;
| ~~~~~~~~~~~~~~~~~~~~
error: aborting due to 1 previous error

View File

@ -0,0 +1,9 @@
// run-rustfix
#![allow(unused_imports)]
fn main() {
use std::mem; //~ ERROR module import `mem` is private
}
pub mod foo {
use std::mem;
}

View File

@ -0,0 +1,9 @@
// run-rustfix
#![allow(unused_imports)]
fn main() {
use foo::mem; //~ ERROR module import `mem` is private
}
pub mod foo {
use std::mem;
}

View File

@ -0,0 +1,23 @@
error[E0603]: module import `mem` is private
--> $DIR/private-std-reexport-suggest-public.rs:4:14
|
LL | use foo::mem;
| ^^^ private module import
|
note: the module import `mem` is defined here...
--> $DIR/private-std-reexport-suggest-public.rs:8:9
|
LL | use std::mem;
| ^^^^^^^^
note: ...and refers to the module `mem` which is defined here
--> $SRC_DIR/std/src/lib.rs:LL:COL
|
= note: you could import this directly
help: import `mem` through the re-export
|
LL | use std::mem;
| ~~~~~~~~
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0603`.

View File

@ -19,7 +19,7 @@ note: ...and refers to the function `foo` which is defined here
--> $DIR/privacy2.rs:16:1
|
LL | pub fn foo() {}
| ^^^^^^^^^^^^ consider importing it directly
| ^^^^^^^^^^^^ you could import this directly
error: requires `sized` lang_item

View File

@ -19,7 +19,11 @@ note: ...and refers to the derive macro `Empty` which is defined here
--> $DIR/auxiliary/test-macros.rs:25:1
|
LL | pub fn empty_derive(_: TokenStream) -> TokenStream {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ consider importing it directly
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ you could import this directly
help: import `Empty` directly
|
LL | use test_macros::Empty;
| ~~~~~~~~~~~~~~~~~~
error: aborting due to 2 previous errors