Skip to content

Commit

Permalink
correct suggestion for unnecessary_sort_by in no_std (#13836)
Browse files Browse the repository at this point in the history
fix #11524

In a `no_std` environment, we can use `core::cmp::Reverse` instead of
`std::cmp::Reverse`.

----

changelog: [`unnecessary_sort_by`]: correct suggestion in `no_std`
  • Loading branch information
llogiq authored Dec 16, 2024
2 parents 968669b + 1a23b5b commit 063c5c1
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 3 deletions.
8 changes: 5 additions & 3 deletions clippy_lints/src/methods/unnecessary_sort_by.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_trait_method;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::implements_trait;
use clippy_utils::{is_trait_method, std_or_core};
use rustc_errors::Applicability;
use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
use rustc_lint::LateContext;
Expand Down Expand Up @@ -212,8 +212,10 @@ pub(super) fn check<'tcx>(
trigger.vec_name,
if is_unstable { "_unstable" } else { "" },
trigger.closure_arg,
if trigger.reverse {
format!("std::cmp::Reverse({})", trigger.closure_body)
if let Some(std_or_core) = std_or_core(cx)
&& trigger.reverse
{
format!("{}::cmp::Reverse({})", std_or_core, trigger.closure_body)
} else {
trigger.closure_body.to_string()
},
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/unnecessary_sort_by_no_std.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![no_std]
extern crate alloc;
use alloc::vec;
use alloc::vec::Vec;

fn issue_11524() -> Vec<i32> {
let mut vec = vec![1, 2, 3];

// Should lint and suggest `vec.sort_by_key(|a| a + 1);`
vec.sort_by_key(|a| a + 1);
vec
}

fn issue_11524_2() -> Vec<i32> {
let mut vec = vec![1, 2, 3];

// Should lint and suggest `vec.sort_by_key(|b| core::cmp::Reverse(b + 1));`
vec.sort_by_key(|b| core::cmp::Reverse(b + 1));
vec
}
20 changes: 20 additions & 0 deletions tests/ui/unnecessary_sort_by_no_std.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![no_std]
extern crate alloc;
use alloc::vec;
use alloc::vec::Vec;

fn issue_11524() -> Vec<i32> {
let mut vec = vec![1, 2, 3];

// Should lint and suggest `vec.sort_by_key(|a| a + 1);`
vec.sort_by(|a, b| (a + 1).cmp(&(b + 1)));
vec
}

fn issue_11524_2() -> Vec<i32> {
let mut vec = vec![1, 2, 3];

// Should lint and suggest `vec.sort_by_key(|b| core::cmp::Reverse(b + 1));`
vec.sort_by(|a, b| (b + 1).cmp(&(a + 1)));
vec
}
17 changes: 17 additions & 0 deletions tests/ui/unnecessary_sort_by_no_std.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: consider using `sort_by_key`
--> tests/ui/unnecessary_sort_by_no_std.rs:10:5
|
LL | vec.sort_by(|a, b| (a + 1).cmp(&(b + 1)));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| a + 1)`
|
= note: `-D clippy::unnecessary-sort-by` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_sort_by)]`

error: consider using `sort_by_key`
--> tests/ui/unnecessary_sort_by_no_std.rs:18:5
|
LL | vec.sort_by(|a, b| (b + 1).cmp(&(a + 1)));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| core::cmp::Reverse(b + 1))`

error: aborting due to 2 previous errors

0 comments on commit 063c5c1

Please sign in to comment.