Skip to content

Commit

Permalink
Compare where predicates to trait bounds.
Browse files Browse the repository at this point in the history 
- only compare where predicates to trait bounds when generating where
  clause specific message to fix rust-lang#9151
- use comparable_trait_ref to account for trait bound generics to fix rust-lang#8757
  • Loading branch information
@aldhsu
aldhsu committed Jul 13, 2022
1 parent 07ab0e8 commit e2d27da
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 70 deletions.
56 changes: 35 additions & 21 deletions clippy_lints/src/trait_bounds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clippy_utils::{SpanlessEq, SpanlessHash};
use core::hash::{Hash, Hasher};
use if_chain::if_chain;
use itertools::Itertools;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::unhash::UnhashMap;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
Expand Down Expand Up @@ -238,31 +238,45 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
return;
}

let mut map = FxHashMap::<_, Vec<_>>::default();
for predicate in gen.predicates {
let where_predicates = gen
.predicates
.iter()
.filter_map(|pred| {
if_chain! {
if pred.in_where_clause();
if let WherePredicate::BoundPredicate(bound) = pred;
if let Ty { kind: TyKind::Path(QPath::Resolved(_, path, ..)) , ..} = bound.bounded_ty;
then {
return Some(bound.bounds.iter().filter_map(|t| {
Some((path.res, into_comparable_trait_ref(t.trait_ref()?)))
}))
}
}
None
})
.flatten()
.collect::<FxHashSet<_>>();

for predicate in gen.predicates.iter().filter(|pred| !pred.in_where_clause()) {
if_chain! {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate;
if bound_predicate.origin != PredicateOrigin::ImplTrait;
if !bound_predicate.span.from_expansion();
if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind;
if let Some(segment) = segments.first();
if let Ty { kind: TyKind::Path(QPath::Resolved(_, path, ..)) , ..} = bound_predicate.bounded_ty;
then {
for (res_where, _, span_where) in bound_predicate.bounds.iter().filter_map(get_trait_info_from_bound) {
let trait_resolutions_direct = map.entry(segment.ident).or_default();
if let Some((_, span_direct)) = trait_resolutions_direct
.iter()
.find(|(res_direct, _)| *res_direct == res_where) {
span_lint_and_help(
cx,
TRAIT_DUPLICATION_IN_BOUNDS,
*span_direct,
"this trait bound is already specified in the where clause",
None,
"consider removing this trait bound",
);
}
else {
trait_resolutions_direct.push((res_where, span_where));
for t in bound_predicate.bounds {
if let Some(trait_ref) = t.trait_ref() {
let key = (path.res, into_comparable_trait_ref(trait_ref));
if where_predicates.contains(&key) {
span_lint_and_help(
cx,
TRAIT_DUPLICATION_IN_BOUNDS,
t.span(),
"this trait bound is already specified in the where clause",
None,
"consider removing this trait bound",
);
}
}
}
}
Expand Down
50 changes: 1 addition & 49 deletions tests/ui/trait_duplication_in_bounds.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,12 @@ LL | Self: Iterator<Item = Foo>,
|
= help: consider removing this trait bound

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
|
Expand All @@ -107,14 +91,6 @@ error: these where clauses contain repeated elements
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
|
Expand All @@ -127,41 +103,17 @@ error: these where clauses contain repeated elements
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
error: aborting due to 16 previous errors

0 comments on commit e2d27da

Please sign in to comment.