mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

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

Fixes #7511
Fixes #8768
Fixes #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)]`
This commit is contained in:
bors 2022-11-20 13:17:02 +00:00
commit d445ced166
8 changed files with 185 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clippy_lints/src/attrs.rs
View File

@ -378,7 +378,9 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
| "enum_glob_use"
| "redundant_pub_crate"
| "macro_use_imports"
| "unsafe_removed_from_name",
| "unsafe_removed_from_name"
| "module_name_repetitions"
| "single_component_path_imports"
)
})
{

2
clippy_lints/src/lib.rs
View File

@ -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 |_| {

225
clippy_lints/src/single_component_path_imports.rs
View File

@ -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 @@
"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();
// 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);
}
}
}
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;
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;
}
// 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));
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;
}
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 name = segments[0].ident.name;
if !macros.contains(&name) {
single_use_usages.push((name, tree.0.span, false));
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(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);
}
}
}
}
}
} 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);
}
}
}
}
}
},
_ => {},
},
_ => {},
}
}
}

16
tests/ui/single_component_path_imports.stderr
View File

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

15
tests/ui/single_component_path_imports_nested_first.stderr
View File

@ -1,3 +1,11 @@
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
|
@ -5,7 +13,6 @@ 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

21
tests/ui/useless_attribute.fixed
View File

@ -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();
}

21
tests/ui/useless_attribute.rs
View File

@ -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 @@
#[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();
}

6
tests/ui/useless_attribute.stderr
View File

@ -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)]`