avoid overlapping privacy suggestion for single nested imports

bvanjoi · bvanjoi · commit 3c7465b30482 · 2024-02-17T01:29:30.000+08:00
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
@@ -474,7 +474,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     pub(crate) fn lint_if_path_starts_with_module(
         &mut self,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'_>>,
         path: &[Segment],
         second_binding: Option<NameBinding<'_>>,
     ) {
@@ -1694,7 +1694,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, imported, dedup_span } =
             *privacy_error;
 
         let res = binding.res();
@@ -1733,7 +1733,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 &import_suggestions,
                 Instead::Yes,
                 FoundUse::Yes,
-                DiagnosticMode::Import,
+                DiagnosticMode::Import {
+                    single_nested: imported.is_some_and(|imported| {
+                        matches!(imported.kind, ImportKind::Single { nested: true, .. })
+                    }),
+                },
                 vec![],
                 "",
             );
@@ -2659,7 +2663,10 @@ pub(crate) enum DiagnosticMode {
     /// The binding is part of a pattern
     Pattern,
     /// The binding is part of a use statement
-    Import,
+    Import {
+        /// Is the format `use a::{b,c}`
+        single_nested: bool,
+    },
 }
 
 pub(crate) fn import_candidates(
@@ -2684,6 +2691,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
@@ -2704,10 +2713,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 {
@@ -2769,6 +2776,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 {
                 DiagnosticMode::Pattern => {
@@ -2780,7 +2796,7 @@ fn show_candidates(
                     );
                     return true;
                 }
-                DiagnosticMode::Import => ("", ""),
+                DiagnosticMode::Import { .. } => ("", ""),
                 DiagnosticMode::Normal => ("use ", ";\n"),
             };
             for candidate in &mut accessible_path_strings {
@@ -2797,13 +2813,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 {
+                DiagnosticMode::Import { single_nested: 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).
@@ -2818,17 +2843,13 @@ 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, DiagnosticMode::Import)) {
+    } else if !(inaccessible_path_strings.is_empty()
+        || matches!(mode, DiagnosticMode::Import { .. }))
+    {
         let prefix = if let DiagnosticMode::Pattern = mode {
             "you might have meant to match on "
         } else {
diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs
@@ -276,7 +276,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         mut ident: Ident,
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ribs: &[Rib<'a>],
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Option<LexicalScopeBinding<'a>> {
@@ -370,7 +370,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         orig_ident: Ident,
         scope_set: ScopeSet<'a>,
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         force: bool,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Result<NameBinding<'a>, Determinacy> {
@@ -711,7 +711,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ident: Ident,
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Result<NameBinding<'a>, Determinacy> {
         self.resolve_ident_in_module_ext(module, ident, ns, parent_scope, finalize, ignore_binding)
@@ -725,7 +725,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         mut ident: Ident,
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Result<NameBinding<'a>, (Determinacy, Weak)> {
         let tmp_parent_scope;
@@ -763,7 +763,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ident: Ident,
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Result<NameBinding<'a>, Determinacy> {
         self.resolve_ident_in_module_unadjusted_ext(
@@ -788,7 +788,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         ns: Namespace,
         parent_scope: &ParentScope<'a>,
         restricted_shadowing: bool,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         // This binding should be ignored during in-module resolution, so that we don't get
         // "self-confirming" import resolutions during import validation and checking.
         ignore_binding: Option<NameBinding<'a>>,
@@ -857,7 +857,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, .. }) = finalize {
+        if let Some(Finalize { path_span, report_private, imported, .. }) = finalize {
             let Some(binding) = binding else {
                 return Err((Determined, Weak::No));
             };
@@ -870,6 +870,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         dedup_span: path_span,
                         outermost_res: None,
                         parent_scope: *parent_scope,
+                        imported,
                     });
                 } else {
                     return Err((Determined, Weak::No));
@@ -1332,7 +1333,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         path: &[Segment],
         opt_ns: Option<Namespace>, // `None` indicates a module path in import
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> PathResult<'a> {
         self.resolve_path_with_ribs(path, opt_ns, parent_scope, finalize, None, ignore_binding)
@@ -1343,7 +1344,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         path: &[Segment],
         opt_ns: Option<Namespace>, // `None` indicates a module path in import
         parent_scope: &ParentScope<'a>,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ribs: Option<&PerNS<Vec<Rib<'a>>>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> PathResult<'a> {
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -716,7 +716,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         &mut diag,
                         Some(err.span),
                         candidates,
-                        DiagnosticMode::Import,
+                        DiagnosticMode::Import { single_nested: false },
                         (source != target)
                             .then(|| format!(" as {target}"))
                             .as_deref()
@@ -857,7 +857,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             _ => None,
         };
         let prev_ambiguity_errors_len = self.ambiguity_errors.len();
-        let finalize = Finalize::with_root_span(import.root_id, import.span, import.root_span);
+        let finalize = Finalize::with_imported(import);
 
         // We'll provide more context to the privacy errors later, up to `len`.
         let privacy_errors_len = self.privacy_errors.len();
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -1324,7 +1324,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         &mut self,
         ident: Ident,
         ns: Namespace,
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
         ignore_binding: Option<NameBinding<'a>>,
     ) -> Option<LexicalScopeBinding<'a>> {
         self.r.resolve_ident_in_lexical_scope(
@@ -1341,7 +1341,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         &mut self,
         path: &[Segment],
         opt_ns: Option<Namespace>, // `None` indicates a module path in import
-        finalize: Option<Finalize>,
+        finalize: Option<Finalize<'a>>,
     ) -> PathResult<'a> {
         self.r.resolve_path_with_ribs(
             path,
@@ -3725,7 +3725,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         qself: &Option<P<QSelf>>,
         path: &[Segment],
         source: PathSource<'ast>,
-        finalize: Finalize,
+        finalize: Finalize<'a>,
         record_partial_res: RecordPartialRes,
     ) -> PartialRes {
         let ns = source.namespace();
@@ -3982,7 +3982,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         primary_ns: Namespace,
         span: Span,
         defer_to_typeck: bool,
-        finalize: Finalize,
+        finalize: Finalize<'a>,
     ) -> Result<Option<PartialRes>, Spanned<ResolutionError<'a>>> {
         let mut fin_res = None;
 
@@ -4024,7 +4024,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         qself: &Option<P<QSelf>>,
         path: &[Segment],
         ns: Namespace,
-        finalize: Finalize,
+        finalize: Finalize<'a>,
     ) -> Result<Option<PartialRes>, Spanned<ResolutionError<'a>>> {
         debug!(
             "resolve_qpath(qself={:?}, path={:?}, ns={:?}, finalize={:?})",
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -712,6 +712,7 @@ struct PrivacyError<'a> {
     dedup_span: Span,
     outermost_res: Option<(Res, Ident)>,
     parent_scope: ParentScope<'a>,
+    imported: Option<Import<'a>>,
 }
 
 #[derive(Debug)]
@@ -2164,7 +2165,7 @@ fn module_to_string(module: Module<'_>) -> Option<String> {
 }
 
 #[derive(Copy, Clone, Debug)]
-struct Finalize {
+struct Finalize<'a> {
     /// Node ID for linting.
     node_id: NodeId,
     /// Span of the whole path or some its characteristic fragment.
@@ -2176,15 +2177,26 @@ struct Finalize {
     /// Whether to report privacy errors or silently return "no resolution" for them,
     /// similarly to speculative resolution.
     report_private: bool,
+    imported: Option<Import<'a>>,
 }
 
-impl Finalize {
-    fn new(node_id: NodeId, path_span: Span) -> Finalize {
+impl<'a> Finalize<'a> {
+    fn new(node_id: NodeId, path_span: Span) -> Finalize<'a> {
         Finalize::with_root_span(node_id, path_span, path_span)
     }
 
-    fn with_root_span(node_id: NodeId, path_span: Span, root_span: Span) -> Finalize {
-        Finalize { node_id, path_span, root_span, report_private: true }
+    fn with_root_span(node_id: NodeId, path_span: Span, root_span: Span) -> Finalize<'a> {
+        Finalize { node_id, path_span, root_span, report_private: true, imported: None }
+    }
+
+    fn with_imported(imported: Import<'a>) -> Finalize<'a> {
+        Finalize {
+            node_id: imported.root_id,
+            path_span: imported.span,
+            root_span: imported.root_span,
+            report_private: true,
+            imported: Some(imported),
+        }
     }
 }
 
diff --git a/tests/ui/imports/issue-114884.rs b/tests/ui/imports/issue-114884.rs
@@ -0,0 +1,14 @@
+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/issue-114884.stderr b/tests/ui/imports/issue-114884.stderr
@@ -0,0 +1,21 @@
+error[E0603]: module `sub_mod` is private
+  --> $DIR/issue-114884.rs:11:12
+   |
+LL | use mod2::{sub_mod::TraitA};
+   |            ^^^^^^^ private module
+   |
+help: consider importing this trait instead:
+      mod1::TraitA
+  --> $DIR/issue-114884.rs:11:12
+   |
+LL | use mod2::{sub_mod::TraitA};
+   |            ^^^^^^^^^^^^^^^
+note: the module `sub_mod` is defined here
+  --> $DIR/issue-114884.rs:6:5
+   |
+LL |     mod sub_mod {
+   |     ^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0603`.
