Rollup merge of rust-lang#58805 - fabric-and-ink:redundant_import, r=petrochenkov

Lint for redundant imports

This is an attempt at solving rust-lang#10178. The changes are suggested by `@petrochenkov`. I need some feedback on how to continue, since I get a lot of false positives in macro invocations in `libcore`. I suspect that the test

```
...
if directive.parent_scope.expansion == Mark::root() {
    return;
}
...
```

is wrong. But I don't know how to check correctly if I am at a macro expansion root – given that this is the reason for the errors in the first place.

Todos:

- [x] Solve problem with false positives
- [x] Turn hard error into warning
- [x] Add unit tests

Closes rust-lang#10178

Author: Centril

src/liballoc/borrow.rs

@@ -135,7 +135,7 @@ impl<T> ToOwned for T
 /// Another example showing how to keep `Cow` in a struct:
 ///
 /// ```
-/// use std::borrow::{Cow, ToOwned};
+/// use std::borrow::Cow;
 ///
 /// struct Items<'a, X: 'a> where [X]: ToOwned<Owned = Vec<X>> {
 ///     values: Cow<'a, [X]>,

src/libcore/cell.rs

@@ -1423,7 +1423,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
 ///
 /// ```
 /// use std::cell::UnsafeCell;
-/// use std::marker::Sync;
 ///
 /// # #[allow(dead_code)]
 /// struct NotThreadSafe<T> {

src/librustc/lint/builtin.rs

@@ -482,6 +482,7 @@ pub enum BuiltinLintDiagnostics {
     UnknownCrateTypes(Span, String, String),
     UnusedImports(String, Vec<(Span, String)>),
     NestedImplTrait { outer_impl_trait_span: Span, inner_impl_trait_span: Span },
+    RedundantImport(Vec<(Span, bool)>, ast::Ident),
 }
 
 impl BuiltinLintDiagnostics {
@@ -578,6 +579,15 @@ impl BuiltinLintDiagnostics {
                 db.span_label(outer_impl_trait_span, "outer `impl Trait`");
                 db.span_label(inner_impl_trait_span, "nested `impl Trait` here");
             }
+            BuiltinLintDiagnostics::RedundantImport(spans, ident) => {
+                for (span, is_imported) in spans {
+                    let introduced = if is_imported { "imported" } else { "defined" };
+                    db.span_label(
+                        span,
+                        format!("the item `{}` is already {} here", ident, introduced)
+                    );
+                }
+            }
         }
     }
 }

src/librustc/ty/query/on_disk_cache.rs

@@ -778,7 +778,6 @@ impl<'enc, 'a, 'tcx, E> CacheEncoder<'enc, 'a, 'tcx, E>
                                                  value: &V)
                                                  -> Result<(), E::Error>
     {
-        use crate::ty::codec::TyEncoder;
         let start_pos = self.position();
 
         tag.encode(self)?;

src/librustc_codegen_llvm/context.rs

@@ -377,7 +377,6 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
     // Returns a Value of the "eh_unwind_resume" lang item if one is defined,
     // otherwise declares it as an external function.
     fn eh_unwind_resume(&self) -> &'ll Value {
-        use crate::attributes;
         let unwresume = &self.eh_unwind_resume;
         if let Some(llfn) = unwresume.get() {
             return llfn;

src/librustc_codegen_ssa/mir/rvalue.rs

@@ -750,7 +750,6 @@ fn cast_int_to_float<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
         // All inputs greater or equal to (f32::MAX + 0.5 ULP) are rounded to infinity,
         // and for everything else LLVM's uitofp works just fine.
         use rustc_apfloat::ieee::Single;
-        use rustc_apfloat::Float;
         const MAX_F32_PLUS_HALF_ULP: u128 = ((1 << (Single::PRECISION + 1)) - 1)
                                             << (Single::MAX_EXP - Single::PRECISION as i16);
         let max = bx.cx().const_uint_big(int_ty, MAX_F32_PLUS_HALF_ULP);

src/librustc_errors/lib.rs

@@ -155,7 +155,7 @@ impl CodeSuggestion {
     /// Returns the assembled code suggestions and whether they should be shown with an underline.
     pub fn splice_lines(&self, cm: &SourceMapperDyn)
                         -> Vec<(String, Vec<SubstitutionPart>)> {
-        use syntax_pos::{CharPos, Loc, Pos};
+        use syntax_pos::{CharPos, Pos};
 
         fn push_trailing(buf: &mut String,
                          line_opt: Option<&Cow<'_, str>>,

src/librustc_interface/profile/mod.rs

@@ -61,7 +61,6 @@ fn total_duration(traces: &[trace::Rec]) -> Duration {
 fn profile_queries_thread(r: Receiver<ProfileQueriesMsg>) {
     use self::trace::*;
     use std::fs::File;
-    use std::time::{Instant};
 
     let mut profq_msgs: Vec<ProfileQueriesMsg> = vec![];
     let mut frame: StackFrame = StackFrame { parse_st: ParseState::Clear, traces: vec![] };

src/librustc_mir/hair/pattern/mod.rs

@@ -427,7 +427,6 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
 
                 let mut kind = match (lo, hi) {
                     (PatternKind::Constant { value: lo }, PatternKind::Constant { value: hi }) => {
-                        use std::cmp::Ordering;
                         let cmp = compare_const_vals(
                             self.tcx,
                             lo,

src/librustc_mir/interpret/operator.rs

@@ -331,8 +331,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         val: ImmTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx, Scalar<M::PointerTag>> {
         use rustc::mir::UnOp::*;
-        use rustc_apfloat::ieee::{Single, Double};
-        use rustc_apfloat::Float;
 
         let layout = val.layout;
         let val = val.to_scalar()?;

src/librustc_resolve/lib.rs

@@ -1733,7 +1733,6 @@ impl<'a> Resolver<'a> {
     /// just that an error occurred.
     pub fn resolve_str_path_error(&mut self, span: Span, path_str: &str, is_value: bool)
         -> Result<hir::Path, ()> {
-        use std::iter;
         let mut errored = false;
 
         let path = if path_str.starts_with("::") {

src/librustc_resolve/resolve_imports.rs

@@ -7,14 +7,19 @@ 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;
 
 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,
+    UNUSED_IMPORTS,
+};
 use rustc::hir::def_id::{CrateNum, DefId};
 use rustc::hir::def::*;
 use rustc::session::DiagnosticMessageId;
@@ -1230,10 +1235,97 @@ 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<Cell<Result<&'b NameBinding<'b>, Determinacy>>>,
+        target_bindings: &PerNS<Cell<Option<&'b NameBinding<'b>>>>,
+        target: Ident,
+    ) {
+        // Skip if the import was produced by a macro.
+        if directive.parent_scope.expansion != Mark::root() {
+            return;
+        }
+
+        // Skip if we are inside a named module (in contrast to an anonymous
+        // module defined by a block).
+        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()
+                        && !other_binding.is_ambiguity()
+                    );
+                    redundant_span[ns] =
+                        Some((other_binding.span, other_binding.is_import()));
+                }
+                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_with_diagnostic(
+                UNUSED_IMPORTS,
+                directive.id,
+                directive.span,
+                &format!("the item `{}` is imported redundantly", ident),
+                BuiltinLintDiagnostics::RedundantImport(
+                    redundant_span.present_items().collect(),
+                    ident,
+                ),
+            );
+        }
+    }
+
     fn resolve_glob_import(&mut self, directive: &'b ImportDirective<'b>) {
         let module = match directive.imported_module.get().unwrap() {
             ModuleOrUniformRoot::Module(module) => module,

src/librustc_traits/lowering/environment.rs

@@ -190,7 +190,7 @@ crate fn environment<'a, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     def_id: DefId
 ) -> Environment<'tcx> {
-    use super::{Lower, IntoFromEnvGoal};
+    use super::IntoFromEnvGoal;
     use rustc::hir::{Node, TraitItemKind, ImplItemKind, ItemKind, ForeignItemKind};
 
     debug!("environment(def_id = {:?})", def_id);

src/librustc_typeck/check/wfcheck.rs

@@ -420,9 +420,6 @@ fn check_where_clauses<'a, 'gcx, 'fcx, 'tcx>(
     def_id: DefId,
     return_ty: Option<Ty<'tcx>>,
 ) {
-    use ty::subst::Subst;
-    use rustc::ty::TypeFoldable;
-
     let predicates = fcx.tcx.predicates_of(def_id);
 
     let generics = tcx.generics_of(def_id);
@@ -1010,8 +1007,6 @@ fn check_false_global_bounds<'a, 'gcx, 'tcx>(
     span: Span,
     id: hir::HirId)
 {
-    use rustc::ty::TypeFoldable;
-
     let empty_env = ty::ParamEnv::empty();
 
     let def_id = fcx.tcx.hir().local_def_id_from_hir_id(id);

src/librustdoc/html/render.rs

@@ -1061,8 +1061,6 @@ themePicker.onblur = handleThemeButtonsBlur;
     }
 
     if cx.shared.include_sources {
-        use std::path::Component;
-
         let mut hierarchy = Hierarchy::new(OsString::new());
         for source in cx.shared.local_sources.iter()
                                              .filter_map(|p| p.0.strip_prefix(&cx.shared.src_root)

src/librustdoc/test.rs

@@ -371,8 +371,7 @@ pub fn make_test(s: &str,
     // Uses libsyntax to parse the doctest and find if there's a main fn and the extern
     // crate already is included.
     let (already_has_main, already_has_extern_crate, found_macro) = crate::syntax::with_globals(|| {
-        use crate::syntax::{ast, parse::{self, ParseSess}, source_map::FilePathMapping};
-        use crate::syntax_pos::FileName;
+        use crate::syntax::{parse::{self, ParseSess}, source_map::FilePathMapping};
         use errors::emitter::EmitterWriter;
         use errors::Handler;
 

src/test/run-pass/binding/match-arm-statics.rs

@@ -45,8 +45,6 @@ pub mod glfw {
 }
 
 fn issue_6533() {
-    use glfw;
-
     fn action_to_str(state: glfw::InputState) -> &'static str {
         use glfw::{RELEASE, PRESS, REPEAT};
         match state {

src/test/run-pass/ifmt.rs

@@ -238,7 +238,6 @@ pub fn main() {
 // Basic test to make sure that we can invoke the `write!` macro with an
 // fmt::Write instance.
 fn test_write() {
-    use std::fmt::Write;
     let mut buf = String::new();
     write!(&mut buf, "{}", 3);
     {
@@ -267,7 +266,6 @@ fn test_print() {
 // Just make sure that the macros are defined, there's not really a lot that we
 // can do with them just yet (to test the output)
 fn test_format_args() {
-    use std::fmt::Write;
     let mut buf = String::new();
     {
         let w = &mut buf;

src/test/run-pass/invalid_const_promotion.rs

@@ -25,7 +25,6 @@ fn foo() {
 #[cfg(unix)]
 fn check_status(status: std::process::ExitStatus)
 {
-    use libc;
     use std::os::unix::process::ExitStatusExt;
 
     assert!(status.signal() == Some(libc::SIGILL)

src/test/run-pass/issues/issue-38556.rs

@@ -9,6 +9,5 @@ macro_rules! reexport {
 reexport!();
 
 fn main() {
-    use Bar;
     fn f(_: Bar) {}
 }

src/test/run-pass/issues/issue-39367.rs

@@ -15,7 +15,6 @@ fn arena() -> &'static ArenaSet<Vec<u8>> {
         fn require_sync<T: Sync>(_: &T) { }
         unsafe fn __stability() -> &'static ArenaSet<Vec<u8>> {
             use std::mem::transmute;
-            use std::boxed::Box;
             static mut DATA: *const ArenaSet<Vec<u8>> = 0 as *const ArenaSet<Vec<u8>>;
 
             static mut ONCE: Once = ONCE_INIT;

src/test/run-pass/out-of-stack.rs

@@ -36,7 +36,6 @@ fn loud_recurse() {
 #[cfg(unix)]
 fn check_status(status: std::process::ExitStatus)
 {
-    use libc;
     use std::os::unix::process::ExitStatusExt;
 
     assert!(!status.success());

src/test/run-pass/rfcs/rfc-2126-extern-absolute-paths/basic.rs

@@ -4,6 +4,8 @@
 // compile-flags:--extern xcrate
 // edition:2018
 
+#![allow(unused_imports)]
+
 use xcrate::Z;
 
 fn f() {

src/test/run-pass/traits/traits-multidispatch-infer-convert-target.rs

@@ -28,7 +28,6 @@ where T : Convert<U>
 }
 
 fn main() {
-    use std::default::Default;
     // T = i16, U = u32
     test(22_i16, Default::default(),  2, 4);
 

src/test/ui/lint/lint-unused-imports.rs

@@ -66,6 +66,7 @@ mod bar {
 
 fn g() {
     use self::g; //~ ERROR unused import: `self::g`
+    //~^ ERROR the item `g` is imported redundantly
     fn f() {
         self::g();
     }
@@ -75,6 +76,7 @@ fn g() {
 #[allow(unused_variables)]
 fn h() {
     use test2::foo; //~ ERROR unused import: `test2::foo`
+    //~^ ERROR the item `foo` is imported redundantly
     let foo = 0;
 }