Skip to content

Commit 3b64c5c

Browse files
Lint reversed ordering in partial ord impl
changelog: [non_canonical_partial_ord_impl] lint reversed ordering Co-authored-by: Samuel Tardieu <sam@rfc1149.net>
1 parent 551870d commit 3b64c5c

File tree

4 files changed

+56
-2
lines changed

4 files changed

+56
-2
lines changed

clippy_lints/src/non_canonical_impls.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,14 @@ fn self_cmp_call<'tcx>(
293293
ExprKind::Call(path, [_, _]) => path_res(cx, path)
294294
.opt_def_id()
295295
.is_some_and(|def_id| cx.tcx.is_diagnostic_item(sym::ord_cmp_method, def_id)),
296-
ExprKind::MethodCall(_, _, [_other], ..) => {
296+
ExprKind::MethodCall(_, recv, [_], ..) => {
297+
let ExprKind::Path(path) = recv.kind else {
298+
return false;
299+
};
300+
if last_path_segment(&path).ident.name != kw::SelfLower {
301+
return false;
302+
}
303+
297304
// We can set this to true here no matter what as if it's a `MethodCall` and goes to the
298305
// `else` branch, it must be a method named `cmp` that isn't `Ord::cmp`
299306
*needs_fully_qualified = true;

tests/ui/non_canonical_partial_ord_impl.fixed

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,19 @@ impl PartialOrd for K {
195195
//~^ non_canonical_partial_ord_impl
196196
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
197197
}
198+
199+
// #14574, check that partial_cmp invokes other.cmp
200+
201+
#[derive(Eq, PartialEq)]
202+
struct L(u32);
203+
204+
impl Ord for L {
205+
fn cmp(&self, other: &Self) -> Ordering {
206+
todo!();
207+
}
208+
}
209+
210+
impl PartialOrd for L {
211+
//~^ non_canonical_partial_ord_impl
212+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
213+
}

tests/ui/non_canonical_partial_ord_impl.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,21 @@ impl PartialOrd for K {
201201
Ordering::Greater.into()
202202
}
203203
}
204+
205+
// #14574, check that partial_cmp invokes other.cmp
206+
207+
#[derive(Eq, PartialEq)]
208+
struct L(u32);
209+
210+
impl Ord for L {
211+
fn cmp(&self, other: &Self) -> Ordering {
212+
todo!();
213+
}
214+
}
215+
216+
impl PartialOrd for L {
217+
//~^ non_canonical_partial_ord_impl
218+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
219+
Some(other.cmp(self))
220+
}
221+
}

tests/ui/non_canonical_partial_ord_impl.stderr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,18 @@ LL | || }
4444
LL | | }
4545
| |__^
4646

47-
error: aborting due to 3 previous errors
47+
error: non-canonical implementation of `partial_cmp` on an `Ord` type
48+
--> tests/ui/non_canonical_partial_ord_impl.rs:216:1
49+
|
50+
LL | / impl PartialOrd for L {
51+
LL | |
52+
LL | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
53+
| | _____________________________________________________________-
54+
LL | || Some(other.cmp(self))
55+
LL | || }
56+
| ||_____- help: change this to: `{ Some(self.cmp(other)) }`
57+
LL | | }
58+
| |__^
59+
60+
error: aborting due to 4 previous errors
4861

0 commit comments

Comments
 (0)