Auto merge of rust-lang#9879 - Alexendoo:allow, r=Manishearth

Fix `#[allow]` for `module_name_repetitions` & `single_component_path_imports`

Fixes rust-lang#7511
Fixes rust-lang#8768
Fixes rust-lang#9401

`single_component_path_imports` needed some changes to the lint itself, it now buffers the found single component paths to emit in the equivalent `check_item`

changelog: Fix `#[allow(clippy::module_name_repetitions)]` and `#[allow(clippy::single_component_path_imports)]`

Author: bors

clippy_lints/src/attrs.rs

@@ -378,7 +378,9 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
                                                         | "enum_glob_use"
                                                         | "redundant_pub_crate"
                                                         | "macro_use_imports"
-                                                        | "unsafe_removed_from_name",
+                                                        | "unsafe_removed_from_name"
+                                                        | "module_name_repetitions"
+                                                        | "single_component_path_imports"
                                                 )
                                             })
                                         {

clippy_lints/src/lib.rs

@@ -792,7 +792,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(floating_point_arithmetic::FloatingPointArithmetic));
     store.register_early_pass(|| Box::new(as_conversions::AsConversions));
     store.register_late_pass(|_| Box::new(let_underscore::LetUnderscore));
-    store.register_early_pass(|| Box::new(single_component_path_imports::SingleComponentPathImports));
+    store.register_early_pass(|| Box::<single_component_path_imports::SingleComponentPathImports>::default());
     let max_fn_params_bools = conf.max_fn_params_bools;
     let max_struct_bools = conf.max_struct_bools;
     store.register_late_pass(move |_| {

clippy_lints/src/single_component_path_imports.rs

@@ -1,8 +1,9 @@
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
+use rustc_ast::node_id::{NodeId, NodeMap};
 use rustc_ast::{ptr::P, Crate, Item, ItemKind, MacroDef, ModKind, UseTreeKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{edition::Edition, symbol::kw, Span, Symbol};
 
 declare_clippy_lint! {
@@ -33,51 +34,32 @@ declare_clippy_lint! {
     "imports with single component path are redundant"
 }
 
-declare_lint_pass!(SingleComponentPathImports => [SINGLE_COMPONENT_PATH_IMPORTS]);
+#[derive(Default)]
+pub struct SingleComponentPathImports {
+    /// Buffer found usages to emit when visiting that item so that `#[allow]` works as expected
+    found: NodeMap<Vec<SingleUse>>,
+}
+
+struct SingleUse {
+    name: Symbol,
+    span: Span,
+    item_id: NodeId,
+    can_suggest: bool,
+}
+
+impl_lint_pass!(SingleComponentPathImports => [SINGLE_COMPONENT_PATH_IMPORTS]);
 
 impl EarlyLintPass for SingleComponentPathImports {
     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();
-
-    // keep track of macros defined in the module as we don't want it to trigger on this (#7106)
-    // ```rust,ignore
-    // macro_rules! foo { () => {} };
-    // pub(crate) use foo;
-    // ```
-    let mut macros = Vec::new();
-
-    for item in items {
-        track_uses(
-            cx,
-            item,
-            &mut imports_reused_with_self,
-            &mut single_use_usages,
-            &mut macros,
-        );
+        self.check_mod(cx, &krate.items);
     }
 
-    for (name, span, can_suggest) in single_use_usages {
-        if !imports_reused_with_self.contains(&name) {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        for SingleUse { span, can_suggest, .. } in self.found.remove(&item.id).into_iter().flatten() {
             if can_suggest {
                 span_lint_and_sugg(
                     cx,
@@ -102,74 +84,127 @@ fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>]) {
     }
 }
 
-fn track_uses(
-    cx: &EarlyContext<'_>,
-    item: &Item,
-    imports_reused_with_self: &mut Vec<Symbol>,
-    single_use_usages: &mut Vec<(Symbol, Span, bool)>,
-    macros: &mut Vec<Symbol>,
-) {
-    if item.span.from_expansion() || item.vis.kind.is_pub() {
-        return;
-    }
+impl SingleComponentPathImports {
+    fn check_mod(&mut self, cx: &EarlyContext<'_>, items: &[P<Item>]) {
+        // keep track of imports reused with `self` keyword, such as `self::crypto_hash` in the example
+        // below. Removing the `use crypto_hash;` would make this a compile error
+        // ```
+        // use crypto_hash;
+        //
+        // use self::crypto_hash::{Algorithm, Hasher};
+        // ```
+        let mut imports_reused_with_self = Vec::new();
 
-    match &item.kind {
-        ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) => {
-            check_mod(cx, items);
-        },
-        ItemKind::MacroDef(MacroDef { macro_rules: true, .. }) => {
-            macros.push(item.ident.name);
-        },
-        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 name = segments[0].ident.name;
-                    if !macros.contains(&name) {
-                        single_use_usages.push((name, item.span, true));
-                    }
-                }
-                return;
+        // keep track of single use statements such as `crypto_hash` in the example below
+        // ```
+        // use crypto_hash;
+        // ```
+        let mut single_use_usages = Vec::new();
+
+        // keep track of macros defined in the module as we don't want it to trigger on this (#7106)
+        // ```
+        // macro_rules! foo { () => {} };
+        // pub(crate) use foo;
+        // ```
+        let mut macros = Vec::new();
+
+        for item in items {
+            self.track_uses(
+                cx,
+                item,
+                &mut imports_reused_with_self,
+                &mut single_use_usages,
+                &mut macros,
+            );
+        }
+
+        for usage in single_use_usages {
+            if !imports_reused_with_self.contains(&usage.name) {
+                self.found.entry(usage.item_id).or_default().push(usage);
             }
+        }
+    }
 
-            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 name = segments[0].ident.name;
-                                if !macros.contains(&name) {
-                                    single_use_usages.push((name, tree.0.span, false));
-                                }
-                            }
+    fn track_uses(
+        &mut self,
+        cx: &EarlyContext<'_>,
+        item: &Item,
+        imports_reused_with_self: &mut Vec<Symbol>,
+        single_use_usages: &mut Vec<SingleUse>,
+        macros: &mut Vec<Symbol>,
+    ) {
+        if item.span.from_expansion() || item.vis.kind.is_pub() {
+            return;
+        }
+
+        match &item.kind {
+            ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) => {
+                self.check_mod(cx, items);
+            },
+            ItemKind::MacroDef(MacroDef { macro_rules: true, .. }) => {
+                macros.push(item.ident.name);
+            },
+            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 name = segments[0].ident.name;
+                        if !macros.contains(&name) {
+                            single_use_usages.push(SingleUse {
+                                name,
+                                span: item.span,
+                                item_id: item.id,
+                                can_suggest: true,
+                            });
                         }
                     }
+                    return;
                 }
-            } 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 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.is_empty() {
-                                imports_reused_with_self.push(segments[0].ident.name);
+                            if segments.len() == 1 {
+                                if let UseTreeKind::Simple(None, _, _) = tree.0.kind {
+                                    let name = segments[0].ident.name;
+                                    if !macros.contains(&name) {
+                                        single_use_usages.push(SingleUse {
+                                            name,
+                                            span: tree.0.span,
+                                            item_id: item.id,
+                                            can_suggest: 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.stderr

@@ -1,16 +1,16 @@
 error: this import is redundant
-  --> $DIR/single_component_path_imports.rs:23:5
+  --> $DIR/single_component_path_imports.rs:5:1
    |
-LL |     use regex;
-   |     ^^^^^^^^^^ help: remove it entirely
+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:5:1
+  --> $DIR/single_component_path_imports.rs:23:5
    |
-LL | use regex;
-   | ^^^^^^^^^^ help: remove it entirely
+LL |     use regex;
+   |     ^^^^^^^^^^ help: remove it entirely
 
 error: aborting due to 2 previous errors
 

tests/ui/single_component_path_imports_nested_first.stderr

@@ -1,11 +1,18 @@
+error: this import is redundant
+  --> $DIR/single_component_path_imports_nested_first.rs:4:1
+   |
+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_nested_first.rs:13:10
    |
 LL |     use {regex, serde};
    |          ^^^^^
    |
    = help: remove this import
-   = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
 
 error: this import is redundant
   --> $DIR/single_component_path_imports_nested_first.rs:13:17
@@ -15,11 +22,5 @@ LL |     use {regex, serde};
    |
    = help: remove this import
 
-error: this import is redundant
-  --> $DIR/single_component_path_imports_nested_first.rs:4:1
-   |
-LL | use regex;
-   | ^^^^^^^^^^ help: remove it entirely
-
 error: aborting due to 3 previous errors
 

tests/ui/useless_attribute.fixed

@@ -1,6 +1,7 @@
 // run-rustfix
 // aux-build:proc_macro_derive.rs
 
+#![allow(unused)]
 #![warn(clippy::useless_attribute)]
 #![warn(unreachable_pub)]
 #![feature(rustc_private)]
@@ -16,6 +17,13 @@ extern crate rustc_middle;
 #[macro_use]
 extern crate proc_macro_derive;
 
+fn test_indented_attr() {
+    #![allow(clippy::almost_swapped)]
+    use std::collections::HashSet;
+
+    let _ = HashSet::<u32>::default();
+}
+
 // don't lint on unused_import for `use` items
 #[allow(unused_imports)]
 use std::collections;
@@ -63,13 +71,16 @@ mod c {
     pub(crate) struct S;
 }
 
-fn test_indented_attr() {
-    #![allow(clippy::almost_swapped)]
-    use std::collections::HashSet;
-
-    let _ = HashSet::<u32>::default();
+// https://github.com/rust-lang/rust-clippy/issues/7511
+pub mod split {
+    #[allow(clippy::module_name_repetitions)]
+    pub use regex::SplitN;
 }
 
+// https://github.com/rust-lang/rust-clippy/issues/8768
+#[allow(clippy::single_component_path_imports)]
+use regex;
+
 fn main() {
     test_indented_attr();
 }