Skip to content

Commit 75f57cf

Browse files
borsbors
committed
Auto merge of rust-lang#12248 - GuillaumeGomez:extend-NONMINIMAL_BOOL, r=blyxyas
Extend `NONMINIMAL_BOOL` lint Fixes rust-lang#5794. r? `@blyxyas` changelog: Extend `NONMINIMAL_BOOL` lint
2 parents fff46c1 + 5e7c437 commit 75f57cf

File tree

5 files changed

+229
-3
lines changed

5 files changed

+229
-3
lines changed

‎clippy_lints/src/booleans.rs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,117 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
8585
) {
8686
NonminimalBoolVisitor { cx }.visit_body(body);
8787
}
88+
89+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
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;
171+
};
172+
let Some(right) = snippet_opt(cx, right.span) else {
173+
return;
174+
};
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+
);
88197
}
198+
89199
struct NonminimalBoolVisitor<'a, 'tcx> {
90200
cx: &'a LateContext<'tcx>,
91201
}

‎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: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,20 @@ fn issue11932() {
156156
x % 3 == 0
157157
};
158158
}
159+
160+
fn issue_5794() {
161+
let a = 0;
162+
if !(12 == a) {} //~ ERROR: this boolean expression can be simplified
163+
if !(a == 12) {} //~ ERROR: this boolean expression can be simplified
164+
if !(12 != a) {} //~ ERROR: this boolean expression can be simplified
165+
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
175+
}

‎tests/ui/nonminimal_bool.stderr

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,104 @@ error: this boolean expression can be simplified
114114
LL | if matches!(true, true) && true {
115115
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(true, true)`
116116

117-
error: aborting due to 13 previous errors
117+
error: this boolean expression can be simplified
118+
--> $DIR/nonminimal_bool.rs:162:8
119+
|
120+
LL | if !(12 == a) {}
121+
| ^^^^^^^^^^ help: try: `12 != a`
122+
123+
error: this boolean expression can be simplified
124+
--> $DIR/nonminimal_bool.rs:163:8
125+
|
126+
LL | if !(a == 12) {}
127+
| ^^^^^^^^^^ help: try: `a != 12`
128+
129+
error: this boolean expression can be simplified
130+
--> $DIR/nonminimal_bool.rs:164:8
131+
|
132+
LL | if !(12 != a) {}
133+
| ^^^^^^^^^^ help: try: `12 == a`
134+
135+
error: this boolean expression can be simplified
136+
--> $DIR/nonminimal_bool.rs:165:8
137+
|
138+
LL | if !(a != 12) {}
139+
| ^^^^^^^^^^ help: try: `a == 12`
140+
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
118217

0 commit comments

Comments
 (0)