Skip to content

Commit 60d3597

Browse files
borsbors
committed
Auto merge of rust-lang#8315 - dswij:8306, r=giraffate
`trait_duplication_in_bounds` checks path segments for trait items closes rust-lang#8306 changelog: [`trait_duplication_in_bounds`] Fix FP when path segments exists for trait items
2 parents d976d8a + 4c1549e commit 60d3597

File tree

3 files changed

+68
-36
lines changed

3 files changed

+68
-36
lines changed

‎clippy_lints/src/trait_bounds.rs

+30-28
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability};
33
use clippy_utils::{SpanlessEq, SpanlessHash};
44
use core::hash::{Hash, Hasher};
55
use if_chain::if_chain;
6-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
6+
use rustc_data_structures::fx::FxHashMap;
77
use rustc_data_structures::unhash::UnhashMap;
88
use rustc_errors::Applicability;
99
use rustc_hir::def::Res;
@@ -91,7 +91,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
9191

9292
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
9393
let Generics { where_clause, .. } = &item.generics;
94-
let mut self_bounds_set = FxHashSet::default();
94+
let mut self_bounds_map = FxHashMap::default();
9595

9696
for predicate in where_clause.predicates {
9797
if_chain! {
@@ -108,27 +108,29 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
108108
)
109109
) = cx.tcx.hir().get_if_local(*def_id);
110110
then {
111-
if self_bounds_set.is_empty() {
111+
if self_bounds_map.is_empty() {
112112
for bound in self_bounds.iter() {
113-
let Some((self_res, _)) = get_trait_res_span_from_bound(bound) else { continue };
114-
self_bounds_set.insert(self_res);
113+
let Some((self_res, self_segments, _)) = get_trait_info_from_bound(bound) else { continue };
114+
self_bounds_map.insert(self_res, self_segments);
115115
}
116116
}
117117

118118
bound_predicate
119119
.bounds
120120
.iter()
121-
.filter_map(get_trait_res_span_from_bound)
122-
.for_each(|(trait_item_res, span)| {
123-
if self_bounds_set.get(&trait_item_res).is_some() {
124-
span_lint_and_help(
125-
cx,
126-
TRAIT_DUPLICATION_IN_BOUNDS,
127-
span,
128-
"this trait bound is already specified in trait declaration",
129-
None,
130-
"consider removing this trait bound",
131-
);
121+
.filter_map(get_trait_info_from_bound)
122+
.for_each(|(trait_item_res, trait_item_segments, span)| {
123+
if let Some(self_segments) = self_bounds_map.get(&trait_item_res) {
124+
if SpanlessEq::new(cx).eq_path_segments(self_segments, trait_item_segments) {
125+
span_lint_and_help(
126+
cx,
127+
TRAIT_DUPLICATION_IN_BOUNDS,
128+
span,
129+
"this trait bound is already specified in trait declaration",
130+
None,
131+
"consider removing this trait bound",
132+
);
133+
}
132134
}
133135
});
134136
}
@@ -137,14 +139,6 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
137139
}
138140
}
139141

140-
fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> {
141-
if let GenericBound::Trait(t, _) = bound {
142-
Some((t.trait_ref.path.res, t.span))
143-
} else {
144-
None
145-
}
146-
}
147-
148142
impl TraitBounds {
149143
fn check_type_repetition<'tcx>(self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) {
150144
struct SpanlessTy<'cx, 'tcx> {
@@ -231,7 +225,7 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
231225
let res = param
232226
.bounds
233227
.iter()
234-
.filter_map(get_trait_res_span_from_bound)
228+
.filter_map(get_trait_info_from_bound)
235229
.collect::<Vec<_>>();
236230
map.insert(*ident, res);
237231
}
@@ -245,10 +239,10 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
245239
if let Some(segment) = segments.first();
246240
if let Some(trait_resolutions_direct) = map.get(&segment.ident);
247241
then {
248-
for (res_where, _) in bound_predicate.bounds.iter().filter_map(get_trait_res_span_from_bound) {
249-
if let Some((_, span_direct)) = trait_resolutions_direct
242+
for (res_where, _, _) in bound_predicate.bounds.iter().filter_map(get_trait_info_from_bound) {
243+
if let Some((_, _, span_direct)) = trait_resolutions_direct
250244
.iter()
251-
.find(|(res_direct, _)| *res_direct == res_where) {
245+
.find(|(res_direct, _, _)| *res_direct == res_where) {
252246
span_lint_and_help(
253247
cx,
254248
TRAIT_DUPLICATION_IN_BOUNDS,
@@ -263,3 +257,11 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
263257
}
264258
}
265259
}
260+
261+
fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> {
262+
if let GenericBound::Trait(t, _) = bound {
263+
Some((t.trait_ref.path.res, t.trait_ref.path.segments, t.span))
264+
} else {
265+
None
266+
}
267+
}

‎tests/ui/trait_duplication_in_bounds.rs

+22
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![deny(clippy::trait_duplication_in_bounds)]
22

3+
use std::collections::BTreeMap;
34
use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
45

56
fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
@@ -73,4 +74,25 @@ impl U for Life {
7374
fn f() {}
7475
}
7576

77+
// should not warn
78+
trait Iter: Iterator {
79+
fn into_group_btreemap<K, V>(self) -> BTreeMap<K, Vec<V>>
80+
where
81+
Self: Iterator<Item = (K, V)> + Sized,
82+
K: Ord + Eq,
83+
{
84+
unimplemented!();
85+
}
86+
}
87+
88+
struct Foo {}
89+
90+
trait FooIter: Iterator<Item = Foo> {
91+
fn bar()
92+
where
93+
Self: Iterator<Item = Foo>,
94+
{
95+
}
96+
}
97+
7698
fn main() {}

‎tests/ui/trait_duplication_in_bounds.stderr

+16-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this trait bound is already specified in the where clause
2-
--> $DIR/trait_duplication_in_bounds.rs:5:15
2+
--> $DIR/trait_duplication_in_bounds.rs:6:15
33
|
44
LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
55
| ^^^^^
@@ -12,52 +12,60 @@ LL | #![deny(clippy::trait_duplication_in_bounds)]
1212
= help: consider removing this trait bound
1313

1414
error: this trait bound is already specified in the where clause
15-
--> $DIR/trait_duplication_in_bounds.rs:5:23
15+
--> $DIR/trait_duplication_in_bounds.rs:6:23
1616
|
1717
LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
1818
| ^^^^^^^
1919
|
2020
= help: consider removing this trait bound
2121

2222
error: this trait bound is already specified in trait declaration
23-
--> $DIR/trait_duplication_in_bounds.rs:34:15
23+
--> $DIR/trait_duplication_in_bounds.rs:35:15
2424
|
2525
LL | Self: Default;
2626
| ^^^^^^^
2727
|
2828
= help: consider removing this trait bound
2929

3030
error: this trait bound is already specified in trait declaration
31-
--> $DIR/trait_duplication_in_bounds.rs:48:15
31+
--> $DIR/trait_duplication_in_bounds.rs:49:15
3232
|
3333
LL | Self: Default + Clone;
3434
| ^^^^^^^
3535
|
3636
= help: consider removing this trait bound
3737

3838
error: this trait bound is already specified in trait declaration
39-
--> $DIR/trait_duplication_in_bounds.rs:54:15
39+
--> $DIR/trait_duplication_in_bounds.rs:55:15
4040
|
4141
LL | Self: Default + Clone;
4242
| ^^^^^^^
4343
|
4444
= help: consider removing this trait bound
4545

4646
error: this trait bound is already specified in trait declaration
47-
--> $DIR/trait_duplication_in_bounds.rs:54:25
47+
--> $DIR/trait_duplication_in_bounds.rs:55:25
4848
|
4949
LL | Self: Default + Clone;
5050
| ^^^^^
5151
|
5252
= help: consider removing this trait bound
5353

5454
error: this trait bound is already specified in trait declaration
55-
--> $DIR/trait_duplication_in_bounds.rs:57:15
55+
--> $DIR/trait_duplication_in_bounds.rs:58:15
5656
|
5757
LL | Self: Default;
5858
| ^^^^^^^
5959
|
6060
= help: consider removing this trait bound
6161

62-
error: aborting due to 7 previous errors
62+
error: this trait bound is already specified in trait declaration
63+
--> $DIR/trait_duplication_in_bounds.rs:93:15
64+
|
65+
LL | Self: Iterator<Item = Foo>,
66+
| ^^^^^^^^^^^^^^^^^^^^
67+
|
68+
= help: consider removing this trait bound
69+
70+
error: aborting due to 8 previous errors
6371

0 commit comments

Comments
 (0)