mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #7824 - dswij:unnecessary_sort_by, r=llogiq

Browse Source 
`unnecessary_sort_by` checks if argument implements `Ord` trait

closes #7822

changelog: [`unnecessary_sort_by`] now checks if argument implements `Ord` trait
This commit is contained in:
bors 2021-10-15 11:16:57 +00:00
commit 70f36e0454
4 changed files with 30 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
clippy_lints/src/unnecessary_sort_by.rs
View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use if_chain::if_chain;
use rustc_errors::Applicability;
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> {
let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
let unstable = name == "sort_unstable_by";
if_chain! {
if let ExprKind::Path(QPath::Resolved(_, Path {
segments: [PathSegment { ident: left_name, .. }], ..
})) = &left_expr.kind {
if left_name == left_ident {
})) = &left_expr.kind;
if left_name == left_ident;
if cx.tcx.get_diagnostic_item(sym::Ord).map_or(false, |id| {
implements_trait(cx, cx.typeck_results().expr_ty(left_expr), id, &[])
});
then {
return Some(LintTrigger::Sort(SortDetection { vec_name, unstable }));
}
}

5
tests/ui/unnecessary_sort_by.fixed
View File

@ -2,6 +2,7 @@
#![allow(clippy::stable_sort_primitive)]
use std::cell::Ref;
use std::cmp::Reverse;
fn unnecessary_sort_by() {
@ -33,6 +34,10 @@ fn unnecessary_sort_by() {
// `Reverse(b)` would borrow in the following cases, don't lint
vec.sort_by(|a, b| b.cmp(a));
vec.sort_unstable_by(|a, b| b.cmp(a));
// No warning if element does not implement `Ord`
let mut vec: Vec<Ref<usize>> = Vec::new();
vec.sort_unstable_by(|a, b| a.cmp(b));
}
// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key`

5
tests/ui/unnecessary_sort_by.rs
View File

@ -2,6 +2,7 @@
#![allow(clippy::stable_sort_primitive)]
use std::cell::Ref;
use std::cmp::Reverse;
fn unnecessary_sort_by() {
@ -33,6 +34,10 @@ fn id(x: isize) -> isize {
// `Reverse(b)` would borrow in the following cases, don't lint
vec.sort_by(|a, b| b.cmp(a));
vec.sort_unstable_by(|a, b| b.cmp(a));
// No warning if element does not implement `Ord`
let mut vec: Vec<Ref<usize>> = Vec::new();
vec.sort_unstable_by(|a, b| a.cmp(b));
}
// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key`

24
tests/ui/unnecessary_sort_by.stderr
View File

@ -1,5 +1,5 @@
error: use Vec::sort here instead
--> $DIR/unnecessary_sort_by.rs:14:5
--> $DIR/unnecessary_sort_by.rs:15:5
|
LL | vec.sort_by(|a, b| a.cmp(b));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()`
@ -7,67 +7,67 @@ LL | vec.sort_by(|a, b| a.cmp(b));
= note: `-D clippy::unnecessary-sort-by` implied by `-D warnings`
error: use Vec::sort here instead
--> $DIR/unnecessary_sort_by.rs:15:5
--> $DIR/unnecessary_sort_by.rs:16:5
|
LL | vec.sort_unstable_by(|a, b| a.cmp(b));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:16:5
--> $DIR/unnecessary_sort_by.rs:17:5
|
LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:17:5
--> $DIR/unnecessary_sort_by.rs:18:5
|
LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b)));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| id(-a))`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:20:5
--> $DIR/unnecessary_sort_by.rs:21:5
|
LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| Reverse((b + 5).abs()))`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:21:5
--> $DIR/unnecessary_sort_by.rs:22:5
|
LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a)));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| Reverse(id(-b)))`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:31:5
--> $DIR/unnecessary_sort_by.rs:32:5
|
LL | vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:32:5
--> $DIR/unnecessary_sort_by.rs:33:5
|
LL | vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| (***a).abs())`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:88:9
--> $DIR/unnecessary_sort_by.rs:93:9
|
LL | args.sort_by(|a, b| a.name().cmp(&b.name()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:89:9
--> $DIR/unnecessary_sort_by.rs:94:9
|
LL | args.sort_unstable_by(|a, b| a.name().cmp(&b.name()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|a| a.name())`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:91:9
--> $DIR/unnecessary_sort_by.rs:96:9
|
LL | args.sort_by(|a, b| b.name().cmp(&a.name()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| Reverse(b.name()))`
error: use Vec::sort_by_key here instead
--> $DIR/unnecessary_sort_by.rs:92:9
--> $DIR/unnecessary_sort_by.rs:97:9
|
LL | args.sort_unstable_by(|a, b| b.name().cmp(&a.name()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| Reverse(b.name()))`