about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-03-06 22:02:46 +0100
committerGitHub <noreply@github.com>2024-03-06 22:02:46 +0100
commitc7fca03240dfba93363505bc9c6839a224cc8ca6 (patch)
treed9f30566f204f3ec6d2dead32f8c8bd8c6ce209d
parent4d9cdd66961283c89874f81314d0e0703f5807dd (diff)
parent73030143818b99f0180a259b161cacd175b7d775 (diff)
Rollup merge of #121190 - bvanjoi:fix-114884, r=petrochenkov
avoid overlapping privacy suggestion for single nested imports

Fixes #114884

This PR aims to avoid confusion inside braces for import suggestions.

r? ``@petrochenkov``
-rw-r--r--compiler/rustc_resolve/src/diagnostics.rs62
-rw-r--r--compiler/rustc_resolve/src/ident.rs3
-rw-r--r--compiler/rustc_resolve/src/imports.rs2
-rw-r--r--compiler/rustc_resolve/src/lib.rs2
-rw-r--r--tests/ui/imports/append-import-suggestion.rs16
-rw-r--r--tests/ui/imports/append-import-suggestion.stderr21
6 files changed, 81 insertions, 25 deletions
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
index 833ebfa03e5..0bb2a69ae99 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<DefId>, &'a Option<String>, 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<DefId>, &Option<String>, bool)> =
-        Vec::new();
-    let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>, &Option<String>, bool)> =
-        Vec::new();
+    let mut accessible_path_strings: Vec<PathString<'_>> = Vec::new();
+    let mut inaccessible_path_strings: Vec<PathString<'_>> = 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<PathString<'_>>| {
+            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 a9fdddbfab9..6518d9735ae 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 6647a9a2792..9e5b2fe094f 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 0434ff17193..5de147c2b24 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 00000000000..6b75804b86f
--- /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 00000000000..6d7b657f3d1
--- /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`.