diff options
| -rw-r--r-- | clippy_lints/src/attrs.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/single_component_path_imports.rs | 219 | ||||
| -rw-r--r-- | tests/ui/single_component_path_imports.stderr | 12 | ||||
| -rw-r--r-- | tests/ui/single_component_path_imports_nested_first.stderr | 15 | ||||
| -rw-r--r-- | tests/ui/useless_attribute.fixed | 21 | ||||
| -rw-r--r-- | tests/ui/useless_attribute.rs | 21 | ||||
| -rw-r--r-- | tests/ui/useless_attribute.stderr | 6 |
8 files changed, 180 insertions, 120 deletions
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 235ab4977ce..ecf8e83375d 100644 --- a/clippy_lints/src/attrs.rs +++ b/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" ) }) { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4e640d7ebf6..3ab5031696d 100644 --- a/clippy_lints/src/lib.rs +++ b/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 |_| { diff --git a/clippy_lints/src/single_component_path_imports.rs b/clippy_lints/src/single_component_path_imports.rs index 66b79513032..2036e85db7e 100644 --- a/clippy_lints/src/single_component_path_imports.rs +++ b/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); + } } } } } - } - }, - _ => {}, + }, + _ => {}, + } } } diff --git a/tests/ui/single_component_path_imports.stderr b/tests/ui/single_component_path_imports.stderr index 509c88ac256..71dcc25d6e5 100644 --- a/tests/ui/single_component_path_imports.stderr +++ b/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 diff --git a/tests/ui/single_component_path_imports_nested_first.stderr b/tests/ui/single_component_path_imports_nested_first.stderr index 633546f6419..330f285202d 100644 --- a/tests/ui/single_component_path_imports_nested_first.stderr +++ b/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 diff --git a/tests/ui/useless_attribute.fixed b/tests/ui/useless_attribute.fixed index c23231a99e9..871e4fb5c3a 100644 --- a/tests/ui/useless_attribute.fixed +++ b/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(); } diff --git a/tests/ui/useless_attribute.rs b/tests/ui/useless_attribute.rs index 7a7b198ea60..cb50736ba39 100644 --- a/tests/ui/useless_attribute.rs +++ b/tests/ui/useless_attribute.rs @@ -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(); } diff --git a/tests/ui/useless_attribute.stderr b/tests/ui/useless_attribute.stderr index 255d2876355..a7ea0df2294 100644 --- a/tests/ui/useless_attribute.stderr +++ b/tests/ui/useless_attribute.stderr @@ -1,5 +1,5 @@ error: useless lint attribute - --> $DIR/useless_attribute.rs:8:1 + --> $DIR/useless_attribute.rs:9:1 | LL | #[allow(dead_code)] | ^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![allow(dead_code)]` @@ -7,13 +7,13 @@ LL | #[allow(dead_code)] = note: `-D clippy::useless-attribute` implied by `-D warnings` error: useless lint attribute - --> $DIR/useless_attribute.rs:9:1 + --> $DIR/useless_attribute.rs:10:1 | LL | #[cfg_attr(feature = "cargo-clippy", allow(dead_code))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(feature = "cargo-clippy", allow(dead_code)` error: useless lint attribute - --> $DIR/useless_attribute.rs:67:5 + --> $DIR/useless_attribute.rs:21:5 | LL | #[allow(clippy::almost_swapped)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![allow(clippy::almost_swapped)]` |