Skip to content

Commit

Permalink
quick impl
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Nov 17, 2024
1 parent 0712689 commit 31a9841
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 8 deletions.
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
166 changes: 160 additions & 6 deletions clippy_lints/src/trait_bounds.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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>) {
Expand Down Expand Up @@ -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 `<Type as Trait>::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 <Assoc: Trait1 + Trait2>
// - 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 <Assoc: Trait1 + Trait2>

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>,
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/trait_duplication_in_bounds_unfixable.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down

0 comments on commit 31a9841

Please sign in to comment.