fix incorrect suggestion for !(a >= b) as i32 == c

Author: CoCo-Japan-pan

clippy_lints/src/booleans.rs

@@ -83,7 +83,11 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
         _: Span,
         _: LocalDefId,
     ) {
-        NonminimalBoolVisitor { cx }.visit_body(body);
+        NonminimalBoolVisitor {
+            cx,
+            parent_expr: vec![],
+        }
+        .visit_body(body);
     }
 
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
@@ -176,13 +180,28 @@ fn check_inverted_bool_in_condition(
     );
 }
 
-fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
+fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>, parent_expr: Option<&Expr<'_>>) {
     if let ExprKind::Unary(UnOp::Not, inner) = &expr.kind
         && !expr.span.from_expansion()
         && !inner.span.from_expansion()
         && let Some(suggestion) = simplify_not(cx, inner)
         && cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, expr.hir_id).0 != Level::Allow
     {
+        let need_parens = 'block: {
+            if matches!(inner.kind, ExprKind::Binary(..)) {
+                if let Some(parent_expr) = parent_expr {
+                    if matches!(parent_expr.kind, ExprKind::Cast(..) | ExprKind::Binary(..)) {
+                        break 'block true;
+                    }
+                }
+            }
+            false
+        };
+        let suggestion = if need_parens {
+            format!("({suggestion})")
+        } else {
+            suggestion
+        };
         span_lint_and_sugg(
             cx,
             NONMINIMAL_BOOL,
@@ -196,6 +215,7 @@ fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
 }
 
 struct NonminimalBoolVisitor<'a, 'tcx> {
+    parent_expr: Vec<&'tcx Expr<'tcx>>, // parent stack
     cx: &'a LateContext<'tcx>,
 }
 
@@ -569,7 +589,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
                 }
             };
             if improvements.is_empty() {
-                check_simplify_not(self.cx, e);
+                check_simplify_not(self.cx, e, self.parent_expr.last().copied());
             } else {
                 nonminimal_bool_lint(
                     improvements
@@ -602,7 +622,9 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
                 _ => {},
             }
         }
+        self.parent_expr.push(e);
         walk_expr(self, e);
+        self.parent_expr.pop();
     }
 }
 

tests/ui/nonminimal_bool_methods.fixed

@@ -115,4 +115,16 @@ fn issue_12625() {
     if a as u64 > b {} //~ ERROR: this boolean expression can be simplified
 }
 
+fn issue_12761() {
+    let a = 0;
+    let b = 0;
+    let c = 0;
+    if (a < b) as i32 == c {} //~ ERROR: this boolean expression can be simplified
+    if (a < b) | (a > c) {} //~ ERROR: this boolean expression can be simplified
+    let opt: Option<usize> = Some(1);
+    let res: Result<usize, usize> = Ok(1);
+    if res.is_err() as i32 == c {} //~ ERROR: this boolean expression can be simplified
+    if res.is_err() | opt.is_some() {} //~ ERROR: this boolean expression can be simplified
+}
+
 fn main() {}

tests/ui/nonminimal_bool_methods.rs

@@ -115,4 +115,16 @@ fn issue_12625() {
     if !(a as u64 <= b) {} //~ ERROR: this boolean expression can be simplified
 }
 
+fn issue_12761() {
+    let a = 0;
+    let b = 0;
+    let c = 0;
+    if !(a >= b) as i32 == c {} //~ ERROR: this boolean expression can be simplified
+    if !(a >= b) | !(a <= c) {} //~ ERROR: this boolean expression can be simplified
+    let opt: Option<usize> = Some(1);
+    let res: Result<usize, usize> = Ok(1);
+    if !res.is_ok() as i32 == c {} //~ ERROR: this boolean expression can be simplified
+    if !res.is_ok() | !opt.is_none() {} //~ ERROR: this boolean expression can be simplified
+}
+
 fn main() {}

tests/ui/nonminimal_bool_methods.stderr

@@ -97,5 +97,41 @@ error: this boolean expression can be simplified
 LL |     if !(a as u64 <= b) {}
    |        ^^^^^^^^^^^^^^^^ help: try: `a as u64 > b`
 
-error: aborting due to 16 previous errors
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:122:8
+   |
+LL |     if !(a >= b) as i32 == c {}
+   |        ^^^^^^^^^ help: try: `(a < b)`
+
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:123:8
+   |
+LL |     if !(a >= b) | !(a <= c) {}
+   |        ^^^^^^^^^ help: try: `(a < b)`
+
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:123:20
+   |
+LL |     if !(a >= b) | !(a <= c) {}
+   |                    ^^^^^^^^^ help: try: `(a > c)`
+
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:126:8
+   |
+LL |     if !res.is_ok() as i32 == c {}
+   |        ^^^^^^^^^^^^ help: try: `res.is_err()`
+
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:127:8
+   |
+LL |     if !res.is_ok() | !opt.is_none() {}
+   |        ^^^^^^^^^^^^ help: try: `res.is_err()`
+
+error: this boolean expression can be simplified
+  --> tests/ui/nonminimal_bool_methods.rs:127:23
+   |
+LL |     if !res.is_ok() | !opt.is_none() {}
+   |                       ^^^^^^^^^^^^^^ help: try: `opt.is_some()`
+
+error: aborting due to 22 previous errors