Fix relative paths in private import suggestions

compiler/rustc_resolve/src/error_helper.rs

@@ -2237,18 +2237,27 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         Some(path)
     }
 
-    /// Shortens a candidate import path to use `super::` (up to 1 level) or `self::` (same module)
-    /// relative to the current scope, if possible. Only applies to crate-local items and
-    /// only when the resulting path is actually shorter than the original.
     fn shorten_candidate_path(
         &self,
         suggestion: &mut ImportSuggestion,
         current_module: Module<'ra>,
+    ) {
+        self.shorten_import_path(suggestion.did, &mut suggestion.path, current_module);
+    }
+
+    /// Shortens an import path to use `super::` (up to 1 level) or `self::` (same module)
+    /// relative to the current scope, if possible. Only applies to crate-local items and
+    /// only when the resulting path is actually shorter than the original.
+    fn shorten_import_path(
+        &self,
+        did: Option<DefId>,
+        path: &mut Path,
+        current_module: Module<'ra>,
     ) {
         const MAX_SUPER_PATH_ITEMS_IN_SUGGESTION: usize = 1;
 
         // Only shorten local items.
-        if suggestion.did.is_none_or(|did| !did.is_local()) {
+        if did.is_none_or(|did| !did.is_local()) {
             return;
         }
 
@@ -2261,12 +2270,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // doesn't start with `Crate`, prepend it (edition 2015 paths are relative
         // to the crate root without an explicit `crate::` prefix).
         let candidate_names = {
-            let filtered_segments: Vec<_> = suggestion
-                .path
-                .segments
-                .iter()
-                .filter(|segment| segment.ident.name != kw::PathRoot)
-                .collect();
+            let filtered_segments: Vec<_> =
+                path.segments.iter().filter(|segment| segment.ident.name != kw::PathRoot).collect();
 
             let mut candidate_names: Vec<Symbol> =
                 filtered_segments.iter().map(|segment| segment.ident.name).collect();
@@ -2315,11 +2320,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
 
         // Only apply if the result is strictly shorter than the original path.
-        if new_segments.len() >= suggestion.path.segments.len() {
+        if new_segments.len() >= path.segments.len() {
             return;
         }
 
-        suggestion.path = Path { span: suggestion.path.span, segments: new_segments, tokens: None };
+        *path = Path { span: path.span, segments: new_segments, tokens: None };
     }
 
     fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
@@ -2472,17 +2477,81 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
             match binding.kind {
                 DeclKind::Import { source_decl, import, .. } => {
-                    // Don't include `{{root}}` in suggestions - it's an internal symbol
-                    // that should never be shown to users.
-                    let path = import
-                        .module_path
-                        .iter()
-                        .filter(|seg| seg.ident.name != kw::PathRoot)
-                        .map(|seg| seg.ident.clone())
-                        .chain(std::iter::once(ident))
-                        .collect::<Vec<_>>();
                     let through_reexport = !matches!(source_decl.kind, DeclKind::Def(_));
-                    sugg_paths.push((path, through_reexport));
+                    let uses_relative_path = import
+                        .module_path
+                        .first()
+                        .is_some_and(|seg| matches!(seg.ident.name, kw::SelfLower | kw::Super));
+                    let res_def_id = res.opt_def_id();
+                    let path = if uses_relative_path {
+                        // A path recovered from `self`/`super` is only useful if both the
+                        // target and every module segment can be named from the failing use site.
+                        let module_path = if let Some(ModuleOrUniformRoot::Module(module)) =
+                            import.imported_module.get()
+                            && module.is_local()
+                            && let Some(module_path) = self.module_path_names(module)
+                            && let Some(mut def_id) = module.opt_def_id()
+                            && res_def_id.is_none_or(|def_id| {
+                                self.is_accessible_from(
+                                    self.tcx.visibility(def_id),
+                                    parent_scope.module,
+                                )
+                            }) {
+                            // `module_path_names` tells us the resolved module's canonical path.
+                            // Before suggesting that path from the failing use site, make sure
+                            // every segment in it can actually be named from there.
+                            let mut visible_from_use_site = true;
+                            while let Some(parent) = self.tcx.opt_parent(def_id) {
+                                if !self.is_accessible_from(
+                                    self.tcx.visibility(def_id),
+                                    parent_scope.module,
+                                ) {
+                                    visible_from_use_site = false;
+                                    break;
+                                }
+                                if parent.is_top_level_module() {
+                                    break;
+                                }
+                                def_id = parent;
+                            }
+                            if visible_from_use_site { Some(module_path) } else { None }
+                        } else {
+                            None
+                        };
+
+                        module_path.map(|module_path| {
+                            // `import.module_path` is relative to the import's module, not to the
+                            // failing use site.
+                            let mut path = Path {
+                                span: ident.span,
+                                segments: module_path
+                                    .into_iter()
+                                    .chain(std::iter::once(ident.name))
+                                    .map(|name| {
+                                        ast::PathSegment::from_ident(Ident::with_dummy_span(name))
+                                    })
+                                    .collect(),
+                                tokens: None,
+                            };
+                            self.shorten_import_path(res_def_id, &mut path, parent_scope.module);
+                            path.segments.iter().map(|seg| seg.ident).collect()
+                        })
+                    } else {
+                        // Don't include `{{root}}` in suggestions - it's an internal symbol
+                        // that should never be shown to users.
+                        Some(
+                            import
+                                .module_path
+                                .iter()
+                                .filter(|seg| seg.ident.name != kw::PathRoot)
+                                .map(|seg| seg.ident.clone())
+                                .chain(std::iter::once(ident))
+                                .collect::<Vec<_>>(),
+                        )
+                    };
+                    if let Some(path) = path {
+                        sugg_paths.push((path, through_reexport));
+                    }
                 }
                 DeclKind::Def(_) => {}
             }

tests/ui/imports/private-import-suggestion-path-156244.edition_2015.stderr

@@ -76,6 +76,44 @@ LL -     use crate::rename::inner::Item as Item1;
 LL +     use outer::actual::Item as Item1;
    |
 
-error: aborting due to 4 previous errors
+error[E0603]: struct import `Hi` is private
+  --> $DIR/private-import-suggestion-path-156244.rs:55:37
+   |
+LL | use inaccessible_ancestor::testing::Hi;
+   |                                     ^^ private struct import
+   |
+note: the struct import `Hi` is defined here...
+  --> $DIR/private-import-suggestion-path-156244.rs:51:13
+   |
+LL |         use super::private::public::Hi;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: ...and refers to the struct `Hi` which is defined here
+  --> $DIR/private-import-suggestion-path-156244.rs:46:13
+   |
+LL |             pub struct Hi;
+   |             ^^^^^^^^^^^^^^ you could import this directly
+
+error[E0603]: module import `mem` is private
+  --> $DIR/private-import-suggestion-path-156244.rs:65:21
+   |
+LL | use external_alias::mem;
+   |                     ^^^ private module import
+   |
+note: the module import `mem` is defined here...
+  --> $DIR/private-import-suggestion-path-156244.rs:62:9
+   |
+LL |     use super::s::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` directly
+   |
+LL - use external_alias::mem;
+LL + use core::mem;
+   |
+
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0603`.

tests/ui/imports/private-import-suggestion-path-156244.edition_2018.stderr

@@ -76,6 +76,44 @@ LL -     use crate::rename::inner::Item as Item1;
 LL +     use crate::outer::actual::Item as Item1;
    |
 
-error: aborting due to 4 previous errors
+error[E0603]: struct import `Hi` is private
+  --> $DIR/private-import-suggestion-path-156244.rs:55:37
+   |
+LL | use inaccessible_ancestor::testing::Hi;
+   |                                     ^^ private struct import
+   |
+note: the struct import `Hi` is defined here...
+  --> $DIR/private-import-suggestion-path-156244.rs:51:13
+   |
+LL |         use super::private::public::Hi;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: ...and refers to the struct `Hi` which is defined here
+  --> $DIR/private-import-suggestion-path-156244.rs:46:13
+   |
+LL |             pub struct Hi;
+   |             ^^^^^^^^^^^^^^ you could import this directly
+
+error[E0603]: module import `mem` is private
+  --> $DIR/private-import-suggestion-path-156244.rs:65:21
+   |
+LL | use external_alias::mem;
+   |                     ^^^ private module import
+   |
+note: the module import `mem` is defined here...
+  --> $DIR/private-import-suggestion-path-156244.rs:62:9
+   |
+LL |     use super::s::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` directly
+   |
+LL - use external_alias::mem;
+LL + use core::mem;
+   |
+
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0603`.

tests/ui/imports/private-import-suggestion-path-156244.rs

@@ -39,4 +39,30 @@ mod bad {
     //~^ ERROR module import `inner` is private [E0603]
 }
 
+// Regression test for https://github.com/rust-lang/rust/issues/157455: no private ancestors.
+mod inaccessible_ancestor {
+    mod private {
+        pub mod public {
+            pub struct Hi;
+        }
+    }
+
+    pub mod testing {
+        use super::private::public::Hi;
+    }
+}
+
+use inaccessible_ancestor::testing::Hi;
+//~^ ERROR struct import `Hi` is private [E0603]
+
+// Regression test for https://github.com/rust-lang/rust/issues/157455: no external alias rewrite.
+use std as s;
+
+mod external_alias {
+    use super::s::mem;
+}
+
+use external_alias::mem;
+//~^ ERROR module import `mem` is private [E0603]
+
 fn main() {}

tests/ui/imports/private-import-suggestion-relative.fixed

@@ -0,0 +1,17 @@
+//@ run-rustfix
+
+#![allow(unused)]
+
+// Regression test for https://github.com/rust-lang/rust/issues/157455.
+mod public {
+    pub struct Hi;
+}
+
+mod testing {
+    use super::public::Hi;
+}
+
+use public::Hi;
+//~^ ERROR struct import `Hi` is private [E0603]
+
+fn main() {}

tests/ui/imports/private-import-suggestion-relative.rs

@@ -0,0 +1,17 @@
+//@ run-rustfix
+
+#![allow(unused)]
+
+// Regression test for https://github.com/rust-lang/rust/issues/157455.
+mod public {
+    pub struct Hi;
+}
+
+mod testing {
+    use super::public::Hi;
+}
+
+use testing::Hi;
+//~^ ERROR struct import `Hi` is private [E0603]
+
+fn main() {}

tests/ui/imports/private-import-suggestion-relative.stderr

@@ -0,0 +1,25 @@
+error[E0603]: struct import `Hi` is private
+  --> $DIR/private-import-suggestion-relative.rs:14:14
+   |
+LL | use testing::Hi;
+   |              ^^ private struct import
+   |
+note: the struct import `Hi` is defined here...
+  --> $DIR/private-import-suggestion-relative.rs:11:9
+   |
+LL |     use super::public::Hi;
+   |         ^^^^^^^^^^^^^^^^^
+note: ...and refers to the struct `Hi` which is defined here
+  --> $DIR/private-import-suggestion-relative.rs:7:5
+   |
+LL |     pub struct Hi;
+   |     ^^^^^^^^^^^^^^ you could import this directly
+help: import `Hi` directly
+   |
+LL - use testing::Hi;
+LL + use public::Hi;
+   |
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0603`.