From 2005e300c0266a6527dbfcb573a86c65d043f72f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 25 Mar 2023 09:23:05 -0700 Subject: [PATCH 1/2] tests: make directory for use redundant lint --- tests/ui/lint/{ => use-redundant}/use-redundant.rs | 0 tests/ui/lint/{ => use-redundant}/use-redundant.stderr | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/ui/lint/{ => use-redundant}/use-redundant.rs (100%) rename tests/ui/lint/{ => use-redundant}/use-redundant.stderr (100%) diff --git a/tests/ui/lint/use-redundant.rs b/tests/ui/lint/use-redundant/use-redundant.rs similarity index 100% rename from tests/ui/lint/use-redundant.rs rename to tests/ui/lint/use-redundant/use-redundant.rs diff --git a/tests/ui/lint/use-redundant.stderr b/tests/ui/lint/use-redundant/use-redundant.stderr similarity index 100% rename from tests/ui/lint/use-redundant.stderr rename to tests/ui/lint/use-redundant/use-redundant.stderr From 000e94e67d2b439c9dc3644c124ae4c405624a14 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 25 Mar 2023 09:28:28 -0700 Subject: [PATCH 2/2] diagnostics: account for glob shadowing when linting redundant imports Co-Authored-By: Vadim Petrochenkov --- compiler/rustc_resolve/src/ident.rs | 35 +++++++++++-------- tests/ui/lint/use-redundant/issue-92904.rs | 17 +++++++++ .../use-redundant-glob-parent.rs | 16 +++++++++ .../use-redundant-glob-parent.stderr | 17 +++++++++ .../lint/use-redundant/use-redundant-glob.rs | 15 ++++++++ .../use-redundant/use-redundant-glob.stderr | 16 +++++++++ .../use-redundant-multiple-namespaces.rs | 21 +++++++++++ .../use-redundant/use-redundant-not-parent.rs | 19 ++++++++++ 8 files changed, 142 insertions(+), 14 deletions(-) create mode 100644 tests/ui/lint/use-redundant/issue-92904.rs create mode 100644 tests/ui/lint/use-redundant/use-redundant-glob-parent.rs create mode 100644 tests/ui/lint/use-redundant/use-redundant-glob-parent.stderr create mode 100644 tests/ui/lint/use-redundant/use-redundant-glob.rs create mode 100644 tests/ui/lint/use-redundant/use-redundant-glob.stderr create mode 100644 tests/ui/lint/use-redundant/use-redundant-multiple-namespaces.rs create mode 100644 tests/ui/lint/use-redundant/use-redundant-not-parent.rs diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 52f0b65fad6..5e1d5d8a8fb 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -869,17 +869,19 @@ fn resolve_ident_in_module_unadjusted_ext( let resolution = self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports. - if let Some(Finalize { path_span, report_private, .. }) = finalize { - // If the primary binding is unusable, search further and return the shadowed glob - // binding if it exists. What we really want here is having two separate scopes in - // a module - one for non-globs and one for globs, but until that's done use this - // hack to avoid inconsistent resolution ICEs during import validation. - let binding = [resolution.binding, resolution.shadowed_glob].into_iter().find_map( - |binding| match (binding, ignore_binding) { + // If the primary binding is unusable, search further and return the shadowed glob + // binding if it exists. What we really want here is having two separate scopes in + // a module - one for non-globs and one for globs, but until that's done use this + // hack to avoid inconsistent resolution ICEs during import validation. + let binding = + [resolution.binding, resolution.shadowed_glob].into_iter().find_map(|binding| { + match (binding, ignore_binding) { (Some(binding), Some(ignored)) if ptr::eq(binding, ignored) => None, _ => binding, - }, - ); + } + }); + + if let Some(Finalize { path_span, report_private, .. }) = finalize { let Some(binding) = binding else { return Err((Determined, Weak::No)); }; @@ -927,15 +929,12 @@ fn resolve_ident_in_module_unadjusted_ext( } let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| { - if let Some(ignored) = ignore_binding && ptr::eq(binding, ignored) { - return Err((Determined, Weak::No)); - } let usable = this.is_accessible_from(binding.vis, parent_scope.module); if usable { Ok(binding) } else { Err((Determined, Weak::No)) } }; // Items and single imports are not shadowable, if we have one, then it's determined. - if let Some(binding) = resolution.binding { + if let Some(binding) = binding { if !binding.is_glob_import() { return check_usable(self, binding); } @@ -952,6 +951,14 @@ fn resolve_ident_in_module_unadjusted_ext( if !self.is_accessible_from(import_vis, parent_scope.module) { continue; } + if let Some(ignored) = ignore_binding && + let NameBindingKind::Import { import, .. } = ignored.kind && + ptr::eq(import, &**single_import) { + // Ignore not just the binding itself, but if it has a shadowed_glob, + // ignore that, too, because this loop is supposed to only process + // named imports. + continue; + } let Some(module) = single_import.imported_module.get() else { return Err((Undetermined, Weak::No)); }; @@ -989,7 +996,7 @@ fn resolve_ident_in_module_unadjusted_ext( // and prohibit access to macro-expanded `macro_export` macros instead (unless restricted // shadowing is enabled, see `macro_expanded_macro_export_errors`). let unexpanded_macros = !module.unexpanded_invocations.borrow().is_empty(); - if let Some(binding) = resolution.binding { + if let Some(binding) = binding { if !unexpanded_macros || ns == MacroNS || restricted_shadowing { return check_usable(self, binding); } else { diff --git a/tests/ui/lint/use-redundant/issue-92904.rs b/tests/ui/lint/use-redundant/issue-92904.rs new file mode 100644 index 00000000000..511d9d263cf --- /dev/null +++ b/tests/ui/lint/use-redundant/issue-92904.rs @@ -0,0 +1,17 @@ +// check-pass + +pub struct Foo(bar::Bar); + +pub mod bar { + pub struct Foo(pub Bar); + pub struct Bar(pub char); +} + +pub fn warning() -> Foo { + use bar::*; + #[deny(unused_imports)] + use self::Foo; // no error + Foo(Bar('a')) +} + +fn main() {} diff --git a/tests/ui/lint/use-redundant/use-redundant-glob-parent.rs b/tests/ui/lint/use-redundant/use-redundant-glob-parent.rs new file mode 100644 index 00000000000..6b1e018d2dc --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-glob-parent.rs @@ -0,0 +1,16 @@ +// check-pass +#![warn(unused_imports)] + +pub mod bar { + pub struct Foo(pub Bar); + pub struct Bar(pub char); +} + +use bar::*; + +pub fn warning() -> Foo { + use bar::Foo; //~ WARNING imported redundantly + Foo(Bar('a')) +} + +fn main() {} diff --git a/tests/ui/lint/use-redundant/use-redundant-glob-parent.stderr b/tests/ui/lint/use-redundant/use-redundant-glob-parent.stderr new file mode 100644 index 00000000000..2c3b3345270 --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-glob-parent.stderr @@ -0,0 +1,17 @@ +warning: the item `Foo` is imported redundantly + --> $DIR/use-redundant-glob-parent.rs:12:9 + | +LL | use bar::*; + | ------ the item `Foo` is already imported here +... +LL | use bar::Foo; + | ^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/use-redundant-glob-parent.rs:2:9 + | +LL | #![warn(unused_imports)] + | ^^^^^^^^^^^^^^ + +warning: 1 warning emitted + diff --git a/tests/ui/lint/use-redundant/use-redundant-glob.rs b/tests/ui/lint/use-redundant/use-redundant-glob.rs new file mode 100644 index 00000000000..bd9e51b6f59 --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-glob.rs @@ -0,0 +1,15 @@ +// check-pass +#![warn(unused_imports)] + +pub mod bar { + pub struct Foo(pub Bar); + pub struct Bar(pub char); +} + +pub fn warning() -> bar::Foo { + use bar::*; + use bar::Foo; //~ WARNING imported redundantly + Foo(Bar('a')) +} + +fn main() {} diff --git a/tests/ui/lint/use-redundant/use-redundant-glob.stderr b/tests/ui/lint/use-redundant/use-redundant-glob.stderr new file mode 100644 index 00000000000..d3b406d82b6 --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-glob.stderr @@ -0,0 +1,16 @@ +warning: the item `Foo` is imported redundantly + --> $DIR/use-redundant-glob.rs:11:9 + | +LL | use bar::*; + | ------ the item `Foo` is already imported here +LL | use bar::Foo; + | ^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/use-redundant-glob.rs:2:9 + | +LL | #![warn(unused_imports)] + | ^^^^^^^^^^^^^^ + +warning: 1 warning emitted + diff --git a/tests/ui/lint/use-redundant/use-redundant-multiple-namespaces.rs b/tests/ui/lint/use-redundant/use-redundant-multiple-namespaces.rs new file mode 100644 index 00000000000..0fb60840f8a --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-multiple-namespaces.rs @@ -0,0 +1,21 @@ +// check-pass +#![allow(nonstandard_style)] + +pub mod bar { + pub struct Foo { pub bar: Bar } + pub struct Bar(pub char); +} + +pub mod x { + use crate::bar; + pub const Foo: bar::Bar = bar::Bar('a'); +} + +pub fn warning() -> bar::Foo { + #![deny(unused_imports)] // no error + use bar::*; + use x::Foo; + Foo { bar: Foo } +} + +fn main() {} diff --git a/tests/ui/lint/use-redundant/use-redundant-not-parent.rs b/tests/ui/lint/use-redundant/use-redundant-not-parent.rs new file mode 100644 index 00000000000..c97a3d34163 --- /dev/null +++ b/tests/ui/lint/use-redundant/use-redundant-not-parent.rs @@ -0,0 +1,19 @@ +// check-pass + +pub mod bar { + pub struct Foo(pub Bar); + pub struct Bar(pub char); +} + +pub mod x { + pub struct Foo(pub crate::bar::Bar); +} + +pub fn warning() -> x::Foo { + use bar::*; + #[deny(unused_imports)] + use x::Foo; // no error + Foo(Bar('a')) +} + +fn main() {}