]>
Commit | Line | Data |
---|---|---|
064997fb FG |
1 | use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; |
2 | use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; | |
5099ac24 FG |
3 | use clippy_utils::{SpanlessEq, SpanlessHash}; |
4 | use core::hash::{Hash, Hasher}; | |
f20569fa | 5 | use if_chain::if_chain; |
064997fb | 6 | use itertools::Itertools; |
f2b60f7d | 7 | use rustc_data_structures::fx::{FxHashMap, FxHashSet}; |
17df50a5 | 8 | use rustc_data_structures::unhash::UnhashMap; |
f20569fa | 9 | use rustc_errors::Applicability; |
5099ac24 FG |
10 | use rustc_hir::def::Res; |
11 | use rustc_hir::{ | |
064997fb FG |
12 | GenericArg, GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, |
13 | TraitBoundModifier, TraitItem, TraitRef, Ty, TyKind, WherePredicate, | |
5099ac24 | 14 | }; |
f20569fa XL |
15 | use rustc_lint::{LateContext, LateLintPass}; |
16 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
064997fb | 17 | use rustc_span::{BytePos, Span}; |
f2b60f7d | 18 | use std::collections::hash_map::Entry; |
f20569fa XL |
19 | |
20 | declare_clippy_lint! { | |
94222f64 XL |
21 | /// ### What it does |
22 | /// This lint warns about unnecessary type repetitions in trait bounds | |
f20569fa | 23 | /// |
94222f64 XL |
24 | /// ### Why is this bad? |
25 | /// Repeating the type for every bound makes the code | |
f20569fa XL |
26 | /// less readable than combining the bounds |
27 | /// | |
94222f64 | 28 | /// ### Example |
f20569fa XL |
29 | /// ```rust |
30 | /// pub fn foo<T>(t: T) where T: Copy, T: Clone {} | |
31 | /// ``` | |
32 | /// | |
923072b8 | 33 | /// Use instead: |
f20569fa XL |
34 | /// ```rust |
35 | /// pub fn foo<T>(t: T) where T: Copy + Clone {} | |
36 | /// ``` | |
a2a8927a | 37 | #[clippy::version = "1.38.0"] |
f20569fa | 38 | pub TYPE_REPETITION_IN_BOUNDS, |
064997fb | 39 | nursery, |
49aad941 | 40 | "types are repeated unnecessarily in trait bounds, use `+` instead of using `T: _, T: _`" |
f20569fa XL |
41 | } |
42 | ||
43 | declare_clippy_lint! { | |
94222f64 | 44 | /// ### What it does |
49aad941 | 45 | /// Checks for cases where generics or trait objects are being used and multiple |
f20569fa XL |
46 | /// syntax specifications for trait bounds are used simultaneously. |
47 | /// | |
94222f64 XL |
48 | /// ### Why is this bad? |
49 | /// Duplicate bounds makes the code | |
5e7ed085 | 50 | /// less readable than specifying them only once. |
f20569fa | 51 | /// |
94222f64 | 52 | /// ### Example |
f20569fa XL |
53 | /// ```rust |
54 | /// fn func<T: Clone + Default>(arg: T) where T: Clone + Default {} | |
55 | /// ``` | |
56 | /// | |
923072b8 | 57 | /// Use instead: |
f20569fa | 58 | /// ```rust |
923072b8 | 59 | /// # mod hidden { |
f20569fa | 60 | /// fn func<T: Clone + Default>(arg: T) {} |
923072b8 FG |
61 | /// # } |
62 | /// | |
63 | /// // or | |
f20569fa | 64 | /// |
f20569fa XL |
65 | /// fn func<T>(arg: T) where T: Clone + Default {} |
66 | /// ``` | |
064997fb FG |
67 | /// |
68 | /// ```rust | |
69 | /// fn foo<T: Default + Default>(bar: T) {} | |
70 | /// ``` | |
71 | /// Use instead: | |
72 | /// ```rust | |
73 | /// fn foo<T: Default>(bar: T) {} | |
74 | /// ``` | |
75 | /// | |
76 | /// ```rust | |
77 | /// fn foo<T>(bar: T) where T: Default + Default {} | |
78 | /// ``` | |
79 | /// Use instead: | |
80 | /// ```rust | |
81 | /// fn foo<T>(bar: T) where T: Default {} | |
82 | /// ``` | |
a2a8927a | 83 | #[clippy::version = "1.47.0"] |
f20569fa | 84 | pub TRAIT_DUPLICATION_IN_BOUNDS, |
064997fb FG |
85 | nursery, |
86 | "check if the same trait bounds are specified more than once during a generic declaration" | |
f20569fa XL |
87 | } |
88 | ||
89 | #[derive(Copy, Clone)] | |
90 | pub struct TraitBounds { | |
91 | max_trait_bounds: u64, | |
92 | } | |
93 | ||
94 | impl TraitBounds { | |
95 | #[must_use] | |
96 | pub fn new(max_trait_bounds: u64) -> Self { | |
97 | Self { max_trait_bounds } | |
98 | } | |
99 | } | |
100 | ||
101 | impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]); | |
102 | ||
103 | impl<'tcx> LateLintPass<'tcx> for TraitBounds { | |
104 | fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { | |
105 | self.check_type_repetition(cx, gen); | |
106 | check_trait_bound_duplication(cx, gen); | |
064997fb FG |
107 | } |
108 | ||
109 | fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { | |
110 | // special handling for self trait bounds as these are not considered generics | |
111 | // ie. trait Foo: Display {} | |
112 | if let Item { | |
113 | kind: ItemKind::Trait(_, _, _, bounds, ..), | |
114 | .. | |
115 | } = item | |
116 | { | |
117 | rollup_traits(cx, bounds, "these bounds contain repeated elements"); | |
118 | } | |
f20569fa | 119 | } |
f20569fa | 120 | |
5099ac24 | 121 | fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) { |
5099ac24 FG |
122 | let mut self_bounds_map = FxHashMap::default(); |
123 | ||
04454e1e | 124 | for predicate in item.generics.predicates { |
5099ac24 FG |
125 | if_chain! { |
126 | if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate; | |
04454e1e | 127 | if bound_predicate.origin != PredicateOrigin::ImplTrait; |
5099ac24 FG |
128 | if !bound_predicate.span.from_expansion(); |
129 | if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind; | |
5e7ed085 | 130 | if let Some(PathSegment { |
2b03887a | 131 | res: Res::SelfTyParam { trait_: def_id }, .. |
5e7ed085 | 132 | }) = segments.first(); |
5099ac24 FG |
133 | if let Some( |
134 | Node::Item( | |
135 | Item { | |
136 | kind: ItemKind::Trait(_, _, _, self_bounds, _), | |
137 | .. } | |
138 | ) | |
139 | ) = cx.tcx.hir().get_if_local(*def_id); | |
140 | then { | |
141 | if self_bounds_map.is_empty() { | |
142 | for bound in self_bounds.iter() { | |
143 | let Some((self_res, self_segments, _)) = get_trait_info_from_bound(bound) else { continue }; | |
144 | self_bounds_map.insert(self_res, self_segments); | |
145 | } | |
146 | } | |
147 | ||
148 | bound_predicate | |
149 | .bounds | |
150 | .iter() | |
151 | .filter_map(get_trait_info_from_bound) | |
152 | .for_each(|(trait_item_res, trait_item_segments, span)| { | |
153 | if let Some(self_segments) = self_bounds_map.get(&trait_item_res) { | |
154 | if SpanlessEq::new(cx).eq_path_segments(self_segments, trait_item_segments) { | |
155 | span_lint_and_help( | |
156 | cx, | |
157 | TRAIT_DUPLICATION_IN_BOUNDS, | |
158 | span, | |
159 | "this trait bound is already specified in trait declaration", | |
160 | None, | |
161 | "consider removing this trait bound", | |
162 | ); | |
163 | } | |
164 | } | |
165 | }); | |
166 | } | |
167 | } | |
168 | } | |
f20569fa | 169 | } |
49aad941 FG |
170 | |
171 | fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { | |
172 | if_chain! { | |
173 | if let TyKind::Ref(.., mut_ty) = &ty.kind; | |
174 | if let TyKind::TraitObject(bounds, ..) = mut_ty.ty.kind; | |
175 | if bounds.len() > 2; | |
176 | then { | |
177 | ||
178 | // Build up a hash of every trait we've seen | |
179 | // When we see a trait for the first time, add it to unique_traits | |
180 | // so we can later use it to build a string of all traits exactly once, without duplicates | |
181 | ||
182 | let mut seen_def_ids = FxHashSet::default(); | |
183 | let mut unique_traits = Vec::new(); | |
184 | ||
185 | // Iterate the bounds and add them to our seen hash | |
186 | // If we haven't yet seen it, add it to the fixed traits | |
187 | for bound in bounds.iter() { | |
188 | let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; }; | |
189 | ||
190 | let new_trait = seen_def_ids.insert(def_id); | |
191 | ||
192 | if new_trait { | |
193 | unique_traits.push(bound); | |
194 | } | |
195 | } | |
196 | ||
197 | // If the number of unique traits isn't the same as the number of traits in the bounds, | |
198 | // there must be 1 or more duplicates | |
199 | if bounds.len() != unique_traits.len() { | |
200 | let mut bounds_span = bounds[0].span; | |
201 | ||
202 | for bound in bounds.iter().skip(1) { | |
203 | bounds_span = bounds_span.to(bound.span); | |
204 | } | |
205 | ||
206 | let fixed_trait_snippet = unique_traits | |
207 | .iter() | |
208 | .filter_map(|b| snippet_opt(cx, b.span)) | |
209 | .collect::<Vec<_>>() | |
210 | .join(" + "); | |
211 | ||
212 | span_lint_and_sugg( | |
213 | cx, | |
214 | TRAIT_DUPLICATION_IN_BOUNDS, | |
215 | bounds_span, | |
216 | "this trait bound is already specified in trait declaration", | |
217 | "try", | |
218 | fixed_trait_snippet, | |
219 | Applicability::MaybeIncorrect, | |
220 | ); | |
221 | } | |
222 | } | |
223 | } | |
224 | } | |
f20569fa XL |
225 | } |
226 | ||
227 | impl TraitBounds { | |
5099ac24 FG |
228 | fn check_type_repetition<'tcx>(self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { |
229 | struct SpanlessTy<'cx, 'tcx> { | |
230 | ty: &'tcx Ty<'tcx>, | |
231 | cx: &'cx LateContext<'tcx>, | |
232 | } | |
233 | impl PartialEq for SpanlessTy<'_, '_> { | |
234 | fn eq(&self, other: &Self) -> bool { | |
235 | let mut eq = SpanlessEq::new(self.cx); | |
236 | eq.inter_expr().eq_ty(self.ty, other.ty) | |
237 | } | |
238 | } | |
239 | impl Hash for SpanlessTy<'_, '_> { | |
240 | fn hash<H: Hasher>(&self, h: &mut H) { | |
241 | let mut t = SpanlessHash::new(self.cx); | |
242 | t.hash_ty(self.ty); | |
243 | h.write_u64(t.finish()); | |
244 | } | |
245 | } | |
246 | impl Eq for SpanlessTy<'_, '_> {} | |
247 | ||
a2a8927a | 248 | if gen.span.from_expansion() { |
f20569fa XL |
249 | return; |
250 | } | |
5099ac24 | 251 | let mut map: UnhashMap<SpanlessTy<'_, '_>, Vec<&GenericBound<'_>>> = UnhashMap::default(); |
f20569fa | 252 | let mut applicability = Applicability::MaybeIncorrect; |
04454e1e | 253 | for bound in gen.predicates { |
f20569fa XL |
254 | if_chain! { |
255 | if let WherePredicate::BoundPredicate(ref p) = bound; | |
04454e1e | 256 | if p.origin != PredicateOrigin::ImplTrait; |
f20569fa | 257 | if p.bounds.len() as u64 <= self.max_trait_bounds; |
a2a8927a | 258 | if !p.span.from_expansion(); |
5099ac24 FG |
259 | if let Some(ref v) = map.insert( |
260 | SpanlessTy { ty: p.bounded_ty, cx }, | |
261 | p.bounds.iter().collect::<Vec<_>>() | |
262 | ); | |
f20569fa XL |
263 | |
264 | then { | |
064997fb FG |
265 | let trait_bounds = v |
266 | .iter() | |
267 | .copied() | |
268 | .chain(p.bounds.iter()) | |
269 | .filter_map(get_trait_info_from_bound) | |
270 | .map(|(_, _, span)| snippet_with_applicability(cx, span, "..", &mut applicability)) | |
271 | .join(" + "); | |
272 | let hint_string = format!( | |
2b03887a | 273 | "consider combining the bounds: `{}: {trait_bounds}`", |
064997fb | 274 | snippet(cx, p.bounded_ty.span, "_"), |
f20569fa | 275 | ); |
f20569fa XL |
276 | span_lint_and_help( |
277 | cx, | |
278 | TYPE_REPETITION_IN_BOUNDS, | |
279 | p.span, | |
280 | "this type has already been used as a bound predicate", | |
281 | None, | |
282 | &hint_string, | |
283 | ); | |
284 | } | |
285 | } | |
286 | } | |
287 | } | |
288 | } | |
289 | ||
290 | fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { | |
f2b60f7d | 291 | if gen.span.from_expansion() { |
f20569fa XL |
292 | return; |
293 | } | |
294 | ||
f2b60f7d FG |
295 | // Explanation: |
296 | // fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) | |
297 | // where T: Clone + Default, { unimplemented!(); } | |
298 | // ^^^^^^^^^^^^^^^^^^ | |
299 | // | | |
300 | // collects each of these where clauses into a set keyed by generic name and comparable trait | |
301 | // eg. (T, Clone) | |
302 | let where_predicates = gen | |
303 | .predicates | |
304 | .iter() | |
305 | .filter_map(|pred| { | |
306 | if_chain! { | |
307 | if pred.in_where_clause(); | |
308 | if let WherePredicate::BoundPredicate(bound_predicate) = pred; | |
309 | if let TyKind::Path(QPath::Resolved(_, path)) = bound_predicate.bounded_ty.kind; | |
310 | then { | |
311 | return Some( | |
312 | rollup_traits(cx, bound_predicate.bounds, "these where clauses contain repeated elements") | |
313 | .into_iter().map(|(trait_ref, _)| (path.res, trait_ref))) | |
314 | } | |
315 | } | |
316 | None | |
317 | }) | |
318 | .flatten() | |
319 | .collect::<FxHashSet<_>>(); | |
320 | ||
321 | // Explanation: | |
322 | // fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) ... | |
323 | // ^^^^^^^^^^^^^^^^^^ ^^^^^^^ | |
324 | // | | |
325 | // compare trait bounds keyed by generic name and comparable trait to collected where | |
326 | // predicates eg. (T, Clone) | |
327 | for predicate in gen.predicates.iter().filter(|pred| !pred.in_where_clause()) { | |
f20569fa | 328 | if_chain! { |
f2b60f7d | 329 | if let WherePredicate::BoundPredicate(bound_predicate) = predicate; |
04454e1e | 330 | if bound_predicate.origin != PredicateOrigin::ImplTrait; |
a2a8927a | 331 | if !bound_predicate.span.from_expansion(); |
f2b60f7d | 332 | if let TyKind::Path(QPath::Resolved(_, path)) = bound_predicate.bounded_ty.kind; |
f20569fa | 333 | then { |
f2b60f7d FG |
334 | let traits = rollup_traits(cx, bound_predicate.bounds, "these bounds contain repeated elements"); |
335 | for (trait_ref, span) in traits { | |
336 | let key = (path.res, trait_ref); | |
337 | if where_predicates.contains(&key) { | |
f20569fa XL |
338 | span_lint_and_help( |
339 | cx, | |
340 | TRAIT_DUPLICATION_IN_BOUNDS, | |
f2b60f7d | 341 | span, |
f20569fa XL |
342 | "this trait bound is already specified in the where clause", |
343 | None, | |
344 | "consider removing this trait bound", | |
f2b60f7d | 345 | ); |
04454e1e | 346 | } |
f20569fa XL |
347 | } |
348 | } | |
349 | } | |
350 | } | |
351 | } | |
5099ac24 | 352 | |
f2b60f7d | 353 | #[derive(Clone, PartialEq, Eq, Hash, Debug)] |
064997fb | 354 | struct ComparableTraitRef(Res, Vec<Res>); |
f2b60f7d FG |
355 | impl Default for ComparableTraitRef { |
356 | fn default() -> Self { | |
357 | Self(Res::Err, Vec::new()) | |
064997fb FG |
358 | } |
359 | } | |
360 | ||
5099ac24 | 361 | fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> { |
064997fb FG |
362 | if let GenericBound::Trait(t, tbm) = bound { |
363 | let trait_path = t.trait_ref.path; | |
364 | let trait_span = { | |
365 | let path_span = trait_path.span; | |
366 | if let TraitBoundModifier::Maybe = tbm { | |
367 | path_span.with_lo(path_span.lo() - BytePos(1)) // include the `?` | |
368 | } else { | |
369 | path_span | |
370 | } | |
371 | }; | |
372 | Some((trait_path.res, trait_path.segments, trait_span)) | |
5099ac24 FG |
373 | } else { |
374 | None | |
375 | } | |
376 | } | |
064997fb FG |
377 | |
378 | // FIXME: ComparableTraitRef does not support nested bounds needed for associated_type_bounds | |
379 | fn into_comparable_trait_ref(trait_ref: &TraitRef<'_>) -> ComparableTraitRef { | |
380 | ComparableTraitRef( | |
381 | trait_ref.path.res, | |
382 | trait_ref | |
383 | .path | |
384 | .segments | |
385 | .iter() | |
386 | .filter_map(|segment| { | |
387 | // get trait bound type arguments | |
388 | Some(segment.args?.args.iter().filter_map(|arg| { | |
389 | if_chain! { | |
390 | if let GenericArg::Type(ty) = arg; | |
391 | if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; | |
392 | then { return Some(path.res) } | |
393 | } | |
394 | None | |
395 | })) | |
396 | }) | |
397 | .flatten() | |
398 | .collect(), | |
399 | ) | |
400 | } | |
401 | ||
f2b60f7d | 402 | fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) -> Vec<(ComparableTraitRef, Span)> { |
064997fb FG |
403 | let mut map = FxHashMap::default(); |
404 | let mut repeated_res = false; | |
405 | ||
406 | let only_comparable_trait_refs = |bound: &GenericBound<'_>| { | |
407 | if let GenericBound::Trait(t, _) = bound { | |
408 | Some((into_comparable_trait_ref(&t.trait_ref), t.span)) | |
409 | } else { | |
410 | None | |
411 | } | |
412 | }; | |
413 | ||
f2b60f7d | 414 | let mut i = 0usize; |
064997fb FG |
415 | for bound in bounds.iter().filter_map(only_comparable_trait_refs) { |
416 | let (comparable_bound, span_direct) = bound; | |
f2b60f7d FG |
417 | match map.entry(comparable_bound) { |
418 | Entry::Occupied(_) => repeated_res = true, | |
419 | Entry::Vacant(e) => { | |
420 | e.insert((span_direct, i)); | |
421 | i += 1; | |
422 | }, | |
064997fb FG |
423 | } |
424 | } | |
425 | ||
f2b60f7d FG |
426 | // Put bounds in source order |
427 | let mut comparable_bounds = vec![Default::default(); map.len()]; | |
428 | for (k, (v, i)) in map { | |
429 | comparable_bounds[i] = (k, v); | |
430 | } | |
431 | ||
064997fb FG |
432 | if_chain! { |
433 | if repeated_res; | |
434 | if let [first_trait, .., last_trait] = bounds; | |
435 | then { | |
436 | let all_trait_span = first_trait.span().to(last_trait.span()); | |
437 | ||
f2b60f7d FG |
438 | let traits = comparable_bounds.iter() |
439 | .filter_map(|&(_, span)| snippet_opt(cx, span)) | |
064997fb | 440 | .collect::<Vec<_>>(); |
064997fb FG |
441 | let traits = traits.join(" + "); |
442 | ||
443 | span_lint_and_sugg( | |
444 | cx, | |
445 | TRAIT_DUPLICATION_IN_BOUNDS, | |
446 | all_trait_span, | |
447 | msg, | |
448 | "try", | |
449 | traits, | |
450 | Applicability::MachineApplicable | |
451 | ); | |
452 | } | |
453 | } | |
f2b60f7d FG |
454 | |
455 | comparable_bounds | |
064997fb | 456 | } |