about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTimo <30553356+y21@users.noreply.github.com>2024-12-10 21:11:42 +0000
committerGitHub <noreply@github.com>2024-12-10 21:11:42 +0000
commit59740a8eb1081fba7a25b60ab7d7acae98f1d3bd (patch)
tree59ddc0d794292089fa39336d9893114636af97d9
parent2a28347897fcf9c87e38b753aeeadbbe72ae1e55 (diff)
parent69411e0e760820052d9f77f0e0a3635e7bc76a00 (diff)
Better help message for comparison_chain lint (#13762)
changelog: [`comparison_chain`]: give explicit help message showing a
clear suggestion

Close #13739
-rw-r--r--clippy_lints/src/comparison_chain.rs16
-rw-r--r--tests/ui/comparison_chain.rs13
-rw-r--r--tests/ui/comparison_chain.stderr53
3 files changed, 51 insertions, 31 deletions
diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs
index c85e3500ebd..61c92d441d0 100644
--- 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
index 266cee4c338..cab460d100d 100644
--- 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
index 96d8d819e6a..814004e3d4b 100644
--- a/tests/ui/comparison_chain.stderr
+++ b/tests/ui/comparison_chain.stderr
@@ -1,5 +1,5 @@
 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 | |
@@ -7,14 +7,13 @@ 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,12 +46,10 @@ 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 | |
@@ -64,12 +57,10 @@ 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