Skip to content

Commit

Permalink
Extend NONMINIMAL_BOOL to check inverted boolean values
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Feb 9, 2024
1 parent 6ee12f1 commit f38ead7
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 22 deletions.
118 changes: 98 additions & 20 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,31 +87,109 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Unary(UnOp::Not, sub) = expr.kind
&& !expr.span.from_expansion()
&& let ExprKind::Binary(op, left, right) = sub.kind
{
let new_op = match op.node {
BinOpKind::Eq => "!=",
BinOpKind::Ne => "==",
_ => return,
match expr.kind {
ExprKind::Unary(UnOp::Not, sub) => check_inverted_condition(cx, expr.span, sub),
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(left) = snippet_opt(cx, left.span) else { return };
let Some(right) = snippet_opt(cx, right.span) else {
return;
};
span_lint_and_sugg(
cx,
NONMINIMAL_BOOL,
expr.span,
"this boolean expression can be simplified",
"try",
format!("{left} {new_op} {right}"),
Applicability::MachineApplicable,
);
}
}
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>,
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/bool_comparison.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/nonminimal_bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,13 @@ fn issue_5794() {
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
}
77 changes: 76 additions & 1 deletion tests/ui/nonminimal_bool.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,80 @@ error: this boolean expression can be simplified
LL | if !(a != 12) {}
| ^^^^^^^^^^ help: try: `a == 12`

error: aborting due to 17 previous errors
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

0 comments on commit f38ead7

Please sign in to comment.