-
Notifications
You must be signed in to change notification settings - Fork 1.7k
new lint: needless traits in scope #10503
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
Changes from 1 commit
fb4bacb
2e082ba
02db985
ef7fac2
a6e854e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
use clippy_utils::diagnostics::span_lint_and_sugg; | ||
use rustc_errors::Applicability; | ||
use rustc_hir::*; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
|
||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// Find traits that are not explicitely used in scope (like `AsRef::as_ref()`) | ||
/// but directly with the trait method (like `opt.as_ref()`). | ||
/// | ||
/// These traits can be imported anonymously with `use crate::Trait as _`. | ||
/// This avoids name collision with other traits (possibly with the same name). | ||
/// It also helps identify the traits in `use` statements. | ||
/// | ||
/// ### Why is this bad? | ||
/// This needlessly brings a trait in trait's namespace. | ||
/// | ||
/// ### Example | ||
/// ```rust | ||
/// use std::io::Read; | ||
/// fn main() { | ||
/// let mut b = "I'm your father!".as_bytes(); | ||
/// let mut buffer = [0; 10]; | ||
/// b.read(&mut buffer) | ||
/// } | ||
/// ``` | ||
/// Use instead: | ||
/// ```rust | ||
/// use std::io::Read as _; | ||
/// fn main() { | ||
/// let mut b = "I'm your father!".as_bytes(); | ||
/// let mut buffer = [0; 10]; | ||
/// b.read(&mut buffer) | ||
/// } | ||
/// ``` | ||
#[clippy::version = "1.69.0"] | ||
pub NEEDLESS_TRAITS_IN_SCOPE, | ||
pedantic, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be a restriction lint. It's nothing that should be frowned upon in normal Rust code and makes the code longer, but it could bring benefits in certain situations (those you mentioned). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, better than |
||
"trait is needlessly imported in trait's namespace, and can be anonymously imported" | ||
woshilapin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
declare_lint_pass!(NeedlessTraitsInScope => [NEEDLESS_TRAITS_IN_SCOPE]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll want to write your own struct so you can |
||
|
||
impl<'tcx> LateLintPass<'tcx> for NeedlessTraitsInScope { | ||
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { | ||
// Only process `use` statements, ignore `UseKind::Glob` | ||
let ItemKind::Use(use_path, UseKind::Single) = item.kind else { | ||
return | ||
}; | ||
// Check if the `use` is aliased with ` as `. | ||
// If aliased, then do not process, it's probably for a good reason | ||
if item.ident != use_path.segments.last().unwrap().ident { | ||
return; | ||
} | ||
let path = use_path | ||
.segments | ||
.iter() | ||
.map(|segment| segment.ident) | ||
.fold(String::new(), |mut acc, ident| { | ||
if !acc.is_empty() { | ||
acc += "::"; | ||
} | ||
acc += ident.as_str(); | ||
acc | ||
}); | ||
span_lint_and_sugg( | ||
cx, | ||
NEEDLESS_TRAITS_IN_SCOPE, | ||
use_path.span, | ||
"trait is needlessly imported in trait's namespace", | ||
woshilapin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"you can import the trait anonymously", | ||
format!("{path} as _"), | ||
Applicability::MachineApplicable, | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// run-rustfix | ||
#![allow(unused)] | ||
#![warn(clippy::needless_traits_in_scope)] | ||
|
||
pub mod useless_trait_in_scope { | ||
use std::io::Read as _; | ||
|
||
pub fn warn() -> std::io::Result<usize> { | ||
let mut b = "The trait is not used explicitely -> 'use std::io::Read' doesn't need to be in scope".as_bytes(); | ||
let mut buffer = [0; 10]; | ||
b.read(&mut buffer) | ||
} | ||
} | ||
|
||
pub mod trait_not_in_scope { | ||
use std::io::Read as _; | ||
|
||
pub fn ok() -> std::io::Result<usize> { | ||
let mut b = "The trait is not used explicitely, but 'use std::io::Read' is already not in scope".as_bytes(); | ||
let mut buffer = [0; 10]; | ||
b.read(&mut buffer) | ||
} | ||
} | ||
|
||
// FIXME: when the trait is explicitely used, the lint should not trigger | ||
// pub mod useful_trait_in_scope { | ||
// use std::io::Read; | ||
|
||
// pub fn ok() -> std::io::Result<usize> { | ||
// let mut b = "Trait is used explicitely -> 'use std::io::Read' is OK".as_bytes(); | ||
// let mut buffer = [0; 10]; | ||
// Read::read(&mut b, &mut buffer) | ||
// } | ||
// } | ||
|
||
fn main() { | ||
useless_trait_in_scope::warn(); | ||
trait_not_in_scope::ok(); | ||
// useful_trait_in_scope::ok(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// run-rustfix | ||
#![allow(unused)] | ||
#![warn(clippy::needless_traits_in_scope)] | ||
|
||
pub mod useless_trait_in_scope { | ||
use std::io::Read; | ||
|
||
pub fn warn() -> std::io::Result<usize> { | ||
let mut b = "The trait is not used explicitely -> 'use std::io::Read' doesn't need to be in scope".as_bytes(); | ||
let mut buffer = [0; 10]; | ||
b.read(&mut buffer) | ||
} | ||
} | ||
|
||
pub mod trait_not_in_scope { | ||
use std::io::Read as _; | ||
|
||
pub fn ok() -> std::io::Result<usize> { | ||
let mut b = "The trait is not used explicitely, but 'use std::io::Read' is already not in scope".as_bytes(); | ||
let mut buffer = [0; 10]; | ||
b.read(&mut buffer) | ||
} | ||
} | ||
|
||
// FIXME: when the trait is explicitely used, the lint should not trigger | ||
// pub mod useful_trait_in_scope { | ||
// use std::io::Read; | ||
|
||
// pub fn ok() -> std::io::Result<usize> { | ||
// let mut b = "Trait is used explicitely -> 'use std::io::Read' is OK".as_bytes(); | ||
// let mut buffer = [0; 10]; | ||
// Read::read(&mut b, &mut buffer) | ||
// } | ||
// } | ||
|
||
fn main() { | ||
useless_trait_in_scope::warn(); | ||
trait_not_in_scope::ok(); | ||
// useful_trait_in_scope::ok(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
error: trait is needlessly imported in trait's namespace | ||
--> $DIR/needless_traits_in_scope.rs:6:9 | ||
| | ||
LL | use std::io::Read; | ||
| ^^^^^^^^^^^^^ help: you can import the trait anonymously: `std::io::Read as _` | ||
| | ||
= note: `-D clippy::needless-traits-in-scope` implied by `-D warnings` | ||
|
||
error: aborting due to previous error | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in ef7fac2, thank you for the improvement.