Skip to content

diagnostics: account for glob shadowing when linting redundant imports #109599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,17 +869,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
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));
};
Expand Down Expand Up @@ -927,15 +929,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

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);
}
Expand All @@ -952,6 +951,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
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));
};
Expand Down Expand Up @@ -989,7 +996,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// 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 {
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/lint/use-redundant/issue-92904.rs
Original file line number Diff line number Diff line change
@@ -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() {}
16 changes: 16 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-glob-parent.rs
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-glob-parent.stderr
Original file line number Diff line number Diff line change
@@ -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

15 changes: 15 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-glob.rs
Original file line number Diff line number Diff line change
@@ -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() {}
16 changes: 16 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-glob.stderr
Original file line number Diff line number Diff line change
@@ -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

21 changes: 21 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-multiple-namespaces.rs
Original file line number Diff line number Diff line change
@@ -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() {}
19 changes: 19 additions & 0 deletions tests/ui/lint/use-redundant/use-redundant-not-parent.rs
Original file line number Diff line number Diff line change
@@ -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() {}