Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[single_component_path_imports] to include super:: and crate::; … #7190

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 46 additions & 9 deletions clippy_lints/src/single_component_path_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,25 @@ impl EarlyLintPass for SingleComponentPathImports {
if cx.sess.opts.edition < Edition::Edition2018 {
return;
}
check_mod(cx, &krate.items);

// keep track of crate-roots and super-roots
// such as `super::crypto_hash` in the example below
// ```rust,ignore
// use super::crypto_hash::{Algorithm, Hasher};
// ```
// or
// ```rust,ignore
// use crate::crypto_hash::{Algorithm, Hasher};
// ```
// This one needs to persist.

let mut imports_reused_as_roots = Vec::new();
check_mod(cx, &krate.items, &mut imports_reused_as_roots);
}
}

fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>]) {
// keep track of imports reused with `self` keyword,
fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>], mut imports_reused_as_roots: &mut Vec<Symbol>) {
// 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};
Expand All @@ -72,13 +85,14 @@ fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>]) {
cx,
&item,
&mut imports_reused_with_self,
&mut imports_reused_as_roots,
&mut single_use_usages,
&mut macros,
);
}

for single_use in &single_use_usages {
if !imports_reused_with_self.contains(&single_use.0) {
if !imports_reused_with_self.contains(&single_use.0) && !imports_reused_as_roots.contains(&single_use.0) {
let can_suggest = single_use.2;
if can_suggest {
span_lint_and_sugg(
Expand Down Expand Up @@ -108,6 +122,7 @@ fn track_uses(
cx: &EarlyContext<'_>,
item: &Item,
imports_reused_with_self: &mut Vec<Symbol>,
imports_reused_as_roots: &mut Vec<Symbol>,
single_use_usages: &mut Vec<(Symbol, Span, bool)>,
macros: &mut Vec<Symbol>,
) {
Expand All @@ -117,14 +132,13 @@ fn track_uses(

match &item.kind {
ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) => {
check_mod(cx, &items);
check_mod(cx, &items, imports_reused_as_roots);
},
ItemKind::MacroDef(MacroDef { macro_rules: true, .. }) => {
macros.push(item.ident.name);
},
ItemKind::Use(use_tree) => {
let segments = &use_tree.prefix.segments;

let should_report =
|name: &Symbol| !macros.contains(name) || matches!(item.vis.kind, VisibilityKind::Inherited);

Expand Down Expand Up @@ -155,9 +169,32 @@ fn track_uses(
}
}
} else {
// keep track of `use self::some_module` usages
if segments[0].ident.name == kw::SelfLower {
// simple case such as `use self::module::SomeStruct`
// keep track of `use {super|crate}::some_module` usages
if segments[0].ident.name == kw::Crate || segments[0].ident.name == kw::Super {
// simple cases such as `use self::module::SomeStruct`
if segments.len() > 1 {
// Get the first one that is NOT super or crate
// something like this:
// `use super::super::super::super::super::module`
for segment in segments {
if segment.ident.name != kw::Super && segment.ident.name != kw::Crate {
imports_reused_as_roots.push(segment.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_as_roots.push(segments[0].ident.name);
}
}
}
// keep track of `use self::some_module` usages
} else if segments[0].ident.name == kw::SelfLower {
// simple cases such as `use self::module::SomeStruct`
if segments.len() > 1 {
imports_reused_with_self.push(segments[1].ident.name);
return;
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/single_component_path_imports_crate_after.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// edition:2018
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use regex;

mod foo {
use crate::regex::Regex;

pub fn bar() {
Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
}
}

fn main() {
foo::bar()
}
16 changes: 16 additions & 0 deletions tests/ui/single_component_path_imports_crate_before.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// edition:2018
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use regex;

mod foo {

pub fn bar() {
regex::Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
}
}

fn main() {
foo::bar()
}
10 changes: 10 additions & 0 deletions tests/ui/single_component_path_imports_crate_before.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: this import is redundant
--> $DIR/single_component_path_imports_crate_before.rs:5: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

20 changes: 20 additions & 0 deletions tests/ui/single_component_path_imports_nested_super.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// edition:2018
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use regex;
use serde;

fn main() {}

mod root_nested_use_mod {
//use crate::regex;
mod internal_1 {
use crate::serde;
mod internal_2 {
use super::super::super::regex;
}
}
#[allow(dead_code)]
fn root_nested_use_mod() {}
}
16 changes: 16 additions & 0 deletions tests/ui/single_component_path_imports_super_after.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// edition:2018
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use regex;

mod foo {
use super::regex::Regex;
pub fn bar() {
Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
}
}

fn main() {
foo::bar()
}
16 changes: 16 additions & 0 deletions tests/ui/single_component_path_imports_super_before.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// edition:2018
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use regex;

mod foo {

pub fn bar() {
regex::Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
}
}

fn main() {
foo::bar()
}
10 changes: 10 additions & 0 deletions tests/ui/single_component_path_imports_super_before.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: this import is redundant
--> $DIR/single_component_path_imports_super_before.rs:5: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