From 73030143818b99f0180a259b161cacd175b7d775 Mon Sep 17 00:00:00 2001 From: bohan Date: Sat, 17 Feb 2024 00:55:35 +0800 Subject: [PATCH] avoid overlapping privacy suggestion for single nested imports --- compiler/rustc_resolve/src/diagnostics.rs | 62 ++++++++++++------- compiler/rustc_resolve/src/ident.rs | 3 +- compiler/rustc_resolve/src/imports.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 2 + tests/ui/imports/append-import-suggestion.rs | 16 +++++ .../imports/append-import-suggestion.stderr | 21 +++++++ 6 files changed, 81 insertions(+), 25 deletions(-) create mode 100644 tests/ui/imports/append-import-suggestion.rs create mode 100644 tests/ui/imports/append-import-suggestion.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 833ebfa03e5fc..0bb2a69ae9926 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1736,7 +1736,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'a>) { - let PrivacyError { ident, binding, outermost_res, parent_scope, dedup_span } = + let PrivacyError { ident, binding, outermost_res, parent_scope, single_nested, dedup_span } = *privacy_error; let res = binding.res(); @@ -1775,7 +1775,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { &import_suggestions, Instead::Yes, FoundUse::Yes, - DiagMode::Import, + DiagMode::Import { append: single_nested }, vec![], "", ); @@ -2701,7 +2701,11 @@ pub(crate) enum DiagMode { /// The binding is part of a pattern Pattern, /// The binding is part of a use statement - Import, + Import { + /// `true` mean add the tips afterward for case `use a::{b,c}`, + /// rather than replacing within. + append: bool, + }, } pub(crate) fn import_candidates( @@ -2726,6 +2730,8 @@ pub(crate) fn import_candidates( ); } +type PathString<'a> = (String, &'a str, Option, &'a Option, bool); + /// When an entity with a given name is not available in scope, we search for /// entities with that name in all crates. This method allows outputting the /// results of this search in a programmer-friendly way. If any entities are @@ -2746,10 +2752,8 @@ fn show_candidates( return false; } - let mut accessible_path_strings: Vec<(String, &str, Option, &Option, bool)> = - Vec::new(); - let mut inaccessible_path_strings: Vec<(String, &str, Option, &Option, bool)> = - Vec::new(); + let mut accessible_path_strings: Vec> = Vec::new(); + let mut inaccessible_path_strings: Vec> = Vec::new(); candidates.iter().for_each(|c| { if c.accessible { @@ -2811,6 +2815,15 @@ fn show_candidates( err.note(note.clone()); } + let append_candidates = |msg: &mut String, accessible_path_strings: Vec>| { + msg.push(':'); + + for candidate in accessible_path_strings { + msg.push('\n'); + msg.push_str(&candidate.0); + } + }; + if let Some(span) = use_placement_span { let (add_use, trailing) = match mode { DiagMode::Pattern => { @@ -2822,7 +2835,7 @@ fn show_candidates( ); return true; } - DiagMode::Import => ("", ""), + DiagMode::Import { .. } => ("", ""), DiagMode::Normal => ("use ", ";\n"), }; for candidate in &mut accessible_path_strings { @@ -2839,13 +2852,22 @@ fn show_candidates( format!("{add_use}{}{append}{trailing}{additional_newline}", &candidate.0); } - err.span_suggestions_with_style( - span, - msg, - accessible_path_strings.into_iter().map(|a| a.0), - Applicability::MaybeIncorrect, - SuggestionStyle::ShowAlways, - ); + match mode { + DiagMode::Import { append: true, .. } => { + append_candidates(&mut msg, accessible_path_strings); + err.span_help(span, msg); + } + _ => { + err.span_suggestions_with_style( + span, + msg, + accessible_path_strings.into_iter().map(|a| a.0), + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); + } + } + if let [first, .., last] = &path[..] { let sp = first.ident.span.until(last.ident.span); // Our suggestion is empty, so make sure the span is not empty (or we'd ICE). @@ -2860,17 +2882,11 @@ fn show_candidates( } } } else { - msg.push(':'); - - for candidate in accessible_path_strings { - msg.push('\n'); - msg.push_str(&candidate.0); - } - + append_candidates(&mut msg, accessible_path_strings); err.help(msg); } true - } else if !(inaccessible_path_strings.is_empty() || matches!(mode, DiagMode::Import)) { + } else if !(inaccessible_path_strings.is_empty() || matches!(mode, DiagMode::Import { .. })) { let prefix = if let DiagMode::Pattern = mode { "you might have meant to match on " } else { "" }; if let [(name, descr, def_id, note, _)] = &inaccessible_path_strings[..] { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index a9fdddbfab90f..6518d9735ae52 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -868,7 +868,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .into_iter() .find_map(|binding| if binding == ignore_binding { None } else { binding }); - if let Some(Finalize { path_span, report_private, used, .. }) = finalize { + if let Some(Finalize { path_span, report_private, used, root_span, .. }) = finalize { let Some(binding) = binding else { return Err((Determined, Weak::No)); }; @@ -881,6 +881,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { dedup_span: path_span, outermost_res: None, parent_scope: *parent_scope, + single_nested: path_span != root_span, }); } else { return Err((Determined, Weak::No)); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6647a9a279285..9e5b2fe094f4c 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -715,7 +715,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { &mut diag, Some(err.span), candidates, - DiagMode::Import, + DiagMode::Import { append: false }, (source != target) .then(|| format!(" as {target}")) .as_deref() diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 0434ff171934e..5de147c2b2444 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -729,6 +729,8 @@ struct PrivacyError<'a> { dedup_span: Span, outermost_res: Option<(Res, Ident)>, parent_scope: ParentScope<'a>, + /// Is the format `use a::{b,c}`? + single_nested: bool, } #[derive(Debug)] diff --git a/tests/ui/imports/append-import-suggestion.rs b/tests/ui/imports/append-import-suggestion.rs new file mode 100644 index 0000000000000..6b75804b86fcb --- /dev/null +++ b/tests/ui/imports/append-import-suggestion.rs @@ -0,0 +1,16 @@ +// https://github.com/rust-lang/rust/issues/114884 + +mod mod1 { + pub trait TraitA {} +} + +mod mod2 { + mod sub_mod { + use super::super::mod1::TraitA; + } +} + +use mod2::{sub_mod::TraitA}; +//~^ ERROR: module `sub_mod` is private + +fn main() {} diff --git a/tests/ui/imports/append-import-suggestion.stderr b/tests/ui/imports/append-import-suggestion.stderr new file mode 100644 index 0000000000000..6d7b657f3d128 --- /dev/null +++ b/tests/ui/imports/append-import-suggestion.stderr @@ -0,0 +1,21 @@ +error[E0603]: module `sub_mod` is private + --> $DIR/append-import-suggestion.rs:13:12 + | +LL | use mod2::{sub_mod::TraitA}; + | ^^^^^^^ private module + | +help: consider importing this trait instead: + mod1::TraitA + --> $DIR/append-import-suggestion.rs:13:12 + | +LL | use mod2::{sub_mod::TraitA}; + | ^^^^^^^^^^^^^^^ +note: the module `sub_mod` is defined here + --> $DIR/append-import-suggestion.rs:8:5 + | +LL | mod sub_mod { + | ^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0603`.