diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 2d1c250ac..0d66f2d64 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -85,7 +85,117 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool { ) { NonminimalBoolVisitor { cx }.visit_body(body); } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + match expr.kind { + ExprKind::Unary(UnOp::Not, sub) => check_inverted_condition(cx, expr.span, sub), + // This check the case where an element in a boolean comparison is inverted, like: + // + // ``` + // let a = true; + // !a == false; + // ``` + ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) => { + check_inverted_bool_in_condition(cx, expr.span, op.node, left, right); + }, + _ => {}, + } + } } + +fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> { + match op { + BinOpKind::Eq => Some("!="), + BinOpKind::Ne => Some("=="), + _ => None, + } +} + +fn bin_op_eq_str(op: BinOpKind) -> Option<&'static str> { + match op { + BinOpKind::Eq => Some("=="), + BinOpKind::Ne => Some("!="), + _ => None, + } +} + +fn check_inverted_condition(cx: &LateContext<'_>, expr_span: Span, sub_expr: &Expr<'_>) { + if !expr_span.from_expansion() + && let ExprKind::Binary(op, left, right) = sub_expr.kind + && let Some(left) = snippet_opt(cx, left.span) + && let Some(right) = snippet_opt(cx, right.span) + { + let Some(op) = inverted_bin_op_eq_str(op.node) else { + return; + }; + span_lint_and_sugg( + cx, + NONMINIMAL_BOOL, + expr_span, + "this boolean expression can be simplified", + "try", + format!("{left} {op} {right}",), + Applicability::MachineApplicable, + ); + } +} + +fn check_inverted_bool_in_condition( + cx: &LateContext<'_>, + expr_span: Span, + op: BinOpKind, + left: &Expr<'_>, + right: &Expr<'_>, +) { + if expr_span.from_expansion() + && (!cx.typeck_results().node_types()[left.hir_id].is_bool() + || !cx.typeck_results().node_types()[right.hir_id].is_bool()) + { + return; + } + + let suggestion = match (left.kind, right.kind) { + (ExprKind::Unary(UnOp::Not, left_sub), ExprKind::Unary(UnOp::Not, right_sub)) => { + let Some(left) = snippet_opt(cx, left_sub.span) else { + return; + }; + let Some(right) = snippet_opt(cx, right_sub.span) else { + return; + }; + let Some(op) = bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + (ExprKind::Unary(UnOp::Not, left_sub), _) => { + let Some(left) = snippet_opt(cx, left_sub.span) else { + return; + }; + let Some(right) = snippet_opt(cx, right.span) else { + return; + }; + let Some(op) = inverted_bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + (_, ExprKind::Unary(UnOp::Not, right_sub)) => { + let Some(left) = snippet_opt(cx, left.span) else { return }; + let Some(right) = snippet_opt(cx, right_sub.span) else { + return; + }; + let Some(op) = inverted_bin_op_eq_str(op) else { return }; + format!("{left} {op} {right}") + }, + _ => return, + }; + span_lint_and_sugg( + cx, + NONMINIMAL_BOOL, + expr_span, + "this boolean expression can be simplified", + "try", + suggestion, + Applicability::MachineApplicable, + ); +} + struct NonminimalBoolVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, } diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed index 02f1d09b8..54bdf7f5d 100644 --- a/tests/ui/bool_comparison.fixed +++ b/tests/ui/bool_comparison.fixed @@ -1,6 +1,6 @@ #![allow(clippy::needless_if)] #![warn(clippy::bool_comparison)] -#![allow(clippy::non_canonical_partial_ord_impl)] +#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)] fn main() { let x = true; diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index 5ef696d85..4fdf23052 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -1,6 +1,6 @@ #![allow(clippy::needless_if)] #![warn(clippy::bool_comparison)] -#![allow(clippy::non_canonical_partial_ord_impl)] +#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)] fn main() { let x = true; diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs index f7c3df706..908ef6cb2 100644 --- a/tests/ui/nonminimal_bool.rs +++ b/tests/ui/nonminimal_bool.rs @@ -156,3 +156,20 @@ fn issue11932() { x % 3 == 0 }; } + +fn issue_5794() { + let a = 0; + if !(12 == a) {} //~ ERROR: this boolean expression can be simplified + if !(a == 12) {} //~ ERROR: this boolean expression can be simplified + if !(12 != a) {} //~ ERROR: this boolean expression can be simplified + if !(a != 12) {} //~ ERROR: this boolean expression can be simplified + + let b = true; + let c = false; + if !b == true {} //~ ERROR: this boolean expression can be simplified + if !b != true {} //~ ERROR: this boolean expression can be simplified + if true == !b {} //~ ERROR: this boolean expression can be simplified + if true != !b {} //~ ERROR: this boolean expression can be simplified + if !b == !c {} //~ ERROR: this boolean expression can be simplified + if !b != !c {} //~ ERROR: this boolean expression can be simplified +} diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr index fd1568d94..a317c8328 100644 --- a/tests/ui/nonminimal_bool.stderr +++ b/tests/ui/nonminimal_bool.stderr @@ -114,5 +114,104 @@ error: this boolean expression can be simplified LL | if matches!(true, true) && true { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(true, true)` -error: aborting due to 13 previous errors +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:162:8 + | +LL | if !(12 == a) {} + | ^^^^^^^^^^ help: try: `12 != a` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:163:8 + | +LL | if !(a == 12) {} + | ^^^^^^^^^^ help: try: `a != 12` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:164:8 + | +LL | if !(12 != a) {} + | ^^^^^^^^^^ help: try: `12 == a` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:165:8 + | +LL | if !(a != 12) {} + | ^^^^^^^^^^ help: try: `a == 12` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:169:8 + | +LL | if !b == true {} + | ^^^^^^^^^^ help: try: `b != true` + +error: this comparison might be written more concisely + --> $DIR/nonminimal_bool.rs:169:8 + | +LL | if !b == true {} + | ^^^^^^^^^^ help: try simplifying it as shown: `b != true` + | + = note: `-D clippy::bool-comparison` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::bool_comparison)]` + +error: equality checks against true are unnecessary + --> $DIR/nonminimal_bool.rs:169:8 + | +LL | if !b == true {} + | ^^^^^^^^^^ help: try simplifying it as shown: `!b` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:170:8 + | +LL | if !b != true {} + | ^^^^^^^^^^ help: try: `b == true` + +error: inequality checks against true can be replaced by a negation + --> $DIR/nonminimal_bool.rs:170:8 + | +LL | if !b != true {} + | ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:171:8 + | +LL | if true == !b {} + | ^^^^^^^^^^ help: try: `true != b` + +error: this comparison might be written more concisely + --> $DIR/nonminimal_bool.rs:171:8 + | +LL | if true == !b {} + | ^^^^^^^^^^ help: try simplifying it as shown: `true != b` + +error: equality checks against true are unnecessary + --> $DIR/nonminimal_bool.rs:171:8 + | +LL | if true == !b {} + | ^^^^^^^^^^ help: try simplifying it as shown: `!b` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:172:8 + | +LL | if true != !b {} + | ^^^^^^^^^^ help: try: `true == b` + +error: inequality checks against true can be replaced by a negation + --> $DIR/nonminimal_bool.rs:172:8 + | +LL | if true != !b {} + | ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:173:8 + | +LL | if !b == !c {} + | ^^^^^^^^ help: try: `b == c` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:174:8 + | +LL | if !b != !c {} + | ^^^^^^^^ help: try: `b != c` + +error: aborting due to 29 previous errors