Skip to content

Commit 5e7c437

Browse files
GuillaumeGomezGuillaumeGomez
committed
Extend NONMINIMAL_BOOL to check inverted boolean values
1 parent a18e0a1 commit 5e7c437

File tree

5 files changed

+191
-23
lines changed

5 files changed

+191
-23
lines changed

‎clippy_lints/src/booleans.rs‎

Lines changed: 104 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -87,31 +87,115 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
8787
}
8888

8989
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
90-
if let ExprKind::Unary(UnOp::Not, sub) = expr.kind
91-
&& !expr.span.from_expansion()
92-
&& let ExprKind::Binary(op, left, right) = sub.kind
93-
{
94-
let new_op = match op.node {
95-
BinOpKind::Eq => "!=",
96-
BinOpKind::Ne => "==",
97-
_ => return,
90+
match expr.kind {
91+
ExprKind::Unary(UnOp::Not, sub) => check_inverted_condition(cx, expr.span, sub),
92+
// This check the case where an element in a boolean comparison is inverted, like:
93+
//
94+
// ```
95+
// let a = true;
96+
// !a == false;
97+
// ```
98+
ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) => {
99+
check_inverted_bool_in_condition(cx, expr.span, op.node, left, right);
100+
},
101+
_ => {},
102+
}
103+
}
104+
}
105+
106+
fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
107+
match op {
108+
BinOpKind::Eq => Some("!="),
109+
BinOpKind::Ne => Some("=="),
110+
_ => None,
111+
}
112+
}
113+
114+
fn bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
115+
match op {
116+
BinOpKind::Eq => Some("=="),
117+
BinOpKind::Ne => Some("!="),
118+
_ => None,
119+
}
120+
}
121+
122+
fn check_inverted_condition(cx: &LateContext<'_>, expr_span: Span, sub_expr: &Expr<'_>) {
123+
if !expr_span.from_expansion()
124+
&& let ExprKind::Binary(op, left, right) = sub_expr.kind
125+
&& let Some(left) = snippet_opt(cx, left.span)
126+
&& let Some(right) = snippet_opt(cx, right.span)
127+
{
128+
let Some(op) = inverted_bin_op_eq_str(op.node) else {
129+
return;
130+
};
131+
span_lint_and_sugg(
132+
cx,
133+
NONMINIMAL_BOOL,
134+
expr_span,
135+
"this boolean expression can be simplified",
136+
"try",
137+
format!("{left} {op} {right}",),
138+
Applicability::MachineApplicable,
139+
);
140+
}
141+
}
142+
143+
fn check_inverted_bool_in_condition(
144+
cx: &LateContext<'_>,
145+
expr_span: Span,
146+
op: BinOpKind,
147+
left: &Expr<'_>,
148+
right: &Expr<'_>,
149+
) {
150+
if expr_span.from_expansion()
151+
&& (!cx.typeck_results().node_types()[left.hir_id].is_bool()
152+
|| !cx.typeck_results().node_types()[right.hir_id].is_bool())
153+
{
154+
return;
155+
}
156+
157+
let suggestion = match (left.kind, right.kind) {
158+
(ExprKind::Unary(UnOp::Not, left_sub), ExprKind::Unary(UnOp::Not, right_sub)) => {
159+
let Some(left) = snippet_opt(cx, left_sub.span) else {
160+
return;
161+
};
162+
let Some(right) = snippet_opt(cx, right_sub.span) else {
163+
return;
164+
};
165+
let Some(op) = bin_op_eq_str(op) else { return };
166+
format!("{left} {op} {right}")
167+
},
168+
(ExprKind::Unary(UnOp::Not, left_sub), _) => {
169+
let Some(left) = snippet_opt(cx, left_sub.span) else {
170+
return;
98171
};
99-
let Some(left) = snippet_opt(cx, left.span) else { return };
100172
let Some(right) = snippet_opt(cx, right.span) else {
101173
return;
102174
};
103-
span_lint_and_sugg(
104-
cx,
105-
NONMINIMAL_BOOL,
106-
expr.span,
107-
"this boolean expression can be simplified",
108-
"try",
109-
format!("{left} {new_op} {right}"),
110-
Applicability::MachineApplicable,
111-
);
112-
}
113-
}
175+
let Some(op) = inverted_bin_op_eq_str(op) else { return };
176+
format!("{left} {op} {right}")
177+
},
178+
(_, ExprKind::Unary(UnOp::Not, right_sub)) => {
179+
let Some(left) = snippet_opt(cx, left.span) else { return };
180+
let Some(right) = snippet_opt(cx, right_sub.span) else {
181+
return;
182+
};
183+
let Some(op) = inverted_bin_op_eq_str(op) else { return };
184+
format!("{left} {op} {right}")
185+
},
186+
_ => return,
187+
};
188+
span_lint_and_sugg(
189+
cx,
190+
NONMINIMAL_BOOL,
191+
expr_span,
192+
"this boolean expression can be simplified",
193+
"try",
194+
suggestion,
195+
Applicability::MachineApplicable,
196+
);
114197
}
198+
115199
struct NonminimalBoolVisitor<'a, 'tcx> {
116200
cx: &'a LateContext<'tcx>,
117201
}

‎tests/ui/bool_comparison.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![allow(clippy::needless_if)]
22
#![warn(clippy::bool_comparison)]
3-
#![allow(clippy::non_canonical_partial_ord_impl)]
3+
#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
44

55
fn main() {
66
let x = true;

‎tests/ui/bool_comparison.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![allow(clippy::needless_if)]
22
#![warn(clippy::bool_comparison)]
3-
#![allow(clippy::non_canonical_partial_ord_impl)]
3+
#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
44

55
fn main() {
66
let x = true;

‎tests/ui/nonminimal_bool.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,13 @@ fn issue_5794() {
163163
if !(a == 12) {} //~ ERROR: this boolean expression can be simplified
164164
if !(12 != a) {} //~ ERROR: this boolean expression can be simplified
165165
if !(a != 12) {} //~ ERROR: this boolean expression can be simplified
166+
167+
let b = true;
168+
let c = false;
169+
if !b == true {} //~ ERROR: this boolean expression can be simplified
170+
if !b != true {} //~ ERROR: this boolean expression can be simplified
171+
if true == !b {} //~ ERROR: this boolean expression can be simplified
172+
if true != !b {} //~ ERROR: this boolean expression can be simplified
173+
if !b == !c {} //~ ERROR: this boolean expression can be simplified
174+
if !b != !c {} //~ ERROR: this boolean expression can be simplified
166175
}

‎tests/ui/nonminimal_bool.stderr‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,5 +138,80 @@ error: this boolean expression can be simplified
138138
LL | if !(a != 12) {}
139139
| ^^^^^^^^^^ help: try: `a == 12`
140140

141-
error: aborting due to 17 previous errors
141+
error: this boolean expression can be simplified
142+
--> $DIR/nonminimal_bool.rs:169:8
143+
|
144+
LL | if !b == true {}
145+
| ^^^^^^^^^^ help: try: `b != true`
146+
147+
error: this comparison might be written more concisely
148+
--> $DIR/nonminimal_bool.rs:169:8
149+
|
150+
LL | if !b == true {}
151+
| ^^^^^^^^^^ help: try simplifying it as shown: `b != true`
152+
|
153+
= note: `-D clippy::bool-comparison` implied by `-D warnings`
154+
= help: to override `-D warnings` add `#[allow(clippy::bool_comparison)]`
155+
156+
error: equality checks against true are unnecessary
157+
--> $DIR/nonminimal_bool.rs:169:8
158+
|
159+
LL | if !b == true {}
160+
| ^^^^^^^^^^ help: try simplifying it as shown: `!b`
161+
162+
error: this boolean expression can be simplified
163+
--> $DIR/nonminimal_bool.rs:170:8
164+
|
165+
LL | if !b != true {}
166+
| ^^^^^^^^^^ help: try: `b == true`
167+
168+
error: inequality checks against true can be replaced by a negation
169+
--> $DIR/nonminimal_bool.rs:170:8
170+
|
171+
LL | if !b != true {}
172+
| ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)`
173+
174+
error: this boolean expression can be simplified
175+
--> $DIR/nonminimal_bool.rs:171:8
176+
|
177+
LL | if true == !b {}
178+
| ^^^^^^^^^^ help: try: `true != b`
179+
180+
error: this comparison might be written more concisely
181+
--> $DIR/nonminimal_bool.rs:171:8
182+
|
183+
LL | if true == !b {}
184+
| ^^^^^^^^^^ help: try simplifying it as shown: `true != b`
185+
186+
error: equality checks against true are unnecessary
187+
--> $DIR/nonminimal_bool.rs:171:8
188+
|
189+
LL | if true == !b {}
190+
| ^^^^^^^^^^ help: try simplifying it as shown: `!b`
191+
192+
error: this boolean expression can be simplified
193+
--> $DIR/nonminimal_bool.rs:172:8
194+
|
195+
LL | if true != !b {}
196+
| ^^^^^^^^^^ help: try: `true == b`
197+
198+
error: inequality checks against true can be replaced by a negation
199+
--> $DIR/nonminimal_bool.rs:172:8
200+
|
201+
LL | if true != !b {}
202+
| ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)`
203+
204+
error: this boolean expression can be simplified
205+
--> $DIR/nonminimal_bool.rs:173:8
206+
|
207+
LL | if !b == !c {}
208+
| ^^^^^^^^ help: try: `b == c`
209+
210+
error: this boolean expression can be simplified
211+
--> $DIR/nonminimal_bool.rs:174:8
212+
|
213+
LL | if !b != !c {}
214+
| ^^^^^^^^ help: try: `b != c`
215+
216+
error: aborting due to 29 previous errors
142217

0 commit comments

Comments
 (0)