Lint for repeated traits within trait bounds or where clauses.

This commit is contained in:
Allen Hsu 2022-04-13 00:21:08 +10:00 committed by Allen Hsu
parent 526f02ef05
commit 8878d674b1
4 changed files with 340 additions and 15 deletions

View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability};
use clippy_utils::{SpanlessEq, SpanlessHash};
use core::hash::{Hash, Hasher};
use if_chain::if_chain;
@ -9,8 +9,8 @@ use rustc_data_structures::unhash::UnhashMap;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::{
GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, TraitBoundModifier,
TraitItem, Ty, TyKind, WherePredicate,
GenericArg, GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath,
TraitBoundModifier, TraitItem, TraitRef, Ty, TyKind, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -36,7 +36,7 @@ declare_clippy_lint! {
#[clippy::version = "1.38.0"]
pub TYPE_REPETITION_IN_BOUNDS,
nursery,
"Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`"
"types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`"
}
declare_clippy_lint! {
@ -63,10 +63,26 @@ declare_clippy_lint! {
///
/// fn func<T>(arg: T) where T: Clone + Default {}
/// ```
///
/// ```rust
/// fn foo<T: Default + Default>(bar: T) {}
/// ```
/// Use instead:
/// ```rust
/// fn foo<T: Default>(bar: T) {}
/// ```
///
/// ```rust
/// fn foo<T>(bar: T) where T: Default + Default {}
/// ```
/// Use instead:
/// ```rust
/// fn foo<T>(bar: T) where T: Default {}
/// ```
#[clippy::version = "1.47.0"]
pub TRAIT_DUPLICATION_IN_BOUNDS,
nursery,
"Check if the same trait bounds are specified twice during a function declaration"
"check if the same trait bounds are specified more than once during a generic declaration"
}
#[derive(Copy, Clone)]
@ -87,6 +103,19 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) {
self.check_type_repetition(cx, gen);
check_trait_bound_duplication(cx, gen);
check_bounds_or_where_duplication(cx, gen);
}
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
// special handling for self trait bounds as these are not considered generics
// ie. trait Foo: Display {}
if let Item {
kind: ItemKind::Trait(_, _, _, bounds, ..),
..
} = item
{
rollup_traits(cx, bounds, "these bounds contain repeated elements");
}
}
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
@ -241,6 +270,26 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
}
}
#[derive(PartialEq, Eq, Hash, Debug)]
struct ComparableTraitRef(Res, Vec<Res>);
fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
if gen.span.from_expansion() {
return;
}
for predicate in gen.predicates {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
let msg = if predicate.in_where_clause() {
"these where clauses contain repeated elements"
} else {
"these bounds contain repeated elements"
};
rollup_traits(cx, bound_predicate.bounds, msg);
}
}
}
fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> {
if let GenericBound::Trait(t, tbm) = bound {
let trait_path = t.trait_ref.path;
@ -257,3 +306,71 @@ fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'
None
}
}
// FIXME: ComparableTraitRef does not support nested bounds needed for associated_type_bounds
fn into_comparable_trait_ref(trait_ref: &TraitRef<'_>) -> ComparableTraitRef {
ComparableTraitRef(
trait_ref.path.res,
trait_ref
.path
.segments
.iter()
.filter_map(|segment| {
// get trait bound type arguments
Some(segment.args?.args.iter().filter_map(|arg| {
if_chain! {
if let GenericArg::Type(ty) = arg;
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind;
then { return Some(path.res) }
}
None
}))
})
.flatten()
.collect(),
)
}
fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
let mut map = FxHashMap::default();
let mut repeated_res = false;
let only_comparable_trait_refs = |bound: &GenericBound<'_>| {
if let GenericBound::Trait(t, _) = bound {
Some((into_comparable_trait_ref(&t.trait_ref), t.span))
} else {
None
}
};
for bound in bounds.iter().filter_map(only_comparable_trait_refs) {
let (comparable_bound, span_direct) = bound;
if map.insert(comparable_bound, span_direct).is_some() {
repeated_res = true;
}
}
if_chain! {
if repeated_res;
if let [first_trait, .., last_trait] = bounds;
then {
let all_trait_span = first_trait.span().to(last_trait.span());
let mut traits = map.values()
.filter_map(|span| snippet_opt(cx, *span))
.collect::<Vec<_>>();
traits.sort_unstable();
let traits = traits.join(" + ");
span_lint_and_sugg(
cx,
TRAIT_DUPLICATION_IN_BOUNDS,
all_trait_span,
msg,
"try",
traits,
Applicability::MachineApplicable
);
}
}
}

View File

@ -397,6 +397,7 @@ const RUSTFIX_COVERAGE_KNOWN_EXCEPTIONS: &[&str] = &[
"single_component_path_imports_nested_first.rs",
"string_add.rs",
"toplevel_ref_arg_non_rustfix.rs",
"trait_duplication_in_bounds.rs",
"unit_arg.rs",
"unnecessary_clone.rs",
"unnecessary_lazy_eval_unfixable.rs",

View File

@ -1,4 +1,5 @@
#![deny(clippy::trait_duplication_in_bounds)]
#![allow(unused)]
use std::collections::BTreeMap;
use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
@ -98,4 +99,114 @@ trait FooIter: Iterator<Item = Foo> {
// This should not lint
fn impl_trait(_: impl AsRef<str>, _: impl AsRef<str>) {}
mod repeated_where_clauses_or_trait_bounds {
fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
unimplemented!();
}
fn bad_bar<T, U>(arg0: T, arg1: U)
where
T: Clone + Clone + Clone + Copy,
U: Clone + Copy,
{
unimplemented!();
}
fn good_bar<T: Clone + Copy, U: Clone + Copy>(arg0: T, arg1: U) {
unimplemented!();
}
fn good_foo<T, U>(arg0: T, arg1: U)
where
T: Clone + Copy,
U: Clone + Copy,
{
unimplemented!();
}
trait GoodSelfTraitBound: Clone + Copy {
fn f();
}
trait GoodSelfWhereClause {
fn f()
where
Self: Clone + Copy;
}
trait BadSelfTraitBound: Clone + Clone + Clone {
fn f();
}
trait BadSelfWhereClause {
fn f()
where
Self: Clone + Clone + Clone;
}
trait GoodTraitBound<T: Clone + Copy, U: Clone + Copy> {
fn f();
}
trait GoodWhereClause<T, U> {
fn f()
where
T: Clone + Copy,
U: Clone + Copy;
}
trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
fn f();
}
trait BadWhereClause<T, U> {
fn f()
where
T: Clone + Clone + Clone + Copy,
U: Clone + Copy;
}
struct GoodStructBound<T: Clone + Copy, U: Clone + Copy> {
t: T,
u: U,
}
impl<T: Clone + Copy, U: Clone + Copy> GoodTraitBound<T, U> for GoodStructBound<T, U> {
// this should not warn
fn f() {}
}
struct GoodStructWhereClause;
impl<T, U> GoodTraitBound<T, U> for GoodStructWhereClause
where
T: Clone + Copy,
U: Clone + Copy,
{
// this should not warn
fn f() {}
}
fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {}
trait GenericTrait<T> {}
// This should not warn but currently does see #8757
fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
unimplemented!();
}
fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
unimplemented!();
}
mod foo {
pub trait Clone {}
}
fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
unimplemented!();
}
}
fn main() {}

View File

@ -1,5 +1,5 @@
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:6:15
--> $DIR/trait_duplication_in_bounds.rs:7:15
|
LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
| ^^^^^
@ -12,7 +12,7 @@ LL | #![deny(clippy::trait_duplication_in_bounds)]
= help: consider removing this trait bound
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:6:23
--> $DIR/trait_duplication_in_bounds.rs:7:23
|
LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
| ^^^^^^^
@ -20,7 +20,7 @@ LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:35:15
--> $DIR/trait_duplication_in_bounds.rs:36:15
|
LL | Self: Default;
| ^^^^^^^
@ -28,7 +28,7 @@ LL | Self: Default;
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:49:15
--> $DIR/trait_duplication_in_bounds.rs:50:15
|
LL | Self: Default + Clone;
| ^^^^^^^
@ -36,7 +36,7 @@ LL | Self: Default + Clone;
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:55:15
--> $DIR/trait_duplication_in_bounds.rs:56:15
|
LL | Self: Default + Clone;
| ^^^^^^^
@ -44,7 +44,7 @@ LL | Self: Default + Clone;
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:55:25
--> $DIR/trait_duplication_in_bounds.rs:56:25
|
LL | Self: Default + Clone;
| ^^^^^
@ -52,7 +52,7 @@ LL | Self: Default + Clone;
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:58:15
--> $DIR/trait_duplication_in_bounds.rs:59:15
|
LL | Self: Default;
| ^^^^^^^
@ -60,12 +60,108 @@ LL | Self: Default;
= help: consider removing this trait bound
error: this trait bound is already specified in trait declaration
--> $DIR/trait_duplication_in_bounds.rs:93:15
--> $DIR/trait_duplication_in_bounds.rs:94:15
|
LL | Self: Iterator<Item = Foo>,
| ^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing this trait bound
error: aborting due to 8 previous errors
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:103:19
|
LL | fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
| ^^^^^
|
= help: consider removing this trait bound
error: these bounds contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:103:19
|
LL | fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:109:12
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^
|
= help: consider removing this trait bound
error: these where clauses contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:109:12
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
error: these bounds contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:137:30
|
LL | trait BadSelfTraitBound: Clone + Clone + Clone {
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
error: these where clauses contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:144:19
|
LL | Self: Clone + Clone + Clone;
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:158:28
|
LL | trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
| ^^^^^
|
= help: consider removing this trait bound
error: these bounds contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:158:28
|
LL | trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
error: these where clauses contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:165:16
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:195:24
|
LL | fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
| ^^^^^^^^^^^^^^^^^
|
= help: consider removing this trait bound
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:199:23
|
LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
| ^^^^^^^^^^^^^^^^^
|
= help: consider removing this trait bound
error: these bounds contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:199:23
|
LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u32> + GenericTrait<u64>`
error: this trait bound is already specified in the where clause
--> $DIR/trait_duplication_in_bounds.rs:207:26
|
LL | fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
| ^^^^^^^^^^^^^^^^^
|
= help: consider removing this trait bound
error: these bounds contain repeated elements
--> $DIR/trait_duplication_in_bounds.rs:207:26
|
LL | fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + foo::Clone`
error: aborting due to 22 previous errors