From 92d2b019dc1b4ff79c0158bebef8d0a20af2e14e Mon Sep 17 00:00:00 2001 From: Allen Hsu Date: Thu, 14 Apr 2022 22:46:08 +1000 Subject: [PATCH] Refactor repeated where clause or trait bound. --- clippy_lints/src/trait_bounds.rs | 126 ++++++------------ ...repeated_where_clause_or_trait_bound.fixed | 4 +- .../repeated_where_clause_or_trait_bound.rs | 4 +- ...epeated_where_clause_or_trait_bound.stderr | 18 +-- 4 files changed, 55 insertions(+), 97 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 58ee829a548a..314322ef22fd 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -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; @@ -290,98 +290,60 @@ 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 repeated_spans { + if_chain! { + 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::>(); - traits.sort_unstable(); - let traits = traits.join(" + "); + let mut traits = map.values() + .filter_map(|span| snippet_opt(cx, *span)) + .collect::>(); + traits.sort_unstable(); + let traits = traits.join(" + "); - span_lint_and_sugg( - cx, - REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND, - all_trait_span, - "this trait bound contains repeated elements", - "try", - traits, - Applicability::MachineApplicable - ); + 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()) { + 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 { 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 repeated_spans { - let all_trait_span = bound_predicate - .bounds - .get(0) - .unwrap() - .span() - .to( - bound_predicate - .bounds - .iter() - .last() - .unwrap() - .span()); - - let mut traits = where_clauses.values() - .filter_map(|span| snippet_opt(cx, *span)) - .collect::>(); - 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 - ); - } + rollup_traits( + cx, + bound_predicate.bounds, + "this where clause contains repeated elements", + ); } } } diff --git a/tests/ui/repeated_where_clause_or_trait_bound.fixed b/tests/ui/repeated_where_clause_or_trait_bound.fixed index ae1bccd88996..4cef01db0a4b 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.fixed +++ b/tests/ui/repeated_where_clause_or_trait_bound.fixed @@ -1,8 +1,6 @@ // run-rustfix // -#![allow( - unused, -)] +#![allow(unused)] #![deny(clippy::repeated_where_clause_or_trait_bound)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/repeated_where_clause_or_trait_bound.rs b/tests/ui/repeated_where_clause_or_trait_bound.rs index 265dc14a893f..68c974ee5a42 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.rs +++ b/tests/ui/repeated_where_clause_or_trait_bound.rs @@ -1,8 +1,6 @@ // run-rustfix // -#![allow( - unused, -)] +#![allow(unused)] #![deny(clippy::repeated_where_clause_or_trait_bound)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/repeated_where_clause_or_trait_bound.stderr b/tests/ui/repeated_where_clause_or_trait_bound.stderr index b399c634e526..64b7c902957a 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.stderr +++ b/tests/ui/repeated_where_clause_or_trait_bound.stderr @@ -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(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 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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`