]> git.proxmox.com Git - rustc.git/blame - src/tools/clippy/clippy_lints/src/implied_bounds_in_impls.rs
bump version to 1.81.0+dfsg1-2~bpo12+pve1
[rustc.git] / src / tools / clippy / clippy_lints / src / implied_bounds_in_impls.rs
CommitLineData
781aab86
FG
1use clippy_utils::diagnostics::span_lint_and_then;
2use clippy_utils::source::snippet;
3use rustc_errors::{Applicability, SuggestionStyle};
c620b35d 4use rustc_hir::def_id::DefId;
781aab86 5use rustc_hir::{
1f0639a9
FG
6 AssocItemConstraint, GenericArg, GenericBound, GenericBounds, ItemKind, PredicateOrigin, TraitBoundModifier,
7 TyKind, WherePredicate,
781aab86 8};
e8be2606 9use rustc_hir_analysis::lower_ty;
781aab86
FG
10use rustc_lint::{LateContext, LateLintPass};
11use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt};
4b012472 12use rustc_session::declare_lint_pass;
781aab86
FG
13use rustc_span::Span;
14
15declare_clippy_lint! {
16 /// ### What it does
17 /// Looks for bounds in `impl Trait` in return position that are implied by other bounds.
18 /// This can happen when a trait is specified that another trait already has as a supertrait
19 /// (e.g. `fn() -> impl Deref + DerefMut<Target = i32>` has an unnecessary `Deref` bound,
20 /// because `Deref` is a supertrait of `DerefMut`)
21 ///
22 /// ### Why is this bad?
23 /// Specifying more bounds than necessary adds needless complexity for the reader.
24 ///
25 /// ### Limitations
26 /// This lint does not check for implied bounds transitively. Meaning that
31ef2f64 27 /// it doesn't check for implied bounds from supertraits of supertraits
781aab86
FG
28 /// (e.g. `trait A {} trait B: A {} trait C: B {}`, then having an `fn() -> impl A + C`)
29 ///
30 /// ### Example
ed00b5ec 31 /// ```no_run
781aab86
FG
32 /// # use std::ops::{Deref,DerefMut};
33 /// fn f() -> impl Deref<Target = i32> + DerefMut<Target = i32> {
34 /// // ^^^^^^^^^^^^^^^^^^^ unnecessary bound, already implied by the `DerefMut` trait bound
35 /// Box::new(123)
36 /// }
37 /// ```
38 /// Use instead:
ed00b5ec 39 /// ```no_run
781aab86
FG
40 /// # use std::ops::{Deref,DerefMut};
41 /// fn f() -> impl DerefMut<Target = i32> {
42 /// Box::new(123)
43 /// }
44 /// ```
4b012472 45 #[clippy::version = "1.74.0"]
781aab86 46 pub IMPLIED_BOUNDS_IN_IMPLS,
4b012472 47 complexity,
781aab86
FG
48 "specifying bounds that are implied by other bounds in `impl Trait` type"
49}
50declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]);
51
781aab86
FG
52fn emit_lint(
53 cx: &LateContext<'_>,
54 poly_trait: &rustc_hir::PolyTraitRef<'_>,
c620b35d 55 bounds: GenericBounds<'_>,
781aab86 56 index: usize,
31ef2f64
FG
57 // The constraints that were implied, used for suggestion purposes since removing a bound with
58 // associated types means we might need to then move it to a different bound.
59 implied_constraints: &[AssocItemConstraint<'_>],
c620b35d 60 bound: &ImplTraitBound<'_>,
781aab86 61) {
c620b35d 62 let implied_by = snippet(cx, bound.span, "..");
781aab86
FG
63
64 span_lint_and_then(
65 cx,
66 IMPLIED_BOUNDS_IN_IMPLS,
67 poly_trait.span,
e8be2606 68 format!("this bound is already specified as the supertrait of `{implied_by}`"),
781aab86
FG
69 |diag| {
70 // If we suggest removing a bound, we may also need to extend the span
71 // to include the `+` token that is ahead or behind,
72 // so we don't end up with something like `impl + B` or `impl A + `
73
c620b35d 74 let implied_span_extended = if let Some(next_bound) = bounds.get(index + 1) {
781aab86
FG
75 poly_trait.span.to(next_bound.span().shrink_to_lo())
76 } else if index > 0
c620b35d 77 && let Some(prev_bound) = bounds.get(index - 1)
781aab86
FG
78 {
79 prev_bound.span().shrink_to_hi().to(poly_trait.span.shrink_to_hi())
80 } else {
81 poly_trait.span
82 };
83
84 let mut sugg = vec![(implied_span_extended, String::new())];
85
1f0639a9
FG
86 // We also might need to include associated item constraints that were specified in the implied
87 // bound, but omitted in the implied-by bound:
781aab86
FG
88 // `fn f() -> impl Deref<Target = u8> + DerefMut`
89 // If we're going to suggest removing `Deref<..>`, we'll need to put `<Target = u8>` on `DerefMut`
31ef2f64 90 let omitted_constraints: Vec<_> = implied_constraints
781aab86 91 .iter()
31ef2f64 92 .filter(|constraint| !bound.constraints.iter().any(|c| c.ident == constraint.ident))
781aab86
FG
93 .collect();
94
31ef2f64
FG
95 if !omitted_constraints.is_empty() {
96 // `<>` needs to be added if there aren't yet any generic arguments or constraints
97 let needs_angle_brackets = bound.args.is_empty() && bound.constraints.is_empty();
98 let insert_span = match (bound.args, bound.constraints) {
99 ([.., arg], [.., constraint]) => arg.span().max(constraint.span).shrink_to_hi(),
781aab86 100 ([.., arg], []) => arg.span().shrink_to_hi(),
31ef2f64 101 ([], [.., constraint]) => constraint.span.shrink_to_hi(),
c620b35d 102 ([], []) => bound.span.shrink_to_hi(),
781aab86
FG
103 };
104
31ef2f64 105 let mut constraints_sugg = if needs_angle_brackets {
781aab86
FG
106 "<".to_owned()
107 } else {
31ef2f64 108 // If angle brackets aren't needed (i.e., there are already generic arguments or constraints),
781aab86
FG
109 // we need to add a comma:
110 // `impl A<B, C >`
111 // ^ if we insert `Assoc=i32` without a comma here, that'd be invalid syntax:
112 // `impl A<B, C Assoc=i32>`
113 ", ".to_owned()
114 };
115
31ef2f64 116 for (index, constraint) in omitted_constraints.into_iter().enumerate() {
781aab86 117 if index > 0 {
31ef2f64 118 constraints_sugg += ", ";
781aab86 119 }
31ef2f64 120 constraints_sugg += &snippet(cx, constraint.span, "..");
781aab86
FG
121 }
122 if needs_angle_brackets {
31ef2f64 123 constraints_sugg += ">";
781aab86 124 }
31ef2f64 125 sugg.push((insert_span, constraints_sugg));
781aab86
FG
126 }
127
128 diag.multipart_suggestion_with_style(
129 "try removing this bound",
130 sugg,
131 Applicability::MachineApplicable,
132 SuggestionStyle::ShowAlways,
133 );
134 },
135 );
136}
137
138/// Tries to "resolve" a type.
139/// The index passed to this function must start with `Self=0`, i.e. it must be a valid
140/// type parameter index.
141/// If the index is out of bounds, it means that the generic parameter has a default type.
142fn try_resolve_type<'tcx>(
143 tcx: TyCtxt<'tcx>,
144 args: &'tcx [GenericArg<'tcx>],
145 generics: &'tcx Generics,
146 index: usize,
147) -> Option<Ty<'tcx>> {
148 match args.get(index - 1) {
e8be2606 149 Some(GenericArg::Type(ty)) => Some(lower_ty(tcx, ty)),
781aab86 150 Some(_) => None,
31ef2f64 151 None => Some(tcx.type_of(generics.own_params[index].def_id).skip_binder()),
781aab86
FG
152 }
153}
154
155/// This function tries to, for all generic type parameters in a supertrait predicate `trait ...<U>:
156/// GenericTrait<U>`, check if the substituted type in the implied-by bound matches with what's
c0240ec0 157/// substituted in the implied bound.
781aab86
FG
158///
159/// Consider this example.
160/// ```rust,ignore
161/// trait GenericTrait<T> {}
162/// trait GenericSubTrait<T, U, V>: GenericTrait<U> {}
163/// ^^^^^^^^^^^^^^^ trait_predicate_args: [Self#0, U#2]
164/// (the Self#0 is implicit: `<Self as GenericTrait<U>>`)
165/// impl GenericTrait<i32> for () {}
166/// impl GenericSubTrait<(), i32, ()> for () {}
167/// impl GenericSubTrait<(), i64, ()> for () {}
168///
169/// fn f() -> impl GenericTrait<i32> + GenericSubTrait<(), i64, ()> {
170/// ^^^ implied_args ^^^^^^^^^^^ implied_by_args
171/// (we are interested in `i64` specifically, as that
172/// is what `U` in `GenericTrait<U>` is substituted with)
173/// }
174/// ```
175/// Here i32 != i64, so this will return false.
176fn is_same_generics<'tcx>(
177 tcx: TyCtxt<'tcx>,
178 trait_predicate_args: &'tcx [ty::GenericArg<'tcx>],
179 implied_by_args: &'tcx [GenericArg<'tcx>],
180 implied_args: &'tcx [GenericArg<'tcx>],
181 implied_by_def_id: DefId,
182 implied_def_id: DefId,
183) -> bool {
184 // Get the generics of the two traits to be able to get default generic parameter.
185 let implied_by_generics = tcx.generics_of(implied_by_def_id);
186 let implied_generics = tcx.generics_of(implied_def_id);
187
188 trait_predicate_args
189 .iter()
190 .enumerate()
191 .skip(1) // skip `Self` implicit arg
192 .all(|(arg_index, arg)| {
4b012472
FG
193 if [
194 implied_by_generics.host_effect_index,
195 implied_generics.host_effect_index,
196 ]
197 .contains(&Some(arg_index))
198 {
199 // skip host effect params in determining whether generics are same
200 return true;
201 }
781aab86
FG
202 if let Some(ty) = arg.as_type() {
203 if let &ty::Param(ty::ParamTy { index, .. }) = ty.kind()
204 // `index == 0` means that it's referring to `Self`,
205 // in which case we don't try to substitute it
206 && index != 0
207 && let Some(ty_a) = try_resolve_type(tcx, implied_by_args, implied_by_generics, index as usize)
208 && let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index)
209 {
210 ty_a == ty_b
211 } else if let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index) {
212 ty == ty_b
213 } else {
214 false
215 }
216 } else {
217 false
218 }
219 })
220}
221
c620b35d
FG
222struct ImplTraitBound<'tcx> {
223 /// The span of the bound in the `impl Trait` type
224 span: Span,
225 /// The predicates defined in the trait referenced by this bound. This also contains the actual
226 /// supertrait bounds
227 predicates: &'tcx [(ty::Clause<'tcx>, Span)],
228 /// The `DefId` of the trait being referenced by this bound
229 trait_def_id: DefId,
230 /// The generic arguments on the `impl Trait` bound
231 args: &'tcx [GenericArg<'tcx>],
31ef2f64
FG
232 /// The associated item constraints of this bound
233 constraints: &'tcx [AssocItemConstraint<'tcx>],
c620b35d 234}
781aab86 235
c620b35d
FG
236/// Given an `impl Trait` type, gets all the supertraits from each bound ("implied bounds").
237///
238/// For `impl Deref + DerefMut + Eq` this returns `[Deref, PartialEq]`.
239/// The `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`, and `PartialEq` comes from
240/// `Eq`.
241fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) -> Vec<ImplTraitBound<'tcx>> {
242 bounds
243 .iter()
244 .filter_map(|bound| {
781aab86
FG
245 if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
246 && let [.., path] = poly_trait.trait_ref.path.segments
c620b35d
FG
247 && poly_trait.bound_generic_params.is_empty()
248 && let Some(trait_def_id) = path.res.opt_def_id()
1f0639a9 249 && let predicates = cx.tcx.explicit_super_predicates_of(trait_def_id).predicates
c620b35d
FG
250 // If the trait has no supertrait, there is no need to collect anything from that bound
251 && !predicates.is_empty()
252 {
253 Some(ImplTraitBound {
254 predicates,
255 args: path.args.map_or([].as_slice(), |p| p.args),
31ef2f64 256 constraints: path.args.map_or([].as_slice(), |p| p.constraints),
c620b35d
FG
257 trait_def_id,
258 span: bound.span(),
259 })
260 } else {
261 None
262 }
263 })
264 .collect()
265}
781aab86 266
c620b35d
FG
267/// Given a bound in an `impl Trait` type, looks for a trait in the set of supertraits (previously
268/// collected in [`collect_supertrait_bounds`]) that matches (same trait and generic arguments).
269fn find_bound_in_supertraits<'a, 'tcx>(
270 cx: &LateContext<'tcx>,
271 trait_def_id: DefId,
272 args: &'tcx [GenericArg<'tcx>],
273 bounds: &'a [ImplTraitBound<'tcx>],
274) -> Option<&'a ImplTraitBound<'tcx>> {
275 bounds.iter().find(|bound| {
276 bound.predicates.iter().any(|(clause, _)| {
277 if let ClauseKind::Trait(tr) = clause.kind().skip_binder()
278 && tr.def_id() == trait_def_id
781aab86 279 {
c620b35d
FG
280 is_same_generics(
281 cx.tcx,
282 tr.trait_ref.args,
283 bound.args,
284 args,
285 bound.trait_def_id,
286 trait_def_id,
287 )
288 } else {
289 false
781aab86 290 }
c620b35d
FG
291 })
292 })
293}
294
295fn check<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) {
296 if bounds.len() == 1 {
297 // Very often there is only a single bound, e.g. `impl Deref<..>`, in which case
298 // we can avoid doing a bunch of stuff unnecessarily; there will trivially be
299 // no duplicate bounds
300 return;
301 }
302
303 let supertraits = collect_supertrait_bounds(cx, bounds);
304
305 // Lint all bounds in the `impl Trait` type that we've previously also seen in the set of
306 // supertraits of each of the bounds.
307 // This involves some extra logic when generic arguments are present, since
308 // simply comparing trait `DefId`s won't be enough. We also need to compare the generics.
309 for (index, bound) in bounds.iter().enumerate() {
310 if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
311 && let [.., path] = poly_trait.trait_ref.path.segments
312 && let implied_args = path.args.map_or([].as_slice(), |a| a.args)
31ef2f64 313 && let implied_constraints = path.args.map_or([].as_slice(), |a| a.constraints)
c620b35d
FG
314 && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
315 && let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
316 // If the implied bound has a type binding that also exists in the implied-by trait,
317 // then we shouldn't lint. See #11880 for an example.
318 && let assocs = cx.tcx.associated_items(bound.trait_def_id)
31ef2f64 319 && !implied_constraints.iter().any(|constraint| {
c620b35d 320 assocs
31ef2f64 321 .filter_by_name_unhygienic(constraint.ident.name)
c620b35d
FG
322 .next()
323 .is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
324 })
325 {
31ef2f64 326 emit_lint(cx, poly_trait, bounds, index, implied_constraints, bound);
781aab86
FG
327 }
328 }
329}
330
c620b35d
FG
331impl<'tcx> LateLintPass<'tcx> for ImpliedBoundsInImpls {
332 fn check_generics(&mut self, cx: &LateContext<'tcx>, generics: &rustc_hir::Generics<'tcx>) {
333 for predicate in generics.predicates {
334 if let WherePredicate::BoundPredicate(predicate) = predicate
335 // In theory, the origin doesn't really matter,
336 // we *could* also lint on explicit where clauses written out by the user,
337 // not just impl trait desugared ones, but that contradicts with the lint name...
338 && let PredicateOrigin::ImplTrait = predicate.origin
339 {
340 check(cx, predicate.bounds);
341 }
781aab86
FG
342 }
343 }
c620b35d
FG
344
345 fn check_ty(&mut self, cx: &LateContext<'_>, ty: &rustc_hir::Ty<'_>) {
346 if let TyKind::OpaqueDef(item_id, ..) = ty.kind
347 && let item = cx.tcx.hir().item(item_id)
348 && let ItemKind::OpaqueTy(opaque_ty) = item.kind
349 {
350 check(cx, opaque_ty.bounds);
781aab86
FG
351 }
352 }
353}