From 564435f20a740d9d95180f3a053aec36feed765d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 27 Dec 2022 04:28:02 +0000 Subject: [PATCH 1/5] Suppress suggestions for nested use tree --- compiler/rustc_resolve/src/imports.rs | 44 ++++++++++++------- src/test/ui/imports/bad-import-in-nested.rs | 14 ++++++ .../ui/imports/bad-import-in-nested.stderr | 9 ++++ 3 files changed, 50 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/imports/bad-import-in-nested.rs create mode 100644 src/test/ui/imports/bad-import-in-nested.stderr diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 4d896b05526..f9121ae046a 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -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::>(), - &import.kind, - err.span, - ); - errors.push((path, err)); + errors.push((import, err)); prev_root_id = import.root_id; } } else if is_indeterminate { @@ -496,8 +491,10 @@ impl<'a, 'b> ImportResolver<'a, 'b> { suggestion: None, candidate: 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::>(); + let paths = errors + .iter() + .map(|(import, err)| { + let path = import_path_to_string( + &import.module_path.iter().map(|seg| seg.ident).collect::>(), + &import.kind, + err.span, + ); + format!("`{path}`") + }) + .collect::>(); 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); } @@ -539,13 +546,16 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } if let Some(candidate) = &err.candidate { - import_candidates( - self.r.session, - &self.r.untracked.source_span, - &mut diag, - Some(err.span), - &candidate, - ) + match &import.kind { + ImportKind::Single { nested: false, .. } => import_candidates( + self.r.session, + &self.r.untracked.source_span, + &mut diag, + Some(err.span), + &candidate, + ), + _ => {} + } } } diff --git a/src/test/ui/imports/bad-import-in-nested.rs b/src/test/ui/imports/bad-import-in-nested.rs new file mode 100644 index 00000000000..5ab0d3d60c9 --- /dev/null +++ b/src/test/ui/imports/bad-import-in-nested.rs @@ -0,0 +1,14 @@ +#![allow(unused)] + +mod A { + pub(crate) type AA = (); +} + +mod C {} + +mod B { + use crate::C::{self, AA}; + //~^ ERROR unresolved import `crate::C::AA` +} + +fn main() {} diff --git a/src/test/ui/imports/bad-import-in-nested.stderr b/src/test/ui/imports/bad-import-in-nested.stderr new file mode 100644 index 00000000000..a107048d579 --- /dev/null +++ b/src/test/ui/imports/bad-import-in-nested.stderr @@ -0,0 +1,9 @@ +error[E0432]: unresolved import `crate::C::AA` + --> $DIR/bad-import-in-nested.rs:10:26 + | +LL | use crate::C::{self, AA}; + | ^^ no `AA` in `C` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0432`. From 9e2536b9389f56386d7f722b403d9730911ee811 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 27 Dec 2022 05:09:33 +0000 Subject: [PATCH 2/5] Note alternative import candidates in nested use tree --- compiler/rustc_resolve/src/diagnostics.rs | 5 ++-- compiler/rustc_resolve/src/imports.rs | 29 +++++++++++++------ src/test/ui/imports/bad-import-in-nested.rs | 17 +++++++++-- .../ui/imports/bad-import-in-nested.stderr | 25 ++++++++++++++-- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 37771693417..2e28dad52c7 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2309,7 +2309,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 +2324,7 @@ pub(crate) fn import_candidates( // This is `None` if all placement locations are inside expansions use_placement_span: Option, candidates: &[ImportSuggestion], + mode: DiagnosticMode, ) { show_candidates( session, @@ -2333,7 +2334,7 @@ pub(crate) fn import_candidates( candidates, Instead::Yes, FoundUse::Yes, - DiagnosticMode::Import, + mode, vec![], ); } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index f9121ae046a..b3593fc9e47 100644 --- 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, note: Option, suggestion: Option, - candidate: Option>, + candidates: Option>, } pub struct ImportResolver<'a, 'b> { @@ -489,7 +489,7 @@ 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 `::` @@ -545,15 +545,26 @@ impl<'a, 'b> ImportResolver<'a, 'b> { diag.multipart_suggestion(&msg, suggestions, applicability); } - if let Some(candidate) = &err.candidate { + if let Some(candidates) = &err.candidates { match &import.kind { ImportKind::Single { nested: false, .. } => import_candidates( self.r.session, &self.r.untracked.source_span, &mut diag, Some(err.span), - &candidate, + &candidates, + DiagnosticMode::Import, ), + ImportKind::Single { nested: true, .. } => { + import_candidates( + self.r.session, + &self.r.untracked.source_span, + &mut diag, + None, + &candidates, + DiagnosticMode::Normal, + ); + } _ => {} } } @@ -717,14 +728,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); @@ -771,7 +782,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { )), note: None, suggestion: None, - candidate: None, + candidates: None, }); } } @@ -953,7 +964,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/imports/bad-import-in-nested.rs b/src/test/ui/imports/bad-import-in-nested.rs index 5ab0d3d60c9..2e95480ad41 100644 --- a/src/test/ui/imports/bad-import-in-nested.rs +++ b/src/test/ui/imports/bad-import-in-nested.rs @@ -1,14 +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 {} +mod C { + pub mod D {} +} mod B { use crate::C::{self, AA}; - //~^ ERROR unresolved import `crate::C::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 index a107048d579..855b1e637e9 100644 --- a/src/test/ui/imports/bad-import-in-nested.stderr +++ b/src/test/ui/imports/bad-import-in-nested.stderr @@ -1,9 +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:10:26 + --> $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: aborting due to previous error +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`. From d2404d6dca59d34d5b9f0c66b425b08229f20f4b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 27 Dec 2022 07:05:45 +0000 Subject: [PATCH 3/5] Dont clobber `as ..` rename in import suggestion --- compiler/rustc_resolve/src/diagnostics.rs | 9 ++++++- compiler/rustc_resolve/src/imports.rs | 6 +++-- src/test/ui/imports/bad-import-with-rename.rs | 16 ++++++++++++ .../ui/imports/bad-import-with-rename.stderr | 25 +++++++++++++++++++ .../inaccessible-test-modules.stderr | 4 +-- 5 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/imports/bad-import-with-rename.rs create mode 100644 src/test/ui/imports/bad-import-with-rename.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 2e28dad52c7..c8b96aae7a6 100644 --- 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, + None, ); err.emit(); } else if let Some((span, msg, sugg, appl)) = suggestion { @@ -690,6 +691,7 @@ impl<'a> Resolver<'a> { FoundUse::Yes, DiagnosticMode::Pattern, vec![], + None, ); } err @@ -1344,6 +1346,7 @@ impl<'a> Resolver<'a> { FoundUse::Yes, DiagnosticMode::Normal, vec![], + None, ); if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) { @@ -2325,6 +2328,7 @@ pub(crate) fn import_candidates( use_placement_span: Option, candidates: &[ImportSuggestion], mode: DiagnosticMode, + append: Option<&str>, ) { show_candidates( session, @@ -2336,6 +2340,7 @@ pub(crate) fn import_candidates( FoundUse::Yes, mode, vec![], + append, ); } @@ -2353,10 +2358,12 @@ fn show_candidates( found_use: FoundUse, mode: DiagnosticMode, path: Vec, + append: Option<&str>, ) { if candidates.is_empty() { return; } + let append = append.unwrap_or(""); let mut accessible_path_strings: Vec<(String, &str, Option, &Option)> = Vec::new(); @@ -2417,7 +2424,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 index b3593fc9e47..d99c1cb6d3c 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -547,15 +547,16 @@ impl<'a, 'b> ImportResolver<'a, 'b> { if let Some(candidates) = &err.candidates { match &import.kind { - ImportKind::Single { nested: false, .. } => import_candidates( + 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(), ), - ImportKind::Single { nested: true, .. } => { + ImportKind::Single { nested: true, source, target, .. } => { import_candidates( self.r.session, &self.r.untracked.source_span, @@ -563,6 +564,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { None, &candidates, DiagnosticMode::Normal, + (source != target).then(|| format!(" as {target}")).as_deref(), ); } _ => {} diff --git a/src/test/ui/imports/bad-import-with-rename.rs b/src/test/ui/imports/bad-import-with-rename.rs new file mode 100644 index 00000000000..ffe56916f92 --- /dev/null +++ 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 new file mode 100644 index 00000000000..cace2a7a51c --- /dev/null +++ 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 index 0c16ecd4c86..17ea648aad6 100644 --- a/src/test/ui/test-attrs/inaccessible-test-modules.stderr +++ b/src/test/ui/test-attrs/inaccessible-test-modules.stderr @@ -19,8 +19,8 @@ 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 From 050bc95ce2b23b034d5f41e5c3a8c6e627dfd52a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 27 Dec 2022 07:17:22 +0000 Subject: [PATCH 4/5] Fix some totally useless suggestions --- compiler/rustc_resolve/src/imports.rs | 2 +- .../ui/hygiene/extern-prelude-from-opaque-fail.stderr | 5 +---- src/test/ui/test-attrs/inaccessible-test-modules.stderr | 9 +-------- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index d99c1cb6d3c..0a2a737e1a4 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -896,7 +896,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() { diff --git a/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr b/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr index e89c19b5881..f1f4caee361 100644 --- 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/test-attrs/inaccessible-test-modules.stderr b/src/test/ui/test-attrs/inaccessible-test-modules.stderr index 17ea648aad6..a45c5bd4588 100644 --- a/src/test/ui/test-attrs/inaccessible-test-modules.stderr +++ b/src/test/ui/test-attrs/inaccessible-test-modules.stderr @@ -2,10 +2,7 @@ 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 @@ -13,10 +10,6 @@ error[E0432]: unresolved import `test` 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 as y; From 1d66a675bb61c21555dcb848ed7378b6f2848de7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 9 Jan 2023 18:07:34 +0000 Subject: [PATCH 5/5] review comment --- compiler/rustc_resolve/src/diagnostics.rs | 11 +++++------ compiler/rustc_resolve/src/imports.rs | 10 ++++++++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index c8b96aae7a6..7d62d67d64f 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -161,7 +161,7 @@ impl<'a> Resolver<'a> { found_use, DiagnosticMode::Normal, path, - None, + "", ); err.emit(); } else if let Some((span, msg, sugg, appl)) = suggestion { @@ -691,7 +691,7 @@ impl<'a> Resolver<'a> { FoundUse::Yes, DiagnosticMode::Pattern, vec![], - None, + "", ); } err @@ -1346,7 +1346,7 @@ impl<'a> Resolver<'a> { FoundUse::Yes, DiagnosticMode::Normal, vec![], - None, + "", ); if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) { @@ -2328,7 +2328,7 @@ pub(crate) fn import_candidates( use_placement_span: Option, candidates: &[ImportSuggestion], mode: DiagnosticMode, - append: Option<&str>, + append: &str, ) { show_candidates( session, @@ -2358,12 +2358,11 @@ fn show_candidates( found_use: FoundUse, mode: DiagnosticMode, path: Vec, - append: Option<&str>, + append: &str, ) { if candidates.is_empty() { return; } - let append = append.unwrap_or(""); let mut accessible_path_strings: Vec<(String, &str, Option, &Option)> = Vec::new(); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 0a2a737e1a4..00f65ac37b6 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -554,7 +554,10 @@ impl<'a, 'b> ImportResolver<'a, 'b> { Some(err.span), &candidates, DiagnosticMode::Import, - (source != target).then(|| format!(" as {target}")).as_deref(), + (source != target) + .then(|| format!(" as {target}")) + .as_deref() + .unwrap_or(""), ), ImportKind::Single { nested: true, source, target, .. } => { import_candidates( @@ -564,7 +567,10 @@ impl<'a, 'b> ImportResolver<'a, 'b> { None, &candidates, DiagnosticMode::Normal, - (source != target).then(|| format!(" as {target}")).as_deref(), + (source != target) + .then(|| format!(" as {target}")) + .as_deref() + .unwrap_or(""), ); } _ => {}