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 authored and Allen Hsu committed Aug 2, 2022
1 parent d72e5f2 commit 171d082
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 71 deletions.
71 changes: 49 additions & 22 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,58 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
return;
}

let mut map = FxHashMap::<_, Vec<_>>::default();
for predicate in gen.predicates {
// Explanation:
// fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
// where T: Clone + Default, { unimplemented!(); }
// ^^^^^^^^^^^^^^^^^^
// |
// collects each of these where clauses into a set keyed by generic name and comparable trait
// eg. (T, Clone)
let where_predicates = gen
.predicates
.iter()
.filter_map(|pred| {
if_chain! {
if pred.in_where_clause();
if let WherePredicate::BoundPredicate(bound_predicate) = pred;
if let TyKind::Path(QPath::Resolved(_, path)) = bound_predicate.bounded_ty.kind;
then {
return Some(bound_predicate.bounds.iter().filter_map(|t| {
Some((path.res, into_comparable_trait_ref(t.trait_ref()?)))
}))
}
}
None
})
.flatten()
.collect::<FxHashSet<_>>();

// Explanation:
// fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) ...
// ^^^^^^^^^^^^^^^^^^ ^^^^^^^
// |
// compare trait bounds keyed by generic name and comparable trait to collected where
// predicates eg. (T, Clone)
for predicate in gen.predicates.iter().filter(|pred| !pred.in_where_clause()) {
if_chain! {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate;
if let WherePredicate::BoundPredicate(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 TyKind::Path(QPath::Resolved(_, path)) = bound_predicate.bounded_ty.kind;
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 171d082

Please sign in to comment.