1use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};2use clippy_utils::res::{MaybeDef, MaybeQPath};3use clippy_utils::ty::implements_trait;4use clippy_utils::{is_from_proc_macro, last_path_segment, std_or_core};5use rustc_errors::Applicability;6use rustc_hir::def_id::DefId;7use rustc_hir::{Block, Body, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, LangItem, UnOp};8use rustc_lint::{LateContext, LateLintPass, LintContext};9use rustc_middle::ty::{TyCtxt, TypeckResults};10use rustc_session::impl_lint_pass;11use rustc_span::sym;12use rustc_span::symbol::kw;1314declare_clippy_lint! {15    /// ### What it does16    /// Checks for non-canonical implementations of `Clone` when `Copy` is already implemented.17    ///18    /// ### Why is this bad?19    /// If both `Clone` and `Copy` are implemented, they must agree. This can done by dereferencing20    /// `self` in `Clone`'s implementation, which will avoid any possibility of the implementations21    /// becoming out of sync.22    ///23    /// ### Example24    /// ```rust,ignore25    /// #[derive(Eq, PartialEq)]26    /// struct A(u32);27    ///28    /// impl Clone for A {29    ///     fn clone(&self) -> Self {30    ///         Self(self.0)31    ///     }32    /// }33    ///34    /// impl Copy for A {}35    /// ```36    /// Use instead:37    /// ```rust,ignore38    /// #[derive(Eq, PartialEq)]39    /// struct A(u32);40    ///41    /// impl Clone for A {42    ///     fn clone(&self) -> Self {43    ///         *self44    ///     }45    /// }46    ///47    /// impl Copy for A {}48    /// ```49    #[clippy::version = "1.72.0"]50    pub NON_CANONICAL_CLONE_IMPL,51    suspicious,52    "non-canonical implementation of `Clone` on a `Copy` type"53}5455declare_clippy_lint! {56    /// ### What it does57    /// Checks for non-canonical implementations of `PartialOrd` when `Ord` is already implemented.58    ///59    /// ### Why is this bad?60    /// If both `PartialOrd` and `Ord` are implemented, they must agree. This is commonly done by61    /// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently62    /// introduce an error upon refactoring.63    ///64    /// ### Known issues65    /// Code that calls the `.into()` method instead will be flagged, despite `.into()` wrapping it66    /// in `Some`.67    ///68    /// ### Example69    /// ```no_run70    /// # use std::cmp::Ordering;71    /// #[derive(Eq, PartialEq)]72    /// struct A(u32);73    ///74    /// impl Ord for A {75    ///     fn cmp(&self, other: &Self) -> Ordering {76    ///         // ...77    /// #       todo!();78    ///     }79    /// }80    ///81    /// impl PartialOrd for A {82    ///     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {83    ///         // ...84    /// #       todo!();85    ///     }86    /// }87    /// ```88    /// Use instead:89    /// ```no_run90    /// # use std::cmp::Ordering;91    /// #[derive(Eq, PartialEq)]92    /// struct A(u32);93    ///94    /// impl Ord for A {95    ///     fn cmp(&self, other: &Self) -> Ordering {96    ///         // ...97    /// #       todo!();98    ///     }99    /// }100    ///101    /// impl PartialOrd for A {102    ///     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {103    ///         Some(self.cmp(other))   // or self.cmp(other).into()104    ///     }105    /// }106    /// ```107    #[clippy::version = "1.73.0"]108    pub NON_CANONICAL_PARTIAL_ORD_IMPL,109    suspicious,110    "non-canonical implementation of `PartialOrd` on an `Ord` type"111}112113impl_lint_pass!(NonCanonicalImpls => [114    NON_CANONICAL_CLONE_IMPL,115    NON_CANONICAL_PARTIAL_ORD_IMPL,116]);117118#[expect(119    clippy::struct_field_names,120    reason = "`_trait` suffix is meaningful on its own, \121              and creating an inner `StoredTraits` struct would just add a level of indirection"122)]123pub(crate) struct NonCanonicalImpls {124    partial_ord_trait: Option<DefId>,125    ord_trait: Option<DefId>,126    clone_trait: Option<DefId>,127    copy_trait: Option<DefId>,128}129130impl NonCanonicalImpls {131    pub(crate) fn new(tcx: TyCtxt<'_>) -> Self {132        let lang_items = tcx.lang_items();133        Self {134            partial_ord_trait: lang_items.partial_ord_trait(),135            ord_trait: tcx.get_diagnostic_item(sym::Ord),136            clone_trait: lang_items.clone_trait(),137            copy_trait: lang_items.copy_trait(),138        }139    }140}141142/// The traits that this lint looks at143enum Trait {144    Clone,145    PartialOrd,146}147148impl LateLintPass<'_> for NonCanonicalImpls {149    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {150        if let ItemKind::Impl(impl_) = item.kind151            // Both `PartialOrd` and `Clone` have one required method, and `PartialOrd` can have 5 methods in total152            && (1..=5).contains(&impl_.items.len())153            && let Some(of_trait) = impl_.of_trait154            && let Some(trait_did) = of_trait.trait_ref.trait_def_id()155            // Check this early to hopefully bail out as soon as possible156            && let trait_ = if Some(trait_did) == self.clone_trait {157                Trait::Clone158            } else if Some(trait_did) == self.partial_ord_trait {159                Trait::PartialOrd160            } else {161                return;162            }163            && !cx.tcx.is_automatically_derived(item.owner_id.to_def_id())164        {165            let mut assoc_fns = impl_166                .items167                .iter()168                .map(|id| cx.tcx.hir_impl_item(*id))169                .filter_map(|assoc| {170                    if let ImplItemKind::Fn(_, body_id) = assoc.kind171                        && let body = cx.tcx.hir_body(body_id)172                        && let ExprKind::Block(block, ..) = body.value.kind173                        && !block.span.in_external_macro(cx.sess().source_map())174                    {175                        Some((assoc, body, block))176                    } else {177                        None178                    }179                });180181            let trait_impl = cx.tcx.impl_trait_ref(item.owner_id).skip_binder();182183            match trait_ {184                Trait::Clone => {185                    if let Some(copy_trait) = self.copy_trait186                        && implements_trait(cx, trait_impl.self_ty(), copy_trait, &[])187                    {188                        for (assoc, _, block) in assoc_fns {189                            check_clone_on_copy(cx, assoc, block);190                        }191                    }192                },193                Trait::PartialOrd => {194                    // If `Self` and `Rhs` are not the same type, then a corresponding `Ord` impl is not possible,195                    // since it doesn't have an `Rhs`196                    if let [lhs, rhs] = trait_impl.args.as_slice()197                        && lhs == rhs198                        && let Some(ord_trait) = self.ord_trait199                        && implements_trait(cx, trait_impl.self_ty(), ord_trait, &[])200                        && let Some((assoc, body, block)) =201                            assoc_fns.find(|(assoc, _, _)| assoc.ident.name == sym::partial_cmp)202                    {203                        check_partial_ord_on_ord(cx, assoc, item, body, block);204                    }205                },206            }207        }208    }209}210211fn check_clone_on_copy(cx: &LateContext<'_>, impl_item: &ImplItem<'_>, block: &Block<'_>) {212    if impl_item.ident.name == sym::clone {213        if block.stmts.is_empty()214            && let Some(expr) = block.expr215            && let ExprKind::Unary(UnOp::Deref, deref) = expr.kind216            && let ExprKind::Path(qpath) = deref.kind217            && last_path_segment(&qpath).ident.name == kw::SelfLower218        {219            // this is the canonical implementation, `fn clone(&self) -> Self { *self }`220            return;221        }222223        if is_from_proc_macro(cx, impl_item) {224            return;225        }226227        span_lint_and_sugg(228            cx,229            NON_CANONICAL_CLONE_IMPL,230            block.span,231            "non-canonical implementation of `clone` on a `Copy` type",232            "change this to",233            "{ *self }".to_owned(),234            Applicability::MaybeIncorrect,235        );236    }237238    if impl_item.ident.name == sym::clone_from && !is_from_proc_macro(cx, impl_item) {239        span_lint_and_sugg(240            cx,241            NON_CANONICAL_CLONE_IMPL,242            impl_item.span,243            "unnecessary implementation of `clone_from` on a `Copy` type",244            "remove it",245            String::new(),246            Applicability::MaybeIncorrect,247        );248    }249}250251fn check_partial_ord_on_ord<'tcx>(252    cx: &LateContext<'tcx>,253    impl_item: &ImplItem<'_>,254    item: &Item<'_>,255    body: &Body<'_>,256    block: &Block<'tcx>,257) {258    // If the `cmp` call likely needs to be fully qualified in the suggestion259    // (like `std::cmp::Ord::cmp`). It's unfortunate we must put this here but we can't260    // access `cmp_expr` in the suggestion without major changes, as we lint in `else`.261262    let mut needs_fully_qualified = false;263    if block.stmts.is_empty()264        && let Some(expr) = block.expr265        && expr_is_cmp(cx, expr, impl_item, &mut needs_fully_qualified)266    {267        return;268    }269    // Fix #12683, allow [`needless_return`] here270    else if block.expr.is_none()271        && let Some(stmt) = block.stmts.first()272        && let rustc_hir::StmtKind::Semi(Expr {273            kind: ExprKind::Ret(Some(ret)),274            ..275        }) = stmt.kind276        && expr_is_cmp(cx, ret, impl_item, &mut needs_fully_qualified)277    {278        return;279    } else if is_from_proc_macro(cx, impl_item) {280        return;281    }282283    span_lint_and_then(284        cx,285        NON_CANONICAL_PARTIAL_ORD_IMPL,286        item.span,287        "non-canonical implementation of `partial_cmp` on an `Ord` type",288        |diag| {289            let [_, other] = body.params else {290                return;291            };292            let Some(std_or_core) = std_or_core(cx) else {293                return;294            };295296            let suggs = match (other.pat.simple_ident(), needs_fully_qualified) {297                (Some(other_ident), true) => vec![(298                    block.span,299                    format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, {})) }}", other_ident.name),300                )],301                (Some(other_ident), false) => {302                    vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]303                },304                (None, true) => vec![305                    (306                        block.span,307                        format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, other)) }}"),308                    ),309                    (other.pat.span, "other".to_owned()),310                ],311                (None, false) => vec![312                    (block.span, "{ Some(self.cmp(other)) }".to_owned()),313                    (other.pat.span, "other".to_owned()),314                ],315            };316317            diag.multipart_suggestion("change this to", suggs, Applicability::Unspecified);318        },319    );320}321322/// Return true if `expr_kind` is a `cmp` call.323fn expr_is_cmp<'tcx>(324    cx: &LateContext<'tcx>,325    expr: &'tcx Expr<'tcx>,326    impl_item: &ImplItem<'_>,327    needs_fully_qualified: &mut bool,328) -> bool {329    let typeck = cx.tcx.typeck(impl_item.owner_id.def_id);330    match expr.kind {331        ExprKind::Call(332            Expr {333                kind: ExprKind::Path(some_path),334                hir_id: some_hir_id,335                ..336            },337            [cmp_expr],338        ) => {339            typeck.qpath_res(some_path, *some_hir_id).ctor_parent(cx).is_lang_item(cx, LangItem::OptionSome)340                // Fix #11178, allow `Self::cmp(self, ..)`341                && self_cmp_call(cx, typeck, cmp_expr, needs_fully_qualified)342        },343        ExprKind::MethodCall(_, recv, [], _) => {344            typeck345                .type_dependent_def(expr.hir_id)346                .assoc_parent(cx)347                .is_diag_item(cx, sym::Into)348                && self_cmp_call(cx, typeck, recv, needs_fully_qualified)349        },350        _ => false,351    }352}353354/// Returns whether this is any of `self.cmp(..)`, `Self::cmp(self, ..)` or `Ord::cmp(self, ..)`.355fn self_cmp_call<'tcx>(356    cx: &LateContext<'tcx>,357    typeck: &TypeckResults<'tcx>,358    cmp_expr: &'tcx Expr<'tcx>,359    needs_fully_qualified: &mut bool,360) -> bool {361    match cmp_expr.kind {362        ExprKind::Call(path, [_, _]) => path.res(typeck).is_diag_item(cx, sym::ord_cmp_method),363        ExprKind::MethodCall(_, recv, [_], ..) => {364            let ExprKind::Path(path) = recv.kind else {365                return false;366            };367            if last_path_segment(&path).ident.name != kw::SelfLower {368                return false;369            }370371            // We can set this to true here no matter what as if it's a `MethodCall` and goes to the372            // `else` branch, it must be a method named `cmp` that isn't `Ord::cmp`373            *needs_fully_qualified = true;374375            typeck376                .type_dependent_def_id(cmp_expr.hir_id)377                .is_some_and(|def_id| cx.tcx.is_diagnostic_item(sym::ord_cmp_method, def_id))378        },379        _ => false,380    }381}