diff --git a/CHANGELOG.md b/CHANGELOG.md index 840a46aa8..fea11bba6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4921,7 +4921,6 @@ Released 2018-09-13 [`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or -[`manual_partial_ord_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_partial_ord_impl [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains [`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid @@ -5021,6 +5020,7 @@ Released 2018-09-13 [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref [`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take [`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals +[`needless_partial_ord_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_partial_ord_impl [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value [`needless_pub_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pub_self [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 7764dd739..a2231b8fb 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -277,7 +277,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::manual_let_else::MANUAL_LET_ELSE_INFO, crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO, crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO, - crate::manual_partial_ord_impl::MANUAL_PARTIAL_ORD_IMPL_INFO, crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO, crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO, crate::manual_retain::MANUAL_RETAIN_INFO, @@ -470,6 +469,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::needless_else::NEEDLESS_ELSE_INFO, crate::needless_for_each::NEEDLESS_FOR_EACH_INFO, crate::needless_if::NEEDLESS_IF_INFO, + crate::needless_impls::NEEDLESS_PARTIAL_ORD_IMPL_INFO, crate::needless_late_init::NEEDLESS_LATE_INIT_INFO, crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO, crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 36a0cf860..1ec5b2e8b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -188,7 +188,6 @@ mod manual_is_ascii_check; mod manual_let_else; mod manual_main_separator_str; mod manual_non_exhaustive; -mod manual_partial_ord_impl; mod manual_range_patterns; mod manual_rem_euclid; mod manual_retain; @@ -228,6 +227,7 @@ mod needless_continue; mod needless_else; mod needless_for_each; mod needless_if; +mod needless_impls; mod needless_late_init; mod needless_parens_on_range_literals; mod needless_pass_by_value; diff --git a/clippy_lints/src/manual_partial_ord_impl.rs b/clippy_lints/src/manual_partial_ord_impl.rs deleted file mode 100644 index 4c43b5e52..000000000 --- a/clippy_lints/src/manual_partial_ord_impl.rs +++ /dev/null @@ -1,132 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::{ - def_path_def_ids, diagnostics::span_lint_and_sugg, get_trait_def_id, match_def_path, path_res, ty::implements_trait, -}; -use rustc_errors::Applicability; -use rustc_hir::def::Res; -use rustc_hir::*; -use rustc_hir_analysis::hir_ty_to_ty; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::def_id::DefId; -use std::cell::OnceCell; - -declare_clippy_lint! { - /// ### What it does - /// Checks for manual implementations of both `PartialOrd` and `Ord` when only `Ord` is - /// necessary. - /// - /// ### Why is this bad? - /// If both `PartialOrd` and `Ord` are implemented, `PartialOrd` will wrap the returned value of - /// `Ord::cmp` in `Some`. Not doing this may silently introduce an error. - /// - /// ### Example - /// ```rust - /// #[derive(Eq, PartialEq)] - /// struct A(u32); - /// - /// impl Ord for A { - /// fn cmp(&self, other: &Self) -> Ordering { - /// todo!(); - /// } - /// } - /// - /// impl PartialOrd for A { - /// fn partial_cmp(&self, other: &Self) -> Option { - /// todo!(); - /// } - /// } - /// ``` - /// Use instead: - /// ```rust - /// #[derive(Eq, PartialEq)] - /// struct A(u32); - /// - /// impl Ord for A { - /// fn cmp(&self, other: &Self) -> Ordering { - /// todo!(); - /// } - /// } - /// - /// impl PartialOrd for A { - /// fn partial_cmp(&self, other: &Self) -> Option { - /// Some(self.cmp(other)) - /// } - /// } - /// ``` - #[clippy::version = "1.71.0"] - pub MANUAL_PARTIAL_ORD_IMPL, - nursery, - "default lint description" -} -impl_lint_pass!(ManualPartialOrdImpl => [MANUAL_PARTIAL_ORD_IMPL]); - -#[derive(Clone)] -pub struct ManualPartialOrdImpl { - pub ord_def_id: OnceCell, -} - -impl LateLintPass<'_> for ManualPartialOrdImpl { - fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if_chain! { - if let ItemKind::Impl(imp) = &item.kind; - if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(item.owner_id); - if cx.tcx.is_diagnostic_item(sym!(PartialOrd), impl_trait_ref.skip_binder().def_id); - then { - lint_impl_body(self, cx, imp, item); - } - } - } -} - -fn lint_impl_body(conf: &mut ManualPartialOrdImpl, cx: &LateContext<'_>, imp: &Impl<'_>, item: &Item<'_>) { - for imp_item in imp.items { - if_chain! { - if imp_item.ident.name == sym!(partial_cmp); - if let ImplItemKind::Fn(_, id) = cx.tcx.hir().impl_item(imp_item.id).kind; - then { - let body = cx.tcx.hir().body(id); - let ord_def_id = conf.ord_def_id.get_or_init(|| get_trait_def_id(cx, &["core", "cmp", "Ord"]).unwrap()); - if let ExprKind::Block(block, ..) - = body.value.kind && implements_trait(cx, hir_ty_to_ty(cx.tcx, imp.self_ty), *ord_def_id, &[]) - { - if_chain! { - if block.stmts.is_empty(); - if let Some(expr) = block.expr; - if let ExprKind::Call(Expr { kind: ExprKind::Path(path), ..}, [cmp_expr]) = expr.kind; - if let QPath::Resolved(_, some_path) = path; - if let Some(some_seg_one) = some_path.segments.get(0); - if some_seg_one.ident.name == sym!(Some); - if let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind; - if cmp_path.ident.name == sym!(cmp); - if let Res::Local(..) = path_res(cx, other_expr); - then {} - else { - span_lint_and_then( - cx, - MANUAL_PARTIAL_ORD_IMPL, - item.span, - "manual implementation of `PartialOrd` when `Ord` is already implemented", - |diag| { - if let Some(param) = body.params.get(1) - && let PatKind::Binding(_, _, param_ident, ..) = param.pat.kind - { - diag.span_suggestion( - block.span, - "change this to", - format!("{{ Some(self.cmp({})) }}", - param_ident.name), - Applicability::MaybeIncorrect - ); - } else { - diag.help("return the value of `self.cmp` wrapped in `Some` instead"); - }; - } - ); - } - } - } - } - } - } -} diff --git a/clippy_lints/src/needless_impls.rs b/clippy_lints/src/needless_impls.rs new file mode 100644 index 000000000..4f8741bf3 --- /dev/null +++ b/clippy_lints/src/needless_impls.rs @@ -0,0 +1,145 @@ +use clippy_utils::{ + diagnostics::span_lint_and_then, get_parent_node, is_res_lang_ctor, path_res, ty::implements_trait, +}; +use rustc_errors::Applicability; +use rustc_hir::{def::Res, Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, LangItem, Node, PatKind}; +use rustc_hir_analysis::hir_ty_to_ty; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::EarlyBinder; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; +use std::borrow::Cow; + +declare_clippy_lint! { + /// ### What it does + /// Checks for manual implementations of both `PartialOrd` and `Ord` when only `Ord` is + /// necessary. + /// + /// ### Why is this bad? + /// If both `PartialOrd` and `Ord` are implemented, they must agree. This is commonly done by + /// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently + /// introduce an error upon refactoring. + /// + /// ### Example + /// ```rust,ignore + /// #[derive(Eq, PartialEq)] + /// struct A(u32); + /// + /// impl Ord for A { + /// fn cmp(&self, other: &Self) -> Ordering { + /// todo!(); + /// } + /// } + /// + /// impl PartialOrd for A { + /// fn partial_cmp(&self, other: &Self) -> Option { + /// todo!(); + /// } + /// } + /// ``` + /// Use instead: + /// ```rust,ignore + /// #[derive(Eq, PartialEq)] + /// struct A(u32); + /// + /// impl Ord for A { + /// fn cmp(&self, other: &Self) -> Ordering { + /// todo!(); + /// } + /// } + /// + /// impl PartialOrd for A { + /// fn partial_cmp(&self, other: &Self) -> Option { + /// Some(self.cmp(other)) + /// } + /// } + /// ``` + #[clippy::version = "1.72.0"] + pub NEEDLESS_PARTIAL_ORD_IMPL, + correctness, + "manual implementation of `PartialOrd` when `Ord` is already implemented" +} +declare_lint_pass!(NeedlessImpls => [NEEDLESS_PARTIAL_ORD_IMPL]); + +impl LateLintPass<'_> for NeedlessImpls { + fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { + let node = get_parent_node(cx.tcx, impl_item.hir_id()); + let Some(Node::Item(item)) = node else { + return; + }; + let ItemKind::Impl(imp) = item.kind else { + return; + }; + let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else { + return; + }; + let trait_impl_def_id = trait_impl.def_id; + if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) { + return; + } + let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else { + return; + }; + let body = cx.tcx.hir().body(impl_item_id); + let ExprKind::Block(block, ..) = body.value.kind else { + return; + }; + + if cx.tcx.is_diagnostic_item(sym::PartialOrd, trait_impl_def_id) + && impl_item.ident.name == sym::partial_cmp + && let Some(ord_def_id) = cx + .tcx + .diagnostic_items(trait_impl.def_id.krate) + .name_to_id + .get(&sym::Ord) + && implements_trait( + cx, + hir_ty_to_ty(cx.tcx, imp.self_ty), + *ord_def_id, + trait_impl.substs, + ) + { + if block.stmts.is_empty() + && let Some(expr) = block.expr + && let ExprKind::Call( + Expr { + kind: ExprKind::Path(some_path), + hir_id: some_hir_id, + .. + }, + [cmp_expr], + ) = expr.kind + && is_res_lang_ctor(cx, cx.qpath_res(some_path, *some_hir_id), LangItem::OptionSome) + && let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind + && cmp_path.ident.name == sym::cmp + && let Res::Local(..) = path_res(cx, other_expr) + {} else { + span_lint_and_then( + cx, + NEEDLESS_PARTIAL_ORD_IMPL, + item.span, + "manual implementation of `PartialOrd` when `Ord` is already implemented", + |diag| { + let (help, app) = if let Some(other) = body.params.get(0) + && let PatKind::Binding(_, _, other_ident, ..) = other.pat.kind + { + ( + Cow::Owned(format!("{{ Some(self.cmp({})) }}", other_ident.name)), + Applicability::Unspecified, + ) + } else { + (Cow::Borrowed("{ Some(self.cmp(...)) }"), Applicability::HasPlaceholders) + }; + + diag.span_suggestion( + block.span, + "change this to", + help, + app, + ); + } + ); + } + } + } +} diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed index d6774c035..33221c777 100644 --- a/tests/ui/bool_comparison.fixed +++ b/tests/ui/bool_comparison.fixed @@ -2,6 +2,7 @@ #![allow(clippy::needless_if)] #![warn(clippy::bool_comparison)] +#![allow(clippy::needless_partial_ord_impl)] fn main() { let x = true; diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index c0483fd73..5f909bd2a 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -2,6 +2,7 @@ #![allow(clippy::needless_if)] #![warn(clippy::bool_comparison)] +#![allow(clippy::needless_partial_ord_impl)] fn main() { let x = true; diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr index f4dded365..19bdf3013 100644 --- a/tests/ui/bool_comparison.stderr +++ b/tests/ui/bool_comparison.stderr @@ -1,5 +1,5 @@ error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:8:8 + --> $DIR/bool_comparison.rs:9:8 | LL | if x == true { | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -7,127 +7,127 @@ LL | if x == true { = note: `-D clippy::bool-comparison` implied by `-D warnings` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:13:8 + --> $DIR/bool_comparison.rs:14:8 | LL | if x == false { | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:18:8 + --> $DIR/bool_comparison.rs:19:8 | LL | if true == x { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:23:8 + --> $DIR/bool_comparison.rs:24:8 | LL | if false == x { | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against true can be replaced by a negation - --> $DIR/bool_comparison.rs:28:8 + --> $DIR/bool_comparison.rs:29:8 | LL | if x != true { | ^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against false are unnecessary - --> $DIR/bool_comparison.rs:33:8 + --> $DIR/bool_comparison.rs:34:8 | LL | if x != false { | ^^^^^^^^^^ help: try simplifying it as shown: `x` error: inequality checks against true can be replaced by a negation - --> $DIR/bool_comparison.rs:38:8 + --> $DIR/bool_comparison.rs:39:8 | LL | if true != x { | ^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against false are unnecessary - --> $DIR/bool_comparison.rs:43:8 + --> $DIR/bool_comparison.rs:44:8 | LL | if false != x { | ^^^^^^^^^^ help: try simplifying it as shown: `x` error: less than comparison against true can be replaced by a negation - --> $DIR/bool_comparison.rs:48:8 + --> $DIR/bool_comparison.rs:49:8 | LL | if x < true { | ^^^^^^^^ help: try simplifying it as shown: `!x` error: greater than checks against false are unnecessary - --> $DIR/bool_comparison.rs:53:8 + --> $DIR/bool_comparison.rs:54:8 | LL | if false < x { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: greater than checks against false are unnecessary - --> $DIR/bool_comparison.rs:58:8 + --> $DIR/bool_comparison.rs:59:8 | LL | if x > false { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: less than comparison against true can be replaced by a negation - --> $DIR/bool_comparison.rs:63:8 + --> $DIR/bool_comparison.rs:64:8 | LL | if true > x { | ^^^^^^^^ help: try simplifying it as shown: `!x` error: order comparisons between booleans can be simplified - --> $DIR/bool_comparison.rs:69:8 + --> $DIR/bool_comparison.rs:70:8 | LL | if x < y { | ^^^^^ help: try simplifying it as shown: `!x & y` error: order comparisons between booleans can be simplified - --> $DIR/bool_comparison.rs:74:8 + --> $DIR/bool_comparison.rs:75:8 | LL | if x > y { | ^^^^^ help: try simplifying it as shown: `x & !y` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:122:8 + --> $DIR/bool_comparison.rs:123:8 | LL | if a == !b {}; | ^^^^^^^ help: try simplifying it as shown: `a != b` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:123:8 + --> $DIR/bool_comparison.rs:124:8 | LL | if !a == b {}; | ^^^^^^^ help: try simplifying it as shown: `a != b` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:127:8 + --> $DIR/bool_comparison.rs:128:8 | LL | if b == !a {}; | ^^^^^^^ help: try simplifying it as shown: `b != a` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:128:8 + --> $DIR/bool_comparison.rs:129:8 | LL | if !b == a {}; | ^^^^^^^ help: try simplifying it as shown: `b != a` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:152:8 + --> $DIR/bool_comparison.rs:153:8 | LL | if false == m!(func) {} | ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:153:8 + --> $DIR/bool_comparison.rs:154:8 | LL | if m!(func) == false {} | ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)` error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:154:8 + --> $DIR/bool_comparison.rs:155:8 | LL | if true == m!(func) {} | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)` error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:155:8 + --> $DIR/bool_comparison.rs:156:8 | LL | if m!(func) == true {} | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)` diff --git a/tests/ui/derive_ord_xor_partial_ord.rs b/tests/ui/derive_ord_xor_partial_ord.rs index 6f12d36d7..5d1a69332 100644 --- a/tests/ui/derive_ord_xor_partial_ord.rs +++ b/tests/ui/derive_ord_xor_partial_ord.rs @@ -1,5 +1,6 @@ #![warn(clippy::derive_ord_xor_partial_ord)] #![allow(clippy::unnecessary_wraps)] +#![allow(clippy::needless_partial_ord_impl)] use std::cmp::Ordering; diff --git a/tests/ui/derive_ord_xor_partial_ord.stderr b/tests/ui/derive_ord_xor_partial_ord.stderr index 58efbb854..bd1488348 100644 --- a/tests/ui/derive_ord_xor_partial_ord.stderr +++ b/tests/ui/derive_ord_xor_partial_ord.stderr @@ -1,11 +1,11 @@ error: you are deriving `Ord` but have implemented `PartialOrd` explicitly - --> $DIR/derive_ord_xor_partial_ord.rs:21:10 + --> $DIR/derive_ord_xor_partial_ord.rs:22:10 | LL | #[derive(Ord, PartialEq, Eq)] | ^^^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:24:1 + --> $DIR/derive_ord_xor_partial_ord.rs:25:1 | LL | impl PartialOrd for DeriveOrd { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -13,20 +13,20 @@ LL | impl PartialOrd for DeriveOrd { = note: this error originates in the derive macro `Ord` (in Nightly builds, run with -Z macro-backtrace for more info) error: you are deriving `Ord` but have implemented `PartialOrd` explicitly - --> $DIR/derive_ord_xor_partial_ord.rs:30:10 + --> $DIR/derive_ord_xor_partial_ord.rs:31:10 | LL | #[derive(Ord, PartialEq, Eq)] | ^^^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:33:1 + --> $DIR/derive_ord_xor_partial_ord.rs:34:1 | LL | impl PartialOrd for DeriveOrdWithExplicitTypeVariable { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the derive macro `Ord` (in Nightly builds, run with -Z macro-backtrace for more info) error: you are implementing `Ord` explicitly but have derived `PartialOrd` - --> $DIR/derive_ord_xor_partial_ord.rs:42:1 + --> $DIR/derive_ord_xor_partial_ord.rs:43:1 | LL | / impl std::cmp::Ord for DerivePartialOrd { LL | | fn cmp(&self, other: &Self) -> Ordering { @@ -36,14 +36,14 @@ LL | | } | |_^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:39:10 + --> $DIR/derive_ord_xor_partial_ord.rs:40:10 | LL | #[derive(PartialOrd, PartialEq, Eq)] | ^^^^^^^^^^ = note: this error originates in the derive macro `PartialOrd` (in Nightly builds, run with -Z macro-backtrace for more info) error: you are implementing `Ord` explicitly but have derived `PartialOrd` - --> $DIR/derive_ord_xor_partial_ord.rs:62:5 + --> $DIR/derive_ord_xor_partial_ord.rs:63:5 | LL | / impl Ord for DerivePartialOrdInUseOrd { LL | | fn cmp(&self, other: &Self) -> Ordering { @@ -53,7 +53,7 @@ LL | | } | |_____^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:59:14 + --> $DIR/derive_ord_xor_partial_ord.rs:60:14 | LL | #[derive(PartialOrd, PartialEq, Eq)] | ^^^^^^^^^^ diff --git a/tests/ui/manual_partial_ord_impl.stderr b/tests/ui/manual_partial_ord_impl.stderr deleted file mode 100644 index 100ddb46e..000000000 --- a/tests/ui/manual_partial_ord_impl.stderr +++ /dev/null @@ -1,28 +0,0 @@ -error: manual implementation of `PartialOrd` when `Ord` is already implemented - --> $DIR/manual_partial_ord_impl.rs:18:1 - | -LL | / impl PartialOrd for A { -LL | | fn partial_cmp(&self, other: &Self) -> Option { - | | _____________________________________________________________- -LL | || todo!(); -LL | || } - | ||_____- help: change this to: `{ Some(self.cmp(other)) }` -LL | | } - | |__^ - | - = note: `-D clippy::manual-partial-ord-impl` implied by `-D warnings` - -error: manual implementation of `PartialOrd` when `Ord` is already implemented - --> $DIR/manual_partial_ord_impl.rs:52:1 - | -LL | / impl PartialOrd for C { -LL | | fn partial_cmp(&self, _: &Self) -> Option { -LL | | todo!(); -LL | | } -LL | | } - | |_^ - | - = help: return the value of `self.cmp` wrapped in `Some` instead - -error: aborting due to 2 previous errors - diff --git a/tests/ui/needless_partial_ord_impl.fixed b/tests/ui/needless_partial_ord_impl.fixed new file mode 100644 index 000000000..7b47773ad --- /dev/null +++ b/tests/ui/needless_partial_ord_impl.fixed @@ -0,0 +1,85 @@ +//@run-rustfix +#![allow(unused)] +#![no_main] + +use std::cmp::Ordering; + +// lint + +#[derive(Eq, PartialEq)] +struct A(u32); + +impl Ord for A { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for A { + fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(self)) } +} + +// do not lint + +#[derive(Eq, PartialEq)] +struct B(u32); + +impl Ord for B { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for B { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +// lint, but we can't give a suggestion since &Self is not named + +#[derive(Eq, PartialEq)] +struct C(u32); + +impl Ord for C { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for C { + fn partial_cmp(&self, _: &Self) -> Option { Some(self.cmp(self)) } +} + +// do not lint derived + +#[derive(Eq, Ord, PartialEq, PartialOrd)] +struct D(u32); + +// do not lint if ord is not manually implemented + +#[derive(Eq, PartialEq)] +struct E(u32); + +impl PartialOrd for E { + fn partial_cmp(&self, other: &Self) -> Option { + todo!(); + } +} + +// do not lint since ord has more restrictive bounds + +#[derive(Eq, PartialEq)] +struct Uwu(A); + +impl Ord for Uwu { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for Uwu { + fn partial_cmp(&self, other: &Self) -> Option { + todo!(); + } +} diff --git a/tests/ui/manual_partial_ord_impl.rs b/tests/ui/needless_partial_ord_impl.rs similarity index 52% rename from tests/ui/manual_partial_ord_impl.rs rename to tests/ui/needless_partial_ord_impl.rs index 9e2c7e2ec..8f5c13373 100644 --- a/tests/ui/manual_partial_ord_impl.rs +++ b/tests/ui/needless_partial_ord_impl.rs @@ -1,5 +1,5 @@ +//@run-rustfix #![allow(unused)] -#![warn(clippy::manual_partial_ord_impl)] #![no_main] use std::cmp::Ordering; @@ -38,7 +38,7 @@ impl PartialOrd for B { } } -// lint, but we cannot give a suggestion since &Self is not named +// lint, but we can't give a suggestion since &Self is not named #[derive(Eq, PartialEq)] struct C(u32); @@ -55,3 +55,35 @@ impl PartialOrd for C { } } +// do not lint derived + +#[derive(Eq, Ord, PartialEq, PartialOrd)] +struct D(u32); + +// do not lint if ord is not manually implemented + +#[derive(Eq, PartialEq)] +struct E(u32); + +impl PartialOrd for E { + fn partial_cmp(&self, other: &Self) -> Option { + todo!(); + } +} + +// do not lint since ord has more restrictive bounds + +#[derive(Eq, PartialEq)] +struct Uwu(A); + +impl Ord for Uwu { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for Uwu { + fn partial_cmp(&self, other: &Self) -> Option { + todo!(); + } +} diff --git a/tests/ui/needless_partial_ord_impl.stderr b/tests/ui/needless_partial_ord_impl.stderr new file mode 100644 index 000000000..b8978462c --- /dev/null +++ b/tests/ui/needless_partial_ord_impl.stderr @@ -0,0 +1,28 @@ +error: manual implementation of `PartialOrd` when `Ord` is already implemented + --> $DIR/needless_partial_ord_impl.rs:18:1 + | +LL | / impl PartialOrd for A { +LL | | fn partial_cmp(&self, other: &Self) -> Option { + | | _____________________________________________________________- +LL | || todo!(); +LL | || } + | ||_____- help: change this to: `{ Some(self.cmp(self)) }` +LL | | } + | |__^ + | + = note: `#[deny(clippy::needless_partial_ord_impl)]` on by default + +error: manual implementation of `PartialOrd` when `Ord` is already implemented + --> $DIR/needless_partial_ord_impl.rs:52:1 + | +LL | / impl PartialOrd for C { +LL | | fn partial_cmp(&self, _: &Self) -> Option { + | | _________________________________________________________- +LL | || todo!(); +LL | || } + | ||_____- help: change this to: `{ Some(self.cmp(self)) }` +LL | | } + | |__^ + +error: aborting due to 2 previous errors +