Auto merge of rust-lang#6905 - ThibsG:fpSingleComponentPathImports5210, r=giraffate

Fix FP in `single_component_path_imports` lint

Fix FP in  `single_component_path_imports` lint when the import is reused with `self`, like in `use self::module`.

Fixes rust-lang#5210

changelog: none

Author: bors

clippy_lints/src/single_component_path_imports.rs

@@ -1,11 +1,10 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::in_macro;
-use if_chain::if_chain;
-use rustc_ast::{Item, ItemKind, UseTreeKind};
+use rustc_ast::{ptr::P, Crate, Item, ItemKind, ModKind, UseTreeKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::edition::Edition;
+use rustc_span::{edition::Edition, symbol::kw, Span, Symbol};
 
 declare_clippy_lint! {
     /// **What it does:** Checking for imports with single component use path.
@@ -38,26 +37,120 @@ declare_clippy_lint! {
 declare_lint_pass!(SingleComponentPathImports => [SINGLE_COMPONENT_PATH_IMPORTS]);
 
 impl EarlyLintPass for SingleComponentPathImports {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
-        if_chain! {
-            if !in_macro(item.span);
-            if cx.sess.opts.edition >= Edition::Edition2018;
-            if !item.vis.kind.is_pub();
-            if let ItemKind::Use(use_tree) = &item.kind;
-            if let segments = &use_tree.prefix.segments;
-            if segments.len() == 1;
-            if let UseTreeKind::Simple(None, _, _) = use_tree.kind;
-            then {
+    fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) {
+        if cx.sess.opts.edition < Edition::Edition2018 {
+            return;
+        }
+        check_mod(cx, &krate.items);
+    }
+}
+
+fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>]) {
+    // keep track of imports reused with `self` keyword,
+    // such as `self::crypto_hash` in the example below
+    // ```rust,ignore
+    // use self::crypto_hash::{Algorithm, Hasher};
+    // ```
+    let mut imports_reused_with_self = Vec::new();
+
+    // keep track of single use statements
+    // such as `crypto_hash` in the example below
+    // ```rust,ignore
+    // use crypto_hash;
+    // ```
+    let mut single_use_usages = Vec::new();
+
+    for item in items {
+        track_uses(cx, &item, &mut imports_reused_with_self, &mut single_use_usages);
+    }
+
+    for single_use in &single_use_usages {
+        if !imports_reused_with_self.contains(&single_use.0) {
+            let can_suggest = single_use.2;
+            if can_suggest {
                 span_lint_and_sugg(
                     cx,
                     SINGLE_COMPONENT_PATH_IMPORTS,
-                    item.span,
+                    single_use.1,
                     "this import is redundant",
                     "remove it entirely",
                     String::new(),
-                    Applicability::MachineApplicable
+                    Applicability::MachineApplicable,
+                );
+            } else {
+                span_lint_and_help(
+                    cx,
+                    SINGLE_COMPONENT_PATH_IMPORTS,
+                    single_use.1,
+                    "this import is redundant",
+                    None,
+                    "remove this import",
                 );
             }
         }
     }
 }
+
+fn track_uses(
+    cx: &EarlyContext<'_>,
+    item: &Item,
+    imports_reused_with_self: &mut Vec<Symbol>,
+    single_use_usages: &mut Vec<(Symbol, Span, bool)>,
+) {
+    if in_macro(item.span) || item.vis.kind.is_pub() {
+        return;
+    }
+
+    match &item.kind {
+        ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) => {
+            check_mod(cx, &items);
+        },
+        ItemKind::Use(use_tree) => {
+            let segments = &use_tree.prefix.segments;
+
+            // keep track of `use some_module;` usages
+            if segments.len() == 1 {
+                if let UseTreeKind::Simple(None, _, _) = use_tree.kind {
+                    let ident = &segments[0].ident;
+                    single_use_usages.push((ident.name, item.span, true));
+                }
+                return;
+            }
+
+            if segments.is_empty() {
+                // keep track of `use {some_module, some_other_module};` usages
+                if let UseTreeKind::Nested(trees) = &use_tree.kind {
+                    for tree in trees {
+                        let segments = &tree.0.prefix.segments;
+                        if segments.len() == 1 {
+                            if let UseTreeKind::Simple(None, _, _) = tree.0.kind {
+                                let ident = &segments[0].ident;
+                                single_use_usages.push((ident.name, tree.0.span, false));
+                            }
+                        }
+                    }
+                }
+            } else {
+                // keep track of `use self::some_module` usages
+                if segments[0].ident.name == kw::SelfLower {
+                    // simple case such as `use self::module::SomeStruct`
+                    if segments.len() > 1 {
+                        imports_reused_with_self.push(segments[1].ident.name);
+                        return;
+                    }
+
+                    // nested case such as `use self::{module1::Struct1, module2::Struct2}`
+                    if let UseTreeKind::Nested(trees) = &use_tree.kind {
+                        for tree in trees {
+                            let segments = &tree.0.prefix.segments;
+                            if !segments.is_empty() {
+                                imports_reused_with_self.push(segments[0].ident.name);
+                            }
+                        }
+                    }
+                }
+            }
+        },
+        _ => {},
+    }
+}

tests/ui/single_component_path_imports.fixed

@@ -19,3 +19,16 @@ fn main() {
     // False positive #5154, shouldn't trigger lint.
     m!();
 }
+
+mod hello_mod {
+    
+    #[allow(dead_code)]
+    fn hello_mod() {}
+}
+
+mod hi_mod {
+    use self::regex::{Regex, RegexSet};
+    use regex;
+    #[allow(dead_code)]
+    fn hi_mod() {}
+}

tests/ui/single_component_path_imports.rs

@@ -19,3 +19,16 @@ fn main() {
     // False positive #5154, shouldn't trigger lint.
     m!();
 }
+
+mod hello_mod {
+    use regex;
+    #[allow(dead_code)]
+    fn hello_mod() {}
+}
+
+mod hi_mod {
+    use self::regex::{Regex, RegexSet};
+    use regex;
+    #[allow(dead_code)]
+    fn hi_mod() {}
+}

tests/ui/single_component_path_imports.stderr

@@ -1,10 +1,16 @@
+error: this import is redundant
+  --> $DIR/single_component_path_imports.rs:24:5
+   |
+LL |     use regex;
+   |     ^^^^^^^^^^ help: remove it entirely
+   |
+   = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
+
 error: this import is redundant
   --> $DIR/single_component_path_imports.rs:6:1
    |
 LL | use regex;
    | ^^^^^^^^^^ help: remove it entirely
-   |
-   = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 

tests/ui/single_component_path_imports_nested_first.rs

@@ -0,0 +1,17 @@
+// edition:2018
+#![warn(clippy::single_component_path_imports)]
+#![allow(unused_imports)]
+
+use regex;
+use serde as edres;
+pub use serde;
+
+fn main() {
+    regex::Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
+}
+
+mod root_nested_use_mod {
+    use {regex, serde};
+    #[allow(dead_code)]
+    fn root_nested_use_mod() {}
+}

tests/ui/single_component_path_imports_nested_first.stderr

@@ -0,0 +1,25 @@
+error: this import is redundant
+  --> $DIR/single_component_path_imports_nested_first.rs:14:10
+   |
+LL |     use {regex, serde};
+   |          ^^^^^
+   |
+   = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
+   = help: remove this import
+
+error: this import is redundant
+  --> $DIR/single_component_path_imports_nested_first.rs:14:17
+   |
+LL |     use {regex, serde};
+   |                 ^^^^^
+   |
+   = help: remove this import
+
+error: this import is redundant
+  --> $DIR/single_component_path_imports_nested_first.rs:5:1
+   |
+LL | use regex;
+   | ^^^^^^^^^^ help: remove it entirely
+
+error: aborting due to 3 previous errors
+

tests/ui/single_component_path_imports_self_after.rs

@@ -0,0 +1,16 @@
+// edition:2018
+#![warn(clippy::single_component_path_imports)]
+#![allow(unused_imports)]
+
+use self::regex::{Regex as xeger, RegexSet as tesxeger};
+pub use self::{
+    regex::{Regex, RegexSet},
+    some_mod::SomeType,
+};
+use regex;
+
+mod some_mod {
+    pub struct SomeType;
+}
+
+fn main() {}

tests/ui/single_component_path_imports_self_before.rs

@@ -0,0 +1,17 @@
+// edition:2018
+#![warn(clippy::single_component_path_imports)]
+#![allow(unused_imports)]
+
+use regex;
+
+use self::regex::{Regex as xeger, RegexSet as tesxeger};
+pub use self::{
+    regex::{Regex, RegexSet},
+    some_mod::SomeType,
+};
+
+mod some_mod {
+    pub struct SomeType;
+}
+
+fn main() {}