use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use rustc_errors::{Applicability, SuggestionStyle}; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{ Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, TraitBoundModifier, TraitItem, TraitItemKind, TyKind, }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, ClauseKind, TyCtxt}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; declare_clippy_lint! { /// ### What it does /// Looks for bounds in `impl Trait` in return position that are implied by other bounds. /// This can happen when a trait is specified that another trait already has as a supertrait /// (e.g. `fn() -> impl Deref + DerefMut` has an unnecessary `Deref` bound, /// because `Deref` is a supertrait of `DerefMut`) /// /// ### Why is this bad? /// Specifying more bounds than necessary adds needless complexity for the reader. /// /// ### Limitations /// This lint does not check for implied bounds transitively. Meaning that /// it does't check for implied bounds from supertraits of supertraits /// (e.g. `trait A {} trait B: A {} trait C: B {}`, then having an `fn() -> impl A + C`) /// /// ### Example /// ```rust /// # use std::ops::{Deref,DerefMut}; /// fn f() -> impl Deref + DerefMut { /// // ^^^^^^^^^^^^^^^^^^^ unnecessary bound, already implied by the `DerefMut` trait bound /// Box::new(123) /// } /// ``` /// Use instead: /// ```rust /// # use std::ops::{Deref,DerefMut}; /// fn f() -> impl DerefMut { /// Box::new(123) /// } /// ``` #[clippy::version = "1.73.0"] pub IMPLIED_BOUNDS_IN_IMPLS, complexity, "specifying bounds that are implied by other bounds in `impl Trait` type" } declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]); /// This function tries to, for all type parameters in a supertype predicate `GenericTrait`, /// check if the substituted type in the implied-by bound matches with what's subtituted in the /// implied type. /// /// Consider this example. /// ```rust,ignore /// trait GenericTrait {} /// trait GenericSubTrait: GenericTrait {} /// ^ trait_predicate_args: [Self#0, U#2] /// impl GenericTrait for () {} /// impl GenericSubTrait<(), i32, ()> for () {} /// impl GenericSubTrait<(), [u8; 8], ()> for () {} /// /// fn f() -> impl GenericTrait + GenericSubTrait<(), [u8; 8], ()> { /// ^^^ implied_args ^^^^^^^^^^^^^^^ implied_by_args /// (we are interested in `[u8; 8]` specifically, as that /// is what `U` in `GenericTrait` is substituted with) /// () /// } /// ``` /// Here i32 != [u8; 8], so this will return false. fn is_same_generics( tcx: TyCtxt<'_>, trait_predicate_args: &[ty::GenericArg<'_>], implied_by_args: &[GenericArg<'_>], implied_args: &[GenericArg<'_>], ) -> bool { trait_predicate_args .iter() .enumerate() .skip(1) // skip `Self` implicit arg .all(|(arg_index, arg)| { if let Some(ty) = arg.as_type() && let &ty::Param(ty::ParamTy{ index, .. }) = ty.kind() // Since `trait_predicate_args` and type params in traits start with `Self=0` // and generic argument lists `GenericTrait` don't have `Self`, // we need to subtract 1 from the index. && let GenericArg::Type(ty_a) = implied_by_args[index as usize - 1] && let GenericArg::Type(ty_b) = implied_args[arg_index - 1] { hir_ty_to_ty(tcx, ty_a) == hir_ty_to_ty(tcx, ty_b) } else { false } }) } fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) { if let FnRetTy::Return(ty) = decl.output &&let TyKind::OpaqueDef(item_id, ..) = ty.kind && let item = cx.tcx.hir().item(item_id) && let ItemKind::OpaqueTy(opaque_ty) = item.kind // Very often there is only a single bound, e.g. `impl Deref<..>`, in which case // we can avoid doing a bunch of stuff unnecessarily. && opaque_ty.bounds.len() > 1 { // Get all the (implied) trait predicates in the bounds. // For `impl Deref + DerefMut` this will contain [`Deref`]. // The implied `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`. // N.B. (G)ATs are fine to disregard, because they must be the same for all of its supertraits. // Example: // `impl Deref + DerefMut` is not allowed. // `DerefMut::Target` needs to match `Deref::Target`. let implied_bounds: Vec<_> = opaque_ty.bounds.iter().filter_map(|bound| { if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound && let [.., path] = poly_trait.trait_ref.path.segments && poly_trait.bound_generic_params.is_empty() && let Some(trait_def_id) = path.res.opt_def_id() && let predicates = cx.tcx.super_predicates_of(trait_def_id).predicates && !predicates.is_empty() // If the trait has no supertrait, there is nothing to add. { Some((bound.span(), path.args.map_or([].as_slice(), |a| a.args), predicates)) } else { None } }).collect(); // Lint all bounds in the `impl Trait` type that are also in the `implied_bounds` vec. // This involves some extra logic when generic arguments are present, since // simply comparing trait `DefId`s won't be enough. We also need to compare the generics. for (index, bound) in opaque_ty.bounds.iter().enumerate() { if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound && let [.., path] = poly_trait.trait_ref.path.segments && let implied_args = path.args.map_or([].as_slice(), |a| a.args) && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id() && let Some(implied_by_span) = implied_bounds.iter().find_map(|&(span, implied_by_args, preds)| { preds.iter().find_map(|(clause, _)| { if let ClauseKind::Trait(tr) = clause.kind().skip_binder() && tr.def_id() == def_id && is_same_generics(cx.tcx, tr.trait_ref.args, implied_by_args, implied_args) { Some(span) } else { None } }) }) { let implied_by = snippet(cx, implied_by_span, ".."); span_lint_and_then( cx, IMPLIED_BOUNDS_IN_IMPLS, poly_trait.span, &format!("this bound is already specified as the supertrait of `{implied_by}`"), |diag| { // If we suggest removing a bound, we may also need extend the span // to include the `+` token, so we don't end up with something like `impl + B` let implied_span_extended = if let Some(next_bound) = opaque_ty.bounds.get(index + 1) { poly_trait.span.to(next_bound.span().shrink_to_lo()) } else { poly_trait.span }; diag.span_suggestion_with_style( implied_span_extended, "try removing this bound", "", Applicability::MachineApplicable, SuggestionStyle::ShowAlways ); } ); } } } } impl LateLintPass<'_> for ImpliedBoundsInImpls { fn check_fn( &mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, _: LocalDefId, ) { check(cx, decl); } fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) { if let TraitItemKind::Fn(sig, ..) = &item.kind { check(cx, sig.decl); } } fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) { if let ImplItemKind::Fn(sig, ..) = &item.kind { check(cx, sig.decl); } } }