about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-06-03 15:37:41 +0000
committerGitHub <noreply@github.com>2025-06-03 15:37:41 +0000
commita4debd23453c05ab0fc278ed4e544458bac1985f (patch)
treeba2aaae243230d94004cb6950168c2e6dd0a6b79
parenteafef8473605f2f9fc5689ef4ba70fc590930d52 (diff)
parent3b64c5c465ff1c9da8d0f6a2e2004299709a579a (diff)
downloadrust-a4debd23453c05ab0fc278ed4e544458bac1985f.tar.gz
rust-a4debd23453c05ab0fc278ed4e544458bac1985f.zip
Lint reversed ordering in partial ord impl (#14945)
Fixes rust-lang/rust-clippy#14574

Lint reversed ordering in partial ord impl

changelog: [`non_canonical_partial_ord_impl`] lint reversed ordering
-rw-r--r--clippy_lints/src/non_canonical_impls.rs9
-rw-r--r--tests/ui/non_canonical_partial_ord_impl.fixed16
-rw-r--r--tests/ui/non_canonical_partial_ord_impl.rs18
-rw-r--r--tests/ui/non_canonical_partial_ord_impl.stderr15
4 files changed, 56 insertions, 2 deletions
diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs
index 93865197ec9..04b09276966 100644
--- a/clippy_lints/src/non_canonical_impls.rs
+++ b/clippy_lints/src/non_canonical_impls.rs
@@ -293,7 +293,14 @@ fn self_cmp_call<'tcx>(
         ExprKind::Call(path, [_, _]) => path_res(cx, path)
             .opt_def_id()
             .is_some_and(|def_id| cx.tcx.is_diagnostic_item(sym::ord_cmp_method, def_id)),
-        ExprKind::MethodCall(_, _, [_other], ..) => {
+        ExprKind::MethodCall(_, recv, [_], ..) => {
+            let ExprKind::Path(path) = recv.kind else {
+                return false;
+            };
+            if last_path_segment(&path).ident.name != kw::SelfLower {
+                return false;
+            }
+
             // We can set this to true here no matter what as if it's a `MethodCall` and goes to the
             // `else` branch, it must be a method named `cmp` that isn't `Ord::cmp`
             *needs_fully_qualified = true;
diff --git a/tests/ui/non_canonical_partial_ord_impl.fixed b/tests/ui/non_canonical_partial_ord_impl.fixed
index 23dbee5a084..6915e1984fb 100644
--- a/tests/ui/non_canonical_partial_ord_impl.fixed
+++ b/tests/ui/non_canonical_partial_ord_impl.fixed
@@ -195,3 +195,19 @@ impl PartialOrd for K {
     //~^ non_canonical_partial_ord_impl
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
 }
+
+// #14574, check that partial_cmp invokes other.cmp
+
+#[derive(Eq, PartialEq)]
+struct L(u32);
+
+impl Ord for L {
+    fn cmp(&self, other: &Self) -> Ordering {
+        todo!();
+    }
+}
+
+impl PartialOrd for L {
+    //~^ non_canonical_partial_ord_impl
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
+}
diff --git a/tests/ui/non_canonical_partial_ord_impl.rs b/tests/ui/non_canonical_partial_ord_impl.rs
index 12f055a542b..7ce4cdc9aec 100644
--- a/tests/ui/non_canonical_partial_ord_impl.rs
+++ b/tests/ui/non_canonical_partial_ord_impl.rs
@@ -201,3 +201,21 @@ impl PartialOrd for K {
         Ordering::Greater.into()
     }
 }
+
+// #14574, check that partial_cmp invokes other.cmp
+
+#[derive(Eq, PartialEq)]
+struct L(u32);
+
+impl Ord for L {
+    fn cmp(&self, other: &Self) -> Ordering {
+        todo!();
+    }
+}
+
+impl PartialOrd for L {
+    //~^ non_canonical_partial_ord_impl
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(other.cmp(self))
+    }
+}
diff --git a/tests/ui/non_canonical_partial_ord_impl.stderr b/tests/ui/non_canonical_partial_ord_impl.stderr
index c7de968588f..9bd6b1f726d 100644
--- a/tests/ui/non_canonical_partial_ord_impl.stderr
+++ b/tests/ui/non_canonical_partial_ord_impl.stderr
@@ -44,5 +44,18 @@ LL | ||     }
 LL | |  }
    | |__^
 
-error: aborting due to 3 previous errors
+error: non-canonical implementation of `partial_cmp` on an `Ord` type
+  --> tests/ui/non_canonical_partial_ord_impl.rs:216:1
+   |
+LL | /  impl PartialOrd for L {
+LL | |
+LL | |      fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+   | | _____________________________________________________________-
+LL | ||         Some(other.cmp(self))
+LL | ||     }
+   | ||_____- help: change this to: `{ Some(self.cmp(other)) }`
+LL | |  }
+   | |__^
+
+error: aborting due to 4 previous errors