Extend NONMINIMAL_BOOL to check inverted boolean values

This commit is contained in:
Guillaume Gomez 2024-02-09 20:47:31 +01:00
parent a18e0a11f7
commit 5e7c437d41
5 changed files with 193 additions and 25 deletions

View file

@ -87,31 +87,115 @@ 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,
};
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,
);
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>,
}

View file

@ -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;

View file

@ -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;

View file

@ -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
}

View file

@ -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