Skip to content

Commit

Permalink
Support trait bounds with type parameters.
Browse files Browse the repository at this point in the history
  • Loading branch information
Allen Hsu committed May 11, 2022
1 parent a3d5dd7 commit 6040fa6
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 58 deletions.
132 changes: 80 additions & 52 deletions clippy_lints/src/trait_bounds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,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, QPath, TraitItem, Ty, TyKind, WherePredicate,
GenericArg, GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, QPath, TraitItem, TraitRef, Ty,
TyKind, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -94,7 +95,7 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.62.0"]
pub REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
pedantic,
nursery,
"Traits are repeated within trait bounds or where clause"
}

Expand Down Expand Up @@ -280,61 +281,22 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
}
}

fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
let mut map = FxHashMap::default();
let mut repeated_spans = false;
for bound in bounds.iter().filter_map(get_trait_info_from_bound) {
let (definition, _, span_direct) = bound;
if map.insert(definition, span_direct).is_some() {
repeated_spans = true;
}
}
#[derive(PartialEq, Eq, Hash, Debug)]
struct ComparableTraitRef(Res, Vec<Res>);

if_chain! {
if repeated_spans;
if let Some(first_trait) = bounds.get(0);
if let Some(last_trait) = bounds.iter().last();
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,
REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
all_trait_span,
msg,
"try",
traits,
Applicability::MachineApplicable
);
}
}
}

if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) {
fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
if gen.span.from_expansion() {
return;
}

for param in gen.params {
if let ParamName::Plain(_) = param.name {
// other alternatives are errors and elided which won't have duplicates
rollup_traits(cx, param.bounds, "this trait bound contains repeated elements");
}
}

for predicate in gen.where_clause.predicates {
for predicate in gen.predicates {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
rollup_traits(
cx,
bound_predicate.bounds,
"this where clause contains repeated elements",
);
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);
}
}
}
Expand All @@ -346,3 +308,69 @@ 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_spans = false;
for bound in bounds.iter().filter_map(|bound| {
if let GenericBound::Trait(t, _) = bound {
Some((into_comparable_trait_ref(&t.trait_ref), t.span))
} else {
None
}
}) {
let (comparable_bound, span_direct) = bound;
if map.insert(comparable_bound, span_direct).is_some() {
repeated_spans = true;
}
}

if_chain! {
if repeated_spans;
if let Some(first_trait) = bounds.get(0);
if let Some(last_trait) = bounds.iter().last();
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,
REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
all_trait_span,
msg,
"try",
traits,
Applicability::MachineApplicable
);
}
}
}
10 changes: 10 additions & 0 deletions tests/ui/repeated_where_clause_or_trait_bound.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ where

fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {}

trait GenericTrait<T> {}

fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
unimplemented!();
}

fn bad_generic<T: GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
unimplemented!();
}

fn main() {}
10 changes: 10 additions & 0 deletions tests/ui/repeated_where_clause_or_trait_bound.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ where

fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {}

trait GenericTrait<T> {}

fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
unimplemented!();
}

fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
unimplemented!();
}

fn main() {}
18 changes: 12 additions & 6 deletions tests/ui/repeated_where_clause_or_trait_bound.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: this trait bound contains repeated elements
error: these bounds contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:6:15
|
LL | fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
Expand All @@ -10,29 +10,35 @@ note: the lint level is defined here
LL | #![deny(clippy::repeated_where_clause_or_trait_bound)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this where clause contains repeated elements
error: these where clauses contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:12:8
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`

error: this where clause contains repeated elements
error: these where clauses contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:47:15
|
LL | Self: Clone + Clone + Clone;
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`

error: this trait bound contains repeated elements
error: these bounds contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:61:24
|
LL | trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`

error: this where clause contains repeated elements
error: these where clauses contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:68:12
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`

error: aborting due to 5 previous errors
error: these bounds contain repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:101:19
|
LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u32> + GenericTrait<u64>`

error: aborting due to 6 previous errors

0 comments on commit 6040fa6

Please sign in to comment.