diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index dff60f76b746..52de13eaaf5e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -702,6 +702,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO, crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO, + crate::trait_bounds::MANUAL_ASSOC_TYPE_BOUNDS_INFO, crate::transmute::CROSSPOINTER_TRANSMUTE_INFO, crate::transmute::EAGER_TRANSMUTE_INFO, crate::transmute::MISSING_TRANSMUTE_ANNOTATIONS_INFO, diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index e641822b5d9b..da58b06316c8 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,17 +1,18 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{SpanRangeExt, snippet, snippet_with_applicability}; -use clippy_utils::{SpanlessEq, SpanlessHash, is_from_proc_macro}; +use clippy_utils::{SpanlessEq, SpanlessHash, is_from_proc_macro, over}; use core::hash::{Hash, Hasher}; use itertools::Itertools; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, IndexEntry}; use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; use rustc_hir::{ - BoundPolarity, GenericBound, Generics, Item, ItemKind, LangItem, Node, Path, PathSegment, PredicateOrigin, QPath, - TraitBoundModifiers, TraitItem, TraitRef, Ty, TyKind, WherePredicate, + AssocItemConstraintKind, BoundPolarity, GenericBound, Generics, Item, ItemKind, LangItem, Node, Path, PathSegment, + PredicateOrigin, QPath, TraitBoundModifiers, TraitItem, TraitRef, Ty, TyKind, WherePredicate, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; @@ -86,6 +87,24 @@ declare_clippy_lint! { "check if the same trait bounds are specified more than once during a generic declaration" } +declare_clippy_lint! { + /// ### What it does + /// TODO + /// + /// ### Why is this bad? + /// TODO + /// + /// ### Example + /// TODO + /// + /// Use instead: + /// TODO + #[clippy::version = "1.84.0"] + pub MANUAL_ASSOC_TYPE_BOUNDS, // TODO: better name + complexity, + "todo" +} + pub struct TraitBounds { max_trait_bounds: u64, msrv: Msrv, @@ -100,12 +119,13 @@ impl TraitBounds { } } -impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]); +impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS, MANUAL_ASSOC_TYPE_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for TraitBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, generics: &'tcx Generics<'_>) { self.check_type_repetition(cx, generics); check_trait_bound_duplication(cx, generics); + suggest_associated_type_bounds_in_generics(cx, generics); } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { @@ -371,6 +391,140 @@ fn check_trait_bound_duplication<'tcx>(cx: &LateContext<'tcx>, generics: &'_ Gen } } +fn suggest_associated_type_bounds_in_generics<'tcx>(cx: &LateContext<'tcx>, generics: &Generics<'tcx>) { + #[derive(Debug)] + struct TyAndTrait<'tcx> { + ty: &'tcx Ty<'tcx>, + trait_def_id: DefId, + trait_path_segment: &'tcx PathSegment<'tcx>, + } + + if generics.span.from_expansion() { + return; + } + + let type_trait_bounds = || { + generics + .predicates + .iter() + .filter_map(|predicate| match predicate { + WherePredicate::BoundPredicate(predicate) => Some(predicate), + _ => None, + }) + .map(|predicate| (predicate.bounded_ty, predicate.bounds, predicate.span)) + }; + + // 1) Collect all `Type: Trait` bounds + let mut trait_bounds = Vec::new(); + for (ty, generic_bounds, _) in type_trait_bounds() { + for generic_bound in generic_bounds { + if let GenericBound::Trait(poly_trait_ref) = generic_bound + && let [.., trait_path_segment] = poly_trait_ref.trait_ref.path.segments + && let Res::Def(DefKind::Trait, trait_def_id) = trait_path_segment.res + { + trait_bounds.push(TyAndTrait { + ty, + trait_def_id, + trait_path_segment, + }); + } + } + } + + // 2) Go through the bounds again and find projection bounds `::Assoc: Trait2` where + // `Type: Trait` exists in the vec. That bound can be merged with the original bound. + for (ty, trait_refs, predicate_span) in type_trait_bounds() { + if let TyKind::Path(QPath::Resolved(Some(self_ty), path)) = ty.kind + && let [.., trait_path, assoc_ty_path] = path.segments + && let Res::Def(DefKind::Trait, projection_trait_def_id) = trait_path.res + && let Res::Def(DefKind::AssocTy, _) = assoc_ty_path.res + && let mut spanless_eq = SpanlessEq::new(cx).paths_by_resolution().inter_expr() + && let Some(trait_bound) = trait_bounds.iter().find(|t| { + t.trait_def_id == projection_trait_def_id + && spanless_eq.eq_ty(self_ty, t.ty) + // NB: intentionally don't check associated types + && over( + trait_path.args().args, + t.trait_path_segment.args().args, + |left, right| spanless_eq.eq_generic_arg(left, right) + ) + }) + && let Ok(remove_span) = cx + .tcx + .sess + .source_map() + .span_extend_while(predicate_span, |s| matches!(s, ' ' | ',')) + && let Some(bounds_span) = trait_refs + .first() + .zip(trait_refs.last()) + .map(|(first, last)| first.span().to(last.span())) + { + // Cases to consider for the suggestion: + // - no generic args present, insert + // - generic args present but `Assoc` type binding is not, add `, Assoc: Trait1 + Trait2` at the end + // - generic args present as well as `Assoc` type binding, add `+ Trait1 + Trait2` to the binding + // Q: what if there exists both an assoc type binding and an equality constraint for the same assoc + // type? + span_lint_and_then( + cx, + &MANUAL_ASSOC_TYPE_BOUNDS, + predicate_span, + "this bound can be merged with a previous bound", + |diag| { + let mut suggestions = vec![(remove_span, String::new())]; + + match trait_bound.trait_path_segment.args { + Some(args) if let Some(args_span) = args.span() => { + if let Some(constraint) = args + .constraints + .iter() + .find(|constraint| constraint.ident == assoc_ty_path.ident) + && let AssocItemConstraintKind::Bound { + bounds: [.., last_bound], + } = constraint.kind + { + // Generic args present as well as `Assoc` type binding, add `+ Trait1 + Trait2` to the + // binding + suggestions.push(( + last_bound.span().shrink_to_hi(), + format!(" + {}", snippet(cx, bounds_span, "..")), + )); + } else { + // Generic args present but `Assoc` type binding is not, add `, Assoc: Trait1 + Trait2` + // at the end + let comma = if !args.args.is_empty() || !args.constraints.is_empty() { + ", " + } else { + "" + }; + + suggestions.push(( + args_span.shrink_to_hi(), + format!("{comma}{}: {}", assoc_ty_path.ident, snippet(cx, bounds_span, "..")), + )); + } + }, + _ => { + // No generic args present, insert + + suggestions.push(( + trait_bound.trait_path_segment.ident.span.shrink_to_hi(), + format!("<{}: {}>", assoc_ty_path.ident, snippet(cx, bounds_span, "..")), + )); + }, + } + + diag.multipart_suggestion_verbose( + "remove the extra predicate and use associated type bounds", + suggestions, + Applicability::MachineApplicable, + ); + }, + ); + } + } +} + struct ComparableTraitRef<'a, 'tcx> { cx: &'a LateContext<'tcx>, trait_ref: &'tcx TraitRef<'tcx>, diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index c73ab4bfa688..5f293cf79e94 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -455,7 +455,7 @@ impl HirEqInterExpr<'_, '_, '_> { left.ident.name == right.ident.name && self.eq_expr(left.expr, right.expr) } - fn eq_generic_arg(&mut self, left: &GenericArg<'_>, right: &GenericArg<'_>) -> bool { + pub fn eq_generic_arg(&mut self, left: &GenericArg<'_>, right: &GenericArg<'_>) -> bool { match (left, right) { (GenericArg::Const(l), GenericArg::Const(r)) => self.eq_const_arg(l, r), (GenericArg::Lifetime(l_lt), GenericArg::Lifetime(r_lt)) => Self::eq_lifetime(l_lt, r_lt), diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.rs b/tests/ui/trait_duplication_in_bounds_unfixable.rs index b0095bb77b5a..42be10fe487a 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.rs +++ b/tests/ui/trait_duplication_in_bounds_unfixable.rs @@ -1,5 +1,5 @@ #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::multiple_bound_locations)] +#![allow(clippy::multiple_bound_locations, clippy::manual_assoc_type_bounds)] // TODO: investigate weird suggestion use std::collections::BTreeMap; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};