diagnostics: account for glob shadowing when linting redundant imports

Co-Authored-By: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
This commit is contained in:
Michael Howell 2023-03-25 09:28:28 -07:00
parent 2005e300c0
commit 000e94e67d
8 changed files with 142 additions and 14 deletions

View File

@ -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));
};
@ -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);
}
@ -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));
};
@ -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 {

View File

@ -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() {}

View File

@ -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() {}

View File

@ -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

View File

@ -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() {}

View File

@ -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

View File

@ -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() {}

View File

@ -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() {}