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

Author: CoCo-Japan-pan

clippy_lints/src/booleans.rs

@@ -1,6 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::eq_expr_value;
 use clippy_utils::source::SpanRangeExt;
+use clippy_utils::sugg::{make_binop, Sugg};
 use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -83,7 +84,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 +181,65 @@ 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: {
+            let Some(parent_expr) = parent_expr else {
+                break 'block false;
+            };
+            match parent_expr.kind {
+                ExprKind::Cast(_, ty) => {
+                    let new_expr = Expr {
+                        kind: ExprKind::Cast(inner, ty),
+                        ..*parent_expr
+                    };
+                    let Some(new_sugg) = Sugg::hir_opt(cx, &new_expr) else {
+                        break 'block false;
+                    };
+                    let Sugg::BinOp(_, lhs, _) = new_sugg else {
+                        break 'block false;
+                    };
+                    lhs.starts_with('(') && lhs.ends_with(')')
+                },
+                ExprKind::Binary(binop, lhs, rhs) if lhs.span == expr.span => {
+                    let Some(lhs_sugg) = Sugg::hir_opt(cx, inner) else {
+                        break 'block false;
+                    };
+                    let Some(rhs_sugg) = Sugg::hir_opt(cx, rhs) else {
+                        break 'block false;
+                    };
+                    let new_sugg = make_binop(binop.node, &lhs_sugg, &rhs_sugg);
+                    let Sugg::BinOp(_, lhs, _) = new_sugg else {
+                        break 'block false;
+                    };
+                    lhs.starts_with('(') && lhs.ends_with(')')
+                },
+                ExprKind::Binary(binop, lhs, rhs) if rhs.span == expr.span => {
+                    let Some(lhs_sugg) = Sugg::hir_opt(cx, lhs) else {
+                        break 'block false;
+                    };
+                    let Some(rhs_sugg) = Sugg::hir_opt(cx, inner) else {
+                        break 'block false;
+                    };
+                    let new_sugg = make_binop(binop.node, &lhs_sugg, &rhs_sugg);
+                    let Sugg::BinOp(_, _, rhs) = new_sugg else {
+                        break 'block false;
+                    };
+                    rhs.starts_with('(') && rhs.ends_with(')')
+                },
+                _ => false,
+            }
+        };
+        let suggestion = if need_parens {
+            format!("({suggestion})")
+        } else {
+            suggestion
+        };
         span_lint_and_sugg(
             cx,
             NONMINIMAL_BOOL,
@@ -196,6 +253,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 +627,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 +660,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