Rollup merge of #106175 - compiler-errors:bad-import-sugg, r=oli-obk

Yuki Okushi · web-flow · commit 684a3717cb78 · 2023-01-10T08:05:34.000+09:00
Fix bad import suggestion with nested `use` tree Fixes #105566 Fixes #105373 Ideally, we'd find some way to turn these into structured suggestions -- perhaps on a separate line as a different `use` statement, but I have no idea how to access the span for the whole `use` from this point in the import resolution code.
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
@@ -161,6 +161,7 @@ impl<'a> Resolver<'a> {
                     found_use,
                     DiagnosticMode::Normal,
                     path,
+                    "",
                 );
                 err.emit();
             } else if let Some((span, msg, sugg, appl)) = suggestion {
@@ -690,6 +691,7 @@ impl<'a> Resolver<'a> {
                         FoundUse::Yes,
                         DiagnosticMode::Pattern,
                         vec![],
+                        "",
                     );
                 }
                 err
@@ -1344,6 +1346,7 @@ impl<'a> Resolver<'a> {
             FoundUse::Yes,
             DiagnosticMode::Normal,
             vec![],
+            "",
         );
 
         if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
@@ -2309,7 +2312,7 @@ enum FoundUse {
 }
 
 /// Whether a binding is part of a pattern or a use statement. Used for diagnostics.
-enum DiagnosticMode {
+pub(crate) enum DiagnosticMode {
     Normal,
     /// The binding is part of a pattern
     Pattern,
@@ -2324,6 +2327,8 @@ pub(crate) fn import_candidates(
     // This is `None` if all placement locations are inside expansions
     use_placement_span: Option<Span>,
     candidates: &[ImportSuggestion],
+    mode: DiagnosticMode,
+    append: &str,
 ) {
     show_candidates(
         session,
@@ -2333,8 +2338,9 @@ pub(crate) fn import_candidates(
         candidates,
         Instead::Yes,
         FoundUse::Yes,
-        DiagnosticMode::Import,
+        mode,
         vec![],
+        append,
     );
 }
 
@@ -2352,6 +2358,7 @@ fn show_candidates(
     found_use: FoundUse,
     mode: DiagnosticMode,
     path: Vec<Segment>,
+    append: &str,
 ) {
     if candidates.is_empty() {
         return;
@@ -2416,7 +2423,7 @@ fn show_candidates(
                 // produce an additional newline to separate the new use statement
                 // from the directly following item.
                 let additional_newline = if let FoundUse::Yes = found_use { "" } else { "\n" };
-                candidate.0 = format!("{}{};\n{}", add_use, &candidate.0, additional_newline);
+                candidate.0 = format!("{add_use}{}{append};\n{additional_newline}", &candidate.0);
             }
 
             err.span_suggestions(
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -1,6 +1,6 @@
 //! A bunch of methods and structures more or less related to resolving imports.
 
-use crate::diagnostics::{import_candidates, Suggestion};
+use crate::diagnostics::{import_candidates, DiagnosticMode, Suggestion};
 use crate::Determinacy::{self, *};
 use crate::Namespace::*;
 use crate::{module_to_string, names_to_string, ImportSuggestion};
@@ -402,7 +402,7 @@ struct UnresolvedImportError {
     label: Option<String>,
     note: Option<String>,
     suggestion: Option<Suggestion>,
-    candidate: Option<Vec<ImportSuggestion>>,
+    candidates: Option<Vec<ImportSuggestion>>,
 }
 
 pub struct ImportResolver<'a, 'b> {
@@ -475,12 +475,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                     errors = vec![];
                 }
                 if seen_spans.insert(err.span) {
-                    let path = import_path_to_string(
-                        &import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
-                        &import.kind,
-                        err.span,
-                    );
-                    errors.push((path, err));
+                    errors.push((import, err));
                     prev_root_id = import.root_id;
                 }
             } else if is_indeterminate {
@@ -494,10 +489,12 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                     label: None,
                     note: None,
                     suggestion: None,
-                    candidate: None,
+                    candidates: None,
                 };
+                // FIXME: there should be a better way of doing this than
+                // formatting this as a string then checking for `::`
                 if path.contains("::") {
-                    errors.push((path, err))
+                    errors.push((import, err))
                 }
             }
         }
@@ -507,7 +504,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         }
     }
 
-    fn throw_unresolved_import_error(&self, errors: Vec<(String, UnresolvedImportError)>) {
+    fn throw_unresolved_import_error(&self, errors: Vec<(&Import<'_>, UnresolvedImportError)>) {
         if errors.is_empty() {
             return;
         }
@@ -516,7 +513,17 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         const MAX_LABEL_COUNT: usize = 10;
 
         let span = MultiSpan::from_spans(errors.iter().map(|(_, err)| err.span).collect());
-        let paths = errors.iter().map(|(path, _)| format!("`{}`", path)).collect::<Vec<_>>();
+        let paths = errors
+            .iter()
+            .map(|(import, err)| {
+                let path = import_path_to_string(
+                    &import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
+                    &import.kind,
+                    err.span,
+                );
+                format!("`{path}`")
+            })
+            .collect::<Vec<_>>();
         let msg = format!("unresolved import{} {}", pluralize!(paths.len()), paths.join(", "),);
 
         let mut diag = struct_span_err!(self.r.session, span, E0432, "{}", &msg);
@@ -525,7 +532,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
             diag.note(note);
         }
 
-        for (_, err) in errors.into_iter().take(MAX_LABEL_COUNT) {
+        for (import, err) in errors.into_iter().take(MAX_LABEL_COUNT) {
             if let Some(label) = err.label {
                 diag.span_label(err.span, label);
             }
@@ -538,14 +545,36 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                 diag.multipart_suggestion(&msg, suggestions, applicability);
             }
 
-            if let Some(candidate) = &err.candidate {
-                import_candidates(
-                    self.r.session,
-                    &self.r.untracked.source_span,
-                    &mut diag,
-                    Some(err.span),
-                    &candidate,
-                )
+            if let Some(candidates) = &err.candidates {
+                match &import.kind {
+                    ImportKind::Single { nested: false, source, target, .. } => import_candidates(
+                        self.r.session,
+                        &self.r.untracked.source_span,
+                        &mut diag,
+                        Some(err.span),
+                        &candidates,
+                        DiagnosticMode::Import,
+                        (source != target)
+                            .then(|| format!(" as {target}"))
+                            .as_deref()
+                            .unwrap_or(""),
+                    ),
+                    ImportKind::Single { nested: true, source, target, .. } => {
+                        import_candidates(
+                            self.r.session,
+                            &self.r.untracked.source_span,
+                            &mut diag,
+                            None,
+                            &candidates,
+                            DiagnosticMode::Normal,
+                            (source != target)
+                                .then(|| format!(" as {target}"))
+                                .as_deref()
+                                .unwrap_or(""),
+                        );
+                    }
+                    _ => {}
+                }
             }
         }
 
@@ -707,14 +736,14 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                                 String::from("a similar path exists"),
                                 Applicability::MaybeIncorrect,
                             )),
-                            candidate: None,
+                            candidates: None,
                         },
                         None => UnresolvedImportError {
                             span,
                             label: Some(label),
                             note: None,
                             suggestion,
-                            candidate: None,
+                            candidates: None,
                         },
                     };
                     return Some(err);
@@ -761,7 +790,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                                 )),
                                 note: None,
                                 suggestion: None,
-                                candidate: None,
+                                candidates: None,
                             });
                         }
                     }
@@ -873,7 +902,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                 let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
                 let names = resolutions
                     .filter_map(|(BindingKey { ident: i, .. }, resolution)| {
-                        if *i == ident {
+                        if i.name == ident.name {
                             return None;
                         } // Never suggest the same name
                         match *resolution.borrow() {
@@ -943,7 +972,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                     label: Some(label),
                     note,
                     suggestion,
-                    candidate: if !parent_suggestion.is_empty() {
+                    candidates: if !parent_suggestion.is_empty() {
                         Some(parent_suggestion)
                     } else {
                         None
diff --git a/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr b/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr
@@ -2,10 +2,7 @@ error[E0432]: unresolved import `my_core`
   --> $DIR/extern-prelude-from-opaque-fail.rs:20:9
    |
 LL |     use my_core;
-   |         ^^^^^^^
-   |         |
-   |         no `my_core` in the root
-   |         help: a similar name exists in the module: `my_core`
+   |         ^^^^^^^ no `my_core` in the root
 
 error[E0432]: unresolved import `my_core`
   --> $DIR/extern-prelude-from-opaque-fail.rs:7:13
diff --git a/src/test/ui/imports/bad-import-in-nested.rs b/src/test/ui/imports/bad-import-in-nested.rs
@@ -0,0 +1,27 @@
+// edition: 2021
+
+#![allow(unused)]
+
+mod A {
+    pub(crate) type AA = ();
+    pub(crate) type BB = ();
+
+    mod A2 {
+        use super::{super::C::D::AA, AA as _};
+        //~^ ERROR unresolved import
+    }
+}
+
+mod C {
+    pub mod D {}
+}
+
+mod B {
+    use crate::C::{self, AA};
+    //~^ ERROR unresolved import
+
+    use crate::{A, C::BB};
+    //~^ ERROR unresolved import
+}
+
+fn main() {}
diff --git a/src/test/ui/imports/bad-import-in-nested.stderr b/src/test/ui/imports/bad-import-in-nested.stderr
@@ -0,0 +1,30 @@
+error[E0432]: unresolved import `super::super::C::D::AA`
+  --> $DIR/bad-import-in-nested.rs:10:21
+   |
+LL |         use super::{super::C::D::AA, AA as _};
+   |                     ^^^^^^^^^^^^^^^ no `AA` in `C::D`
+   |
+   = note: consider importing this type alias instead:
+           crate::A::AA
+
+error[E0432]: unresolved import `crate::C::AA`
+  --> $DIR/bad-import-in-nested.rs:20:26
+   |
+LL |     use crate::C::{self, AA};
+   |                          ^^ no `AA` in `C`
+   |
+   = note: consider importing this type alias instead:
+           crate::A::AA
+
+error[E0432]: unresolved import `crate::C::BB`
+  --> $DIR/bad-import-in-nested.rs:23:20
+   |
+LL |     use crate::{A, C::BB};
+   |                    ^^^^^ no `BB` in `C`
+   |
+   = note: consider importing this type alias instead:
+           crate::A::BB
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0432`.
diff --git a/src/test/ui/imports/bad-import-with-rename.rs b/src/test/ui/imports/bad-import-with-rename.rs
@@ -0,0 +1,16 @@
+mod A {
+    pub type B = ();
+    pub type B2 = ();
+}
+
+mod C {
+    use crate::D::B as _;
+    //~^ ERROR unresolved import `crate::D::B`
+
+    use crate::D::B2;
+    //~^ ERROR unresolved import `crate::D::B2`
+}
+
+mod D {}
+
+fn main() {}
diff --git a/src/test/ui/imports/bad-import-with-rename.stderr b/src/test/ui/imports/bad-import-with-rename.stderr
@@ -0,0 +1,25 @@
+error[E0432]: unresolved import `crate::D::B`
+  --> $DIR/bad-import-with-rename.rs:7:9
+   |
+LL |     use crate::D::B as _;
+   |         ^^^^^^^^^^^^^^^^ no `B` in `D`
+   |
+help: consider importing this type alias instead
+   |
+LL |     use A::B as _;
+   |         ~~~~~~~~~~
+
+error[E0432]: unresolved import `crate::D::B2`
+  --> $DIR/bad-import-with-rename.rs:10:9
+   |
+LL |     use crate::D::B2;
+   |         ^^^^^^^^^^^^ no `B2` in `D`
+   |
+help: consider importing this type alias instead
+   |
+LL |     use A::B2;
+   |         ~~~~~~
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0432`.
diff --git a/src/test/ui/test-attrs/inaccessible-test-modules.stderr b/src/test/ui/test-attrs/inaccessible-test-modules.stderr
@@ -2,25 +2,18 @@ error[E0432]: unresolved import `main`
   --> $DIR/inaccessible-test-modules.rs:5:5
    |
 LL | use main as x;
-   |     ----^^^^^
-   |     |
-   |     no `main` in the root
-   |     help: a similar name exists in the module: `main`
+   |     ^^^^^^^^^ no `main` in the root
 
 error[E0432]: unresolved import `test`
   --> $DIR/inaccessible-test-modules.rs:6:5
    |
 LL | use test as y;
    |     ^^^^^^^^^ no `test` in the root
    |
-help: a similar name exists in the module
-   |
-LL | use test as y;
-   |     ~~~~
 help: consider importing this module instead
    |
-LL | use test::test;
-   |     ~~~~~~~~~~~
+LL | use test::test as y;
+   |     ~~~~~~~~~~~~~~~~
 
 error: aborting due to 2 previous errors
 
