Fix unnecessary_sort_by lint method consistency in message and suggestion (#15416)

The `unnecessary_sort_by` lint displays different method names in
message and suggestion, which is a bit confusing.

Also got a question about `UNNECESSARY_SORT_BY` lint definition. Should
we extend its message to also cover *_unstable_* methods?

changelog: [`unnecessary-sort-by`]: sort method consistency in message
and suggestion

Author: samueltardieu

clippy_lints/src/methods/unnecessary_sort_by.rs

@@ -200,44 +200,50 @@ pub(super) fn check<'tcx>(
     is_unstable: bool,
 ) {
     match detect_lint(cx, expr, recv, arg) {
-        Some(LintTrigger::SortByKey(trigger)) => span_lint_and_sugg(
-            cx,
-            UNNECESSARY_SORT_BY,
-            expr.span,
-            "consider using `sort_by_key`",
-            "try",
-            format!(
-                "{}.sort{}_by_key(|{}| {})",
-                trigger.vec_name,
-                if is_unstable { "_unstable" } else { "" },
-                trigger.closure_arg,
-                if let Some(std_or_core) = std_or_core(cx)
-                    && trigger.reverse
-                {
-                    format!("{}::cmp::Reverse({})", std_or_core, trigger.closure_body)
+        Some(LintTrigger::SortByKey(trigger)) => {
+            let method = if is_unstable {
+                "sort_unstable_by_key"
+            } else {
+                "sort_by_key"
+            };
+            span_lint_and_sugg(
+                cx,
+                UNNECESSARY_SORT_BY,
+                expr.span,
+                format!("consider using `{method}`"),
+                "try",
+                format!(
+                    "{}.{}(|{}| {})",
+                    trigger.vec_name,
+                    method,
+                    trigger.closure_arg,
+                    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
+                    },
+                ),
+                if trigger.reverse {
+                    Applicability::MaybeIncorrect
                 } else {
-                    trigger.closure_body
+                    Applicability::MachineApplicable
                 },
-            ),
-            if trigger.reverse {
-                Applicability::MaybeIncorrect
-            } else {
-                Applicability::MachineApplicable
-            },
-        ),
-        Some(LintTrigger::Sort(trigger)) => span_lint_and_sugg(
-            cx,
-            UNNECESSARY_SORT_BY,
-            expr.span,
-            "consider using `sort`",
-            "try",
-            format!(
-                "{}.sort{}()",
-                trigger.vec_name,
-                if is_unstable { "_unstable" } else { "" },
-            ),
-            Applicability::MachineApplicable,
-        ),
+            );
+        },
+        Some(LintTrigger::Sort(trigger)) => {
+            let method = if is_unstable { "sort_unstable" } else { "sort" };
+            span_lint_and_sugg(
+                cx,
+                UNNECESSARY_SORT_BY,
+                expr.span,
+                format!("consider using `{method}`"),
+                "try",
+                format!("{}.{}()", trigger.vec_name, method),
+                Applicability::MachineApplicable,
+            );
+        },
         None => {},
     }
 }

tests/ui/unnecessary_sort_by.stderr

@@ -7,7 +7,7 @@ LL |     vec.sort_by(|a, b| a.cmp(b));
    = 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`
+error: consider using `sort_unstable`
   --> tests/ui/unnecessary_sort_by.rs:14:5
    |
 LL |     vec.sort_unstable_by(|a, b| a.cmp(b));
@@ -19,7 +19,7 @@ error: consider using `sort_by_key`
 LL |     vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs()));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())`
 
-error: consider using `sort_by_key`
+error: consider using `sort_unstable_by_key`
   --> tests/ui/unnecessary_sort_by.rs:18:5
    |
 LL |     vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b)));
@@ -31,7 +31,7 @@ error: consider using `sort_by_key`
 LL |     vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs()));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| std::cmp::Reverse((b + 5).abs()))`
 
-error: consider using `sort_by_key`
+error: consider using `sort_unstable_by_key`
   --> tests/ui/unnecessary_sort_by.rs:24:5
    |
 LL |     vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a)));
@@ -43,7 +43,7 @@ error: consider using `sort_by_key`
 LL |     vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs()));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())`
 
-error: consider using `sort_by_key`
+error: consider using `sort_unstable_by_key`
   --> tests/ui/unnecessary_sort_by.rs:37:5
    |
 LL |     vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs()));
@@ -55,7 +55,7 @@ error: consider using `sort_by_key`
 LL |         args.sort_by(|a, b| a.name().cmp(&b.name()));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())`
 
-error: consider using `sort_by_key`
+error: consider using `sort_unstable_by_key`
   --> tests/ui/unnecessary_sort_by.rs:99:9
    |
 LL |         args.sort_unstable_by(|a, b| a.name().cmp(&b.name()));
@@ -67,7 +67,7 @@ error: consider using `sort_by_key`
 LL |         args.sort_by(|a, b| b.name().cmp(&a.name()));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| std::cmp::Reverse(b.name()))`
 
-error: consider using `sort_by_key`
+error: consider using `sort_unstable_by_key`
   --> tests/ui/unnecessary_sort_by.rs:104:9
    |
 LL |         args.sort_unstable_by(|a, b| b.name().cmp(&a.name()));