diff options
| -rw-r--r-- | clippy_lints/src/trait_bounds.rs | 34 | ||||
| -rw-r--r-- | tests/ui/trait_duplication_in_bounds.fixed | 4 | ||||
| -rw-r--r-- | tests/ui/trait_duplication_in_bounds.stderr | 4 | ||||
| -rw-r--r-- | tests/ui/trait_duplication_in_bounds_unfixable.stderr | 8 |
4 files changed, 33 insertions, 17 deletions
diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 0434720f79b..2ffa022b04f 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -15,6 +15,7 @@ use rustc_hir::{ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{BytePos, Span}; +use std::collections::hash_map::Entry; declare_clippy_lint! { /// ### What it does @@ -255,7 +256,7 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { then { return Some( rollup_traits(cx, bound_predicate.bounds, "these where clauses contain repeated elements") - .into_keys().map(|trait_ref| (path.res, trait_ref))) + .into_iter().map(|(trait_ref, _)| (path.res, trait_ref))) } } None @@ -295,8 +296,13 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { } } -#[derive(PartialEq, Eq, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, Hash, Debug)] struct ComparableTraitRef(Res, Vec<Res>); +impl Default for ComparableTraitRef { + fn default() -> Self { + Self(Res::Err, Vec::new()) + } +} fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> { if let GenericBound::Trait(t, tbm) = bound { @@ -339,7 +345,7 @@ fn into_comparable_trait_ref(trait_ref: &TraitRef<'_>) -> ComparableTraitRef { ) } -fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) -> FxHashMap<ComparableTraitRef, Span> { +fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) -> Vec<(ComparableTraitRef, Span)> { let mut map = FxHashMap::default(); let mut repeated_res = false; @@ -351,23 +357,33 @@ fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) - } }; + let mut i = 0usize; for bound in bounds.iter().filter_map(only_comparable_trait_refs) { let (comparable_bound, span_direct) = bound; - if map.insert(comparable_bound, span_direct).is_some() { - repeated_res = true; + match map.entry(comparable_bound) { + Entry::Occupied(_) => repeated_res = true, + Entry::Vacant(e) => { + e.insert((span_direct, i)); + i += 1; + }, } } + // Put bounds in source order + let mut comparable_bounds = vec![Default::default(); map.len()]; + for (k, (v, i)) in map { + comparable_bounds[i] = (k, v); + } + if_chain! { if repeated_res; if let [first_trait, .., last_trait] = bounds; then { let all_trait_span = first_trait.span().to(last_trait.span()); - let mut traits = map.values() - .filter_map(|span| snippet_opt(cx, *span)) + let traits = comparable_bounds.iter() + .filter_map(|&(_, span)| snippet_opt(cx, span)) .collect::<Vec<_>>(); - traits.sort_unstable(); let traits = traits.join(" + "); span_lint_and_sugg( @@ -382,5 +398,5 @@ fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) - } } - map + comparable_bounds } diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index b4e6bf0ea1c..4ce5d421782 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/tests/ui/trait_duplication_in_bounds.fixed @@ -97,7 +97,7 @@ fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) { unimplemented!(); } -fn bad_generic<T: GenericTrait<u32> + GenericTrait<u64>>(arg0: T) { +fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) { unimplemented!(); } @@ -105,7 +105,7 @@ mod foo { pub trait Clone {} } -fn qualified_path<T: Clone + foo::Clone>(arg0: T) { +fn qualified_path<T: std::clone::Clone + foo::Clone>(arg0: T) { unimplemented!(); } diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index 86c593811a7..af800ba7888 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/tests/ui/trait_duplication_in_bounds.stderr @@ -44,13 +44,13 @@ error: these bounds contain repeated elements --> $DIR/trait_duplication_in_bounds.rs:100:19 | LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u32> + GenericTrait<u64>` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u64> + GenericTrait<u32>` error: these bounds contain repeated elements --> $DIR/trait_duplication_in_bounds.rs:108:22 | LL | fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + foo::Clone` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::clone::Clone + foo::Clone` error: aborting due to 8 previous errors diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.stderr b/tests/ui/trait_duplication_in_bounds_unfixable.stderr index aa44114eb6c..fbd9abb005f 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.stderr +++ b/tests/ui/trait_duplication_in_bounds_unfixable.stderr @@ -1,8 +1,8 @@ error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:23 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:15 | LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) - | ^^^^^^^ + | ^^^^^ | note: the lint level is defined here --> $DIR/trait_duplication_in_bounds_unfixable.rs:1:9 @@ -12,10 +12,10 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] = help: consider removing this trait bound error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:23 | LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) - | ^^^^^ + | ^^^^^^^ | = help: consider removing this trait bound |