From 29d68edc6e1d4f5e6d1599202e3af38080840977 Mon Sep 17 00:00:00 2001 From: Fabian Drinck Date: Fri, 22 Feb 2019 15:07:18 +0100 Subject: [PATCH] Add lint for redundant imports Co-authored-by: Stephan Schauerte --- src/librustc/lint/builtin.rs | 6 ++ src/librustc_resolve/resolve_imports.rs | 93 ++++++++++++++++++++++++- src/test/ui/lint/use-redundant.rs | 17 +++++ src/test/ui/lint/use-redundant.stderr | 14 ++++ 4 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/lint/use-redundant.rs create mode 100644 src/test/ui/lint/use-redundant.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 10a5c1479fa..615bc0af37a 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -392,6 +392,12 @@ declare_lint! { "nested occurrence of `impl Trait` type" } +declare_lint! { + pub REDUNDANT_IMPORT, + Warn, + "redundant import" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. #[derive(Copy, Clone)] diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index c2d2bd753c8..b40b68b0853 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -7,6 +7,7 @@ use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyErro use crate::{Resolver, Segment}; use crate::{names_to_string, module_to_string}; use crate::{resolve_error, ResolutionError, Suggestion}; +use crate::ModuleKind; use crate::macros::ParentScope; use errors::Applicability; @@ -14,7 +15,11 @@ use errors::Applicability; use rustc_data_structures::ptr_key::PtrKey; use rustc::ty; use rustc::lint::builtin::BuiltinLintDiagnostics; -use rustc::lint::builtin::{DUPLICATE_MACRO_EXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE}; +use rustc::lint::builtin::{ + DUPLICATE_MACRO_EXPORTS, + PUB_USE_OF_PRIVATE_EXTERN_CRATE, + REDUNDANT_IMPORT, +}; use rustc::hir::def_id::{CrateNum, DefId}; use rustc::hir::def::*; use rustc::session::DiagnosticMessageId; @@ -1227,10 +1232,96 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { import[ns] = Some(PathResolution::new(def)); }); + self.check_for_redundant_imports( + ident, + directive, + source_bindings, + target_bindings, + target, + ); + debug!("(resolving single import) successfully resolved import"); None } + fn check_for_redundant_imports( + &mut self, + ident: Ident, + directive: &'b ImportDirective<'b>, + source_bindings: &PerNS, Determinacy>>>, + target_bindings: &PerNS>>>, + target: Ident, + ) { + // Check if we are at the root of a macro expansion and skip if we are. + if directive.parent_scope.expansion != Mark::root() { + return; + } + + if let ModuleKind::Def(_, _) = directive.parent_scope.module.kind { + return; + } + + let mut is_redundant = PerNS { + value_ns: None, + type_ns: None, + macro_ns: None, + }; + + let mut redundant_span = PerNS { + value_ns: None, + type_ns: None, + macro_ns: None, + }; + + self.per_ns(|this, ns| if let Some(binding) = source_bindings[ns].get().ok() { + if binding.def() == Def::Err { + return; + } + + let orig_blacklisted_binding = mem::replace( + &mut this.blacklisted_binding, + target_bindings[ns].get() + ); + + match this.early_resolve_ident_in_lexical_scope( + target, + ScopeSet::Import(ns), + &directive.parent_scope, + false, + false, + directive.span, + ) { + Ok(other_binding) => { + is_redundant[ns] = Some(binding.def() == other_binding.def()); + redundant_span[ns] = Some(other_binding.span); + } + Err(_) => is_redundant[ns] = Some(false) + } + + this.blacklisted_binding = orig_blacklisted_binding; + }); + + if !is_redundant.is_empty() && + is_redundant.present_items().all(|is_redundant| is_redundant) + { + self.session.buffer_lint( + REDUNDANT_IMPORT, + directive.id, + directive.span, + &format!("the item `{}` is imported redundantly", ident), + ); + + for span in redundant_span.present_items() { + self.session.buffer_lint( + REDUNDANT_IMPORT, + directive.id, + span, + "another import" + ); + } + } + } + fn resolve_glob_import(&mut self, directive: &'b ImportDirective<'b>) { let module = match directive.imported_module.get().unwrap() { ModuleOrUniformRoot::Module(module) => module, diff --git a/src/test/ui/lint/use-redundant.rs b/src/test/ui/lint/use-redundant.rs new file mode 100644 index 00000000000..50d4d30625a --- /dev/null +++ b/src/test/ui/lint/use-redundant.rs @@ -0,0 +1,17 @@ +// compile-pass + +use crate::foo::Bar; //~ WARNING first import + +mod foo { + pub type Bar = i32; +} + +fn baz() -> Bar { + 3 +} + +fn main() { + use crate::foo::Bar; //~ WARNING redundant import + let _a: Bar = 3; + baz(); +} diff --git a/src/test/ui/lint/use-redundant.stderr b/src/test/ui/lint/use-redundant.stderr new file mode 100644 index 00000000000..6a6becc5e61 --- /dev/null +++ b/src/test/ui/lint/use-redundant.stderr @@ -0,0 +1,14 @@ +warning: the item `Bar` is imported redundantly + --> $DIR/use-redundant.rs:14:9 + | +LL | use crate::foo::Bar; //~ WARNING redundant import + | ^^^^^^^^^^^^^^^ + | + = note: #[warn(redundant_import)] on by default + +warning: another import + --> $DIR/use-redundant.rs:3:5 + | +LL | use crate::foo::Bar; //~ WARNING first import + | ^^^^^^^^^^^^^^^ +