Skip to content

Commit

Permalink
Refactor repeated where clause or trait bound.
Browse files Browse the repository at this point in the history
  • Loading branch information
Allen Hsu committed May 7, 2022
1 parent a40b035 commit a3d5dd7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 84 deletions.
100 changes: 31 additions & 69 deletions clippy_lints/src/trait_bounds.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_help};
use clippy_utils::source::{snippet, snippet_with_applicability, snippet_opt};
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;
Expand Down Expand Up @@ -281,34 +281,22 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
}

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

for param in gen.params {
fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
let mut map = FxHashMap::default();
let mut repeated_spans = false;
if let ParamName::Plain(name) = param.name { // other alternatives are errors and elided which won't have duplicates
for bound in param.bounds.iter().filter_map(get_trait_info_from_bound) {
let (definition, _, span_direct) = bound;
if let Some(_) = map.insert(definition, span_direct) {
repeated_spans = true;
}
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;
}
}

if repeated_spans {
let all_trait_span = param
.bounds
.get(0)
.unwrap()
.span()
.to(
param
.bounds
.iter()
.last()
.unwrap()
.span());
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))
Expand All @@ -320,7 +308,7 @@ fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>
cx,
REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
all_trait_span,
"this trait bound contains repeated elements",
msg,
"try",
traits,
Applicability::MachineApplicable
Expand All @@ -329,50 +317,24 @@ fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>
}
}

for predicate in gen.where_clause.predicates {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
let mut where_clauses = FxHashMap::default();
let mut repeated_spans = false;

for (definition, _, span_direct) in bound_predicate
.bounds
.iter()
.filter_map(get_trait_info_from_bound)
{
if let Some(_) = where_clauses.insert(definition, span_direct) {
repeated_spans = true;
}
}
if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) {
return;
}

if repeated_spans {
let all_trait_span = bound_predicate
.bounds
.get(0)
.unwrap()
.span()
.to(
bound_predicate
.bounds
.iter()
.last()
.unwrap()
.span());
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");
}
}

let mut traits = where_clauses.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,
"this where clause has already been specified",
"try",
traits,
Applicability::MachineApplicable
);
}
for predicate in gen.where_clause.predicates {
if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
rollup_traits(
cx,
bound_predicate.bounds,
"this where clause contains repeated elements",
);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/repeated_where_clause_or_trait_bound.fixed
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// run-rustfix
//
#![allow(
unused,
)]
#![allow(unused)]
#![deny(clippy::repeated_where_clause_or_trait_bound)]

fn bad_foo<T: Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/repeated_where_clause_or_trait_bound.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// run-rustfix
//
#![allow(
unused,
)]
#![allow(unused)]
#![deny(clippy::repeated_where_clause_or_trait_bound)]

fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/repeated_where_clause_or_trait_bound.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
error: this trait bound contains repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:8:15
--> $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) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
|
note: the lint level is defined here
--> $DIR/repeated_where_clause_or_trait_bound.rs:6:9
--> $DIR/repeated_where_clause_or_trait_bound.rs:4:9
|
LL | #![deny(clippy::repeated_where_clause_or_trait_bound)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this where clause has already been specified
--> $DIR/repeated_where_clause_or_trait_bound.rs:14:8
error: this where clause contains 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 has already been specified
--> $DIR/repeated_where_clause_or_trait_bound.rs:49:15
error: this where clause contains 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
--> $DIR/repeated_where_clause_or_trait_bound.rs:63:24
--> $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 has already been specified
--> $DIR/repeated_where_clause_or_trait_bound.rs:70:12
error: this where clause contains repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:68:12
|
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
Expand Down

0 comments on commit a3d5dd7

Please sign in to comment.