Better help message for comparison_chain lint (#13762)

changelog: [`comparison_chain`]: give explicit help message showing a clear suggestion Close #13739
rust-lang · Dec 10, 2024 · 59740a8 · 59740a8
2 parents 2a28347 + 69411e0
commit 59740a8
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 31 deletions.
diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs
@@ -1,6 +1,8 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::implements_trait;
 use clippy_utils::{SpanlessEq, if_sequence, is_else_clause, is_in_const_context};
+use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
@@ -120,13 +122,19 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
                 return;
             }
         }
-        span_lint_and_help(
+        let ExprKind::Binary(_, lhs, rhs) = conds[0].kind else {
+            unreachable!();
+        };
+        let lhs = Sugg::hir(cx, lhs, "..").maybe_par();
+        let rhs = Sugg::hir(cx, rhs, "..").addr();
+        span_lint_and_sugg(
             cx,
             COMPARISON_CHAIN,
             expr.span,
             "`if` chain can be rewritten with `match`",
-            None,
-            "consider rewriting the `if` chain to use `cmp` and `match`",
+            "consider rewriting the `if` chain with `match`",
+            format!("match {lhs}.cmp({rhs}) {{...}}"),
+            Applicability::HasPlaceholders,
         );
     }
 }

diff --git a/tests/ui/comparison_chain.rs b/tests/ui/comparison_chain.rs
@@ -1,3 +1,4 @@
+//@no-rustfix
 #![allow(dead_code)]
 #![warn(clippy::comparison_chain)]
 
@@ -238,4 +239,16 @@ const fn sign_i8(n: i8) -> Sign {
     }
 }
 
+fn needs_parens() -> &'static str {
+    let (x, y) = (1, 2);
+    if x + 1 > y * 2 {
+        //~^ ERROR: `if` chain can be rewritten with `match`
+        "aa"
+    } else if x + 1 < y * 2 {
+        "bb"
+    } else {
+        "cc"
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/comparison_chain.stderr b/tests/ui/comparison_chain.stderr
@@ -1,20 +1,19 @@
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:14:5
+  --> tests/ui/comparison_chain.rs:15:5
    |
 LL | /     if x > y {
 LL | |
 LL | |         a()
 LL | |     } else if x < y {
 LL | |         b()
 LL | |     }
-   | |_____^
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
    |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
    = note: `-D clippy::comparison-chain` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::comparison_chain)]`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:28:5
+  --> tests/ui/comparison_chain.rs:29:5
    |
 LL | /     if x > y {
 LL | |
@@ -23,12 +22,10 @@ LL | |     } else if x < y {
 ...  |
 LL | |         c()
 LL | |     }
-   | |_____^
-   |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:37:5
+  --> tests/ui/comparison_chain.rs:38:5
    |
 LL | /     if x > y {
 LL | |
@@ -37,12 +34,10 @@ LL | |     } else if y > x {
 ...  |
 LL | |         c()
 LL | |     }
-   | |_____^
-   |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:46:5
+  --> tests/ui/comparison_chain.rs:47:5
    |
 LL | /     if x > 1 {
 LL | |
@@ -51,25 +46,21 @@ LL | |     } else if x < 1 {
 ...  |
 LL | |         c()
 LL | |     }
-   | |_____^
-   |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&1) {...}`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:121:5
+  --> tests/ui/comparison_chain.rs:122:5
    |
 LL | /     if x > y {
 LL | |
 LL | |         a()
 LL | |     } else if x < y {
 LL | |         b()
 LL | |     }
-   | |_____^
-   |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:128:5
+  --> tests/ui/comparison_chain.rs:129:5
    |
 LL | /     if x > y {
 LL | |
@@ -78,12 +69,10 @@ LL | |     } else if x < y {
 ...  |
 LL | |         c()
 LL | |     }
-   | |_____^
-   |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
 
 error: `if` chain can be rewritten with `match`
-  --> tests/ui/comparison_chain.rs:137:5
+  --> tests/ui/comparison_chain.rs:138:5
    |
 LL | /     if x > y {
 LL | |
@@ -92,9 +81,19 @@ LL | |     } else if y > x {
 ...  |
 LL | |         c()
 LL | |     }
-   | |_____^
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match x.cmp(&y) {...}`
+
+error: `if` chain can be rewritten with `match`
+  --> tests/ui/comparison_chain.rs:244:5
    |
-   = help: consider rewriting the `if` chain to use `cmp` and `match`
+LL | /     if x + 1 > y * 2 {
+LL | |
+LL | |         "aa"
+LL | |     } else if x + 1 < y * 2 {
+...  |
+LL | |         "cc"
+LL | |     }
+   | |_____^ help: consider rewriting the `if` chain with `match`: `match (x + 1).cmp(&(y * 2)) {...}`
 
-error: aborting due to 7 previous errors
+error: aborting due to 8 previous errors