Skip to content

Commit 70f36e0

Browse files
borsbors
committed
Auto merge of #7824 - dswij:unnecessary_sort_by, r=llogiq
`unnecessary_sort_by` checks if argument implements `Ord` trait closes #7822 changelog: [`unnecessary_sort_by`] now checks if argument implements `Ord` trait
2 parents 4996e17 + e4ac4c2 commit 70f36e0

File tree

4 files changed

+30
-15
lines changed

4 files changed

+30
-15
lines changed

‎clippy_lints/src/unnecessary_sort_by.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::sugg::Sugg;
3-
use clippy_utils::ty::is_type_diagnostic_item;
3+
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
66
use rustc_hir::{Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
@@ -193,10 +193,15 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> {
193193
let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
194194
let unstable = name == "sort_unstable_by";
195195

196+
if_chain! {
196197
if let ExprKind::Path(QPath::Resolved(_, Path {
197198
segments: [PathSegment { ident: left_name, .. }], ..
198-
})) = &left_expr.kind {
199-
if left_name == left_ident {
199+
})) = &left_expr.kind;
200+
if left_name == left_ident;
201+
if cx.tcx.get_diagnostic_item(sym::Ord).map_or(false, |id| {
202+
implements_trait(cx, cx.typeck_results().expr_ty(left_expr), id, &[])
203+
});
204+
then {
200205
return Some(LintTrigger::Sort(SortDetection { vec_name, unstable }));
201206
}
202207
}

‎tests/ui/unnecessary_sort_by.fixed

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(clippy::stable_sort_primitive)]
44

5+
use std::cell::Ref;
56
use std::cmp::Reverse;
67

78
fn unnecessary_sort_by() {
@@ -33,6 +34,10 @@ fn unnecessary_sort_by() {
3334
// `Reverse(b)` would borrow in the following cases, don't lint
3435
vec.sort_by(|a, b| b.cmp(a));
3536
vec.sort_unstable_by(|a, b| b.cmp(a));
37+
38+
// No warning if element does not implement `Ord`
39+
let mut vec: Vec<Ref<usize>> = Vec::new();
40+
vec.sort_unstable_by(|a, b| a.cmp(b));
3641
}
3742

3843
// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key`

‎tests/ui/unnecessary_sort_by.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(clippy::stable_sort_primitive)]
44

5+
use std::cell::Ref;
56
use std::cmp::Reverse;
67

78
fn unnecessary_sort_by() {
@@ -33,6 +34,10 @@ fn unnecessary_sort_by() {
3334
// `Reverse(b)` would borrow in the following cases, don't lint
3435
vec.sort_by(|a, b| b.cmp(a));
3536
vec.sort_unstable_by(|a, b| b.cmp(a));
37+
38+
// No warning if element does not implement `Ord`
39+
let mut vec: Vec<Ref<usize>> = Vec::new();
40+
vec.sort_unstable_by(|a, b| a.cmp(b));
3641
}
3742

3843
// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key`

‎tests/ui/unnecessary_sort_by.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,73 @@
11
error: use Vec::sort here instead
2-
--> $DIR/unnecessary_sort_by.rs:14:5
2+
--> $DIR/unnecessary_sort_by.rs:15:5
33
|
44
LL | vec.sort_by(|a, b| a.cmp(b));
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()`
66
|
77
= note: `-D clippy::unnecessary-sort-by` implied by `-D warnings`
88

99
error: use Vec::sort here instead
10-
--> $DIR/unnecessary_sort_by.rs:15:5
10+
--> $DIR/unnecessary_sort_by.rs:16:5
1111
|
1212
LL | vec.sort_unstable_by(|a, b| a.cmp(b));
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()`
1414

1515
error: use Vec::sort_by_key here instead
16-
--> $DIR/unnecessary_sort_by.rs:16:5
16+
--> $DIR/unnecessary_sort_by.rs:17:5
1717
|
1818
LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs()));
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())`
2020

2121
error: use Vec::sort_by_key here instead
22-
--> $DIR/unnecessary_sort_by.rs:17:5
22+
--> $DIR/unnecessary_sort_by.rs:18:5
2323
|
2424
LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b)));
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| id(-a))`
2626

2727
error: use Vec::sort_by_key here instead
28-
--> $DIR/unnecessary_sort_by.rs:20:5
28+
--> $DIR/unnecessary_sort_by.rs:21:5
2929
|
3030
LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs()));
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| Reverse((b + 5).abs()))`
3232

3333
error: use Vec::sort_by_key here instead
34-
--> $DIR/unnecessary_sort_by.rs:21:5
34+
--> $DIR/unnecessary_sort_by.rs:22:5
3535
|
3636
LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a)));
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| Reverse(id(-b)))`
3838

3939
error: use Vec::sort_by_key here instead
40-
--> $DIR/unnecessary_sort_by.rs:31:5
40+
--> $DIR/unnecessary_sort_by.rs:32:5
4141
|
4242
LL | vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs()));
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())`
4444

4545
error: use Vec::sort_by_key here instead
46-
--> $DIR/unnecessary_sort_by.rs:32:5
46+
--> $DIR/unnecessary_sort_by.rs:33:5
4747
|
4848
LL | vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs()));
4949
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| (***a).abs())`
5050

5151
error: use Vec::sort_by_key here instead
52-
--> $DIR/unnecessary_sort_by.rs:88:9
52+
--> $DIR/unnecessary_sort_by.rs:93:9
5353
|
5454
LL | args.sort_by(|a, b| a.name().cmp(&b.name()));
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())`
5656

5757
error: use Vec::sort_by_key here instead
58-
--> $DIR/unnecessary_sort_by.rs:89:9
58+
--> $DIR/unnecessary_sort_by.rs:94:9
5959
|
6060
LL | args.sort_unstable_by(|a, b| a.name().cmp(&b.name()));
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|a| a.name())`
6262

6363
error: use Vec::sort_by_key here instead
64-
--> $DIR/unnecessary_sort_by.rs:91:9
64+
--> $DIR/unnecessary_sort_by.rs:96:9
6565
|
6666
LL | args.sort_by(|a, b| b.name().cmp(&a.name()));
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| Reverse(b.name()))`
6868

6969
error: use Vec::sort_by_key here instead
70-
--> $DIR/unnecessary_sort_by.rs:92:9
70+
--> $DIR/unnecessary_sort_by.rs:97:9
7171
|
7272
LL | args.sort_unstable_by(|a, b| b.name().cmp(&a.name()));
7373
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| Reverse(b.name()))`

0 commit comments

Comments
 (0)