diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/unnecessary_sort_by.rs
index c0858ec4c88..33d8331c292 100644
--- a/clippy_lints/src/unnecessary_sort_by.rs
+++ b/clippy_lints/src/unnecessary_sort_by.rs
@@ -18,15 +18,25 @@ declare_clippy_lint! {
/// possible) than to use `Vec::sort_by` and and a more complicated
/// closure.
///
- /// **Known problems:** None.
+ /// **Known problems:**
+ /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't
+ /// imported by a use statement in the current frame, then a `use`
+ /// statement that imports it will need to be added (which this lint
+ /// can't do).
///
/// **Example:**
///
/// ```rust
- /// vec.sort_by(|a, b| a.foo().cmp(b.foo()));
+ /// # struct A;
+ /// # impl A { fn foo(&self) {} }
+ /// # let mut vec: Vec = Vec::new();
+ /// vec.sort_by(|a, b| a.foo().cmp(&b.foo()));
/// ```
/// Use instead:
/// ```rust
+ /// # struct A;
+ /// # impl A { fn foo(&self) {} }
+ /// # let mut vec: Vec = Vec::new();
/// vec.sort_by_key(|a| a.foo());
/// ```
pub UNNECESSARY_SORT_BY,
@@ -50,6 +60,7 @@ struct SortByKeyDetection {
vec_name: String,
closure_arg: String,
closure_body: String,
+ reverse: bool,
unstable: bool,
}
@@ -172,16 +183,16 @@ fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option
if let ExprKind::MethodCall(method_path, _, [ref left_expr, ref right_expr]) = &closure_body.value.kind;
if method_path.ident.name.to_ident_string() == "cmp";
then {
- let (closure_body, closure_arg) = if mirrored_exprs(
+ let (closure_body, closure_arg, reverse) = if mirrored_exprs(
&cx,
&left_expr,
&left_ident,
&right_expr,
&right_ident
) {
- (Sugg::hir(cx, &left_expr, "..").to_string(), left_ident.name.to_string())
+ (Sugg::hir(cx, &left_expr, "..").to_string(), left_ident.name.to_string(), false)
} else if mirrored_exprs(&cx, &left_expr, &right_ident, &right_expr, &left_ident) {
- (format!("Reverse({})", Sugg::hir(cx, &left_expr, "..").to_string()), right_ident.name.to_string())
+ (Sugg::hir(cx, &left_expr, "..").to_string(), right_ident.name.to_string(), true)
} else {
return None;
};
@@ -196,7 +207,13 @@ fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option
Some(LintTrigger::Sort(SortDetection { vec_name, unstable }))
}
else {
- Some(LintTrigger::SortByKey(SortByKeyDetection { vec_name, unstable, closure_arg, closure_body }))
+ Some(LintTrigger::SortByKey(SortByKeyDetection {
+ vec_name,
+ unstable,
+ closure_arg,
+ closure_body,
+ reverse
+ }))
}
}
} else {
@@ -219,9 +236,17 @@ impl LateLintPass<'_, '_> for UnnecessarySortBy {
trigger.vec_name,
if trigger.unstable { "_unstable" } else { "" },
trigger.closure_arg,
- trigger.closure_body,
+ if trigger.reverse {
+ format!("Reverse({})", trigger.closure_body)
+ } else {
+ trigger.closure_body.to_string()
+ },
),
- Applicability::MachineApplicable,
+ if trigger.reverse {
+ Applicability::MaybeIncorrect
+ } else {
+ Applicability::MachineApplicable
+ },
),
Some(LintTrigger::Sort(trigger)) => utils::span_lint_and_sugg(
cx,
diff --git a/tests/ui/unnecessary_sort_by.fixed b/tests/ui/unnecessary_sort_by.fixed
index 4521ae38d49..779fd57707a 100644
--- a/tests/ui/unnecessary_sort_by.fixed
+++ b/tests/ui/unnecessary_sort_by.fixed
@@ -10,16 +10,17 @@ fn main() {
let mut vec: Vec = vec![3, 6, 1, 2, 5];
// Forward examples
vec.sort();
+ vec.sort_unstable();
vec.sort_by_key(|&a| (a + 5).abs());
- vec.sort_by_key(|&a| id(-a));
+ vec.sort_unstable_by_key(|&a| id(-a));
// Reverse examples
vec.sort_by_key(|&b| Reverse(b));
vec.sort_by_key(|&b| Reverse((b + 5).abs()));
- vec.sort_by_key(|&b| Reverse(id(-b)));
+ vec.sort_unstable_by_key(|&b| Reverse(id(-b)));
// Negative examples (shouldn't be changed)
let c = &7;
vec.sort_by(|a, b| (b - a).cmp(&(a - b)));
vec.sort_by(|_, b| b.cmp(&5));
vec.sort_by(|_, b| b.cmp(c));
- vec.sort_by(|a, _| a.cmp(c));
+ vec.sort_unstable_by(|a, _| a.cmp(c));
}
diff --git a/tests/ui/unnecessary_sort_by.rs b/tests/ui/unnecessary_sort_by.rs
index fdb5a823369..0485a5630af 100644
--- a/tests/ui/unnecessary_sort_by.rs
+++ b/tests/ui/unnecessary_sort_by.rs
@@ -10,16 +10,17 @@ fn main() {
let mut vec: Vec = vec![3, 6, 1, 2, 5];
// Forward examples
vec.sort_by(|a, b| a.cmp(b));
+ vec.sort_unstable_by(|a, b| a.cmp(b));
vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs()));
- vec.sort_by(|a, b| id(-a).cmp(&id(-b)));
+ vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b)));
// Reverse examples
vec.sort_by(|a, b| b.cmp(a));
vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs()));
- vec.sort_by(|a, b| id(-b).cmp(&id(-a)));
+ vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a)));
// Negative examples (shouldn't be changed)
let c = &7;
vec.sort_by(|a, b| (b - a).cmp(&(a - b)));
vec.sort_by(|_, b| b.cmp(&5));
vec.sort_by(|_, b| b.cmp(c));
- vec.sort_by(|a, _| a.cmp(c));
+ vec.sort_unstable_by(|a, _| a.cmp(c));
}
diff --git a/tests/ui/unnecessary_sort_by.stderr b/tests/ui/unnecessary_sort_by.stderr
index b6365c1709d..903b6e5099c 100644
--- a/tests/ui/unnecessary_sort_by.stderr
+++ b/tests/ui/unnecessary_sort_by.stderr
@@ -6,35 +6,41 @@ LL | vec.sort_by(|a, b| a.cmp(b));
|
= note: `-D clippy::unnecessary-sort-by` implied by `-D warnings`
-error: use Vec::sort_by_key here instead
+error: use Vec::sort here instead
--> $DIR/unnecessary_sort_by.rs:13: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:14: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:14:5
+ --> $DIR/unnecessary_sort_by.rs:15:5
|
-LL | vec.sort_by(|a, b| id(-a).cmp(&id(-b)));
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&a| id(-a))`
+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:16:5
+ --> $DIR/unnecessary_sort_by.rs:17:5
|
LL | vec.sort_by(|a, b| b.cmp(a));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse(b))`
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_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:18:5
+ --> $DIR/unnecessary_sort_by.rs:19:5
|
-LL | vec.sort_by(|a, b| id(-b).cmp(&id(-a)));
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse(id(-b)))`
+LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a)));
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|&b| Reverse(id(-b)))`
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors