diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 93db6cfc4635d..0de8e91f0e36b 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -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> = 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::>(), + 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(); } diff --git a/tests/ui/imports/issue-55884-2.stderr b/tests/ui/imports/issue-55884-2.stderr index 67d4114149a4b..4fd3cb6fbea88 100644 --- a/tests/ui/imports/issue-55884-2.stderr +++ b/tests/ui/imports/issue-55884-2.stderr @@ -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 previous error diff --git a/tests/ui/imports/private-std-reexport-suggest-public.fixed b/tests/ui/imports/private-std-reexport-suggest-public.fixed new file mode 100644 index 0000000000000..b6fd22f5d3ffc --- /dev/null +++ b/tests/ui/imports/private-std-reexport-suggest-public.fixed @@ -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; +} diff --git a/tests/ui/imports/private-std-reexport-suggest-public.rs b/tests/ui/imports/private-std-reexport-suggest-public.rs new file mode 100644 index 0000000000000..1247055af6063 --- /dev/null +++ b/tests/ui/imports/private-std-reexport-suggest-public.rs @@ -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; +} diff --git a/tests/ui/imports/private-std-reexport-suggest-public.stderr b/tests/ui/imports/private-std-reexport-suggest-public.stderr new file mode 100644 index 0000000000000..09693c2d59ce3 --- /dev/null +++ b/tests/ui/imports/private-std-reexport-suggest-public.stderr @@ -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 previous error + +For more information about this error, try `rustc --explain E0603`. diff --git a/tests/ui/privacy/privacy2.stderr b/tests/ui/privacy/privacy2.stderr index e7135d3fd8a61..46bb9823dbf8f 100644 --- a/tests/ui/privacy/privacy2.stderr +++ b/tests/ui/privacy/privacy2.stderr @@ -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 diff --git a/tests/ui/proc-macro/disappearing-resolution.stderr b/tests/ui/proc-macro/disappearing-resolution.stderr index 5b969549a117c..e6d0868687e27 100644 --- a/tests/ui/proc-macro/disappearing-resolution.stderr +++ b/tests/ui/proc-macro/disappearing-resolution.stderr @@ -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