Skip to content

Commit addeb20

Browse files
ada4aada4a
committed
fix: don't check for !x = y/x = !y
already done by `nonminimal_bool`
1 parent 95bb0d8 commit addeb20

File tree

6 files changed

+72
-171
lines changed

6 files changed

+72
-171
lines changed

‎clippy_lints/src/needless_bool.rs

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clippy_utils::{
77
};
88
use rustc_ast::ast::LitKind;
99
use rustc_errors::Applicability;
10-
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
10+
use rustc_hir::{BinOpKind, Expr, ExprKind};
1111
use rustc_lint::{LateContext, LateLintPass};
1212
use rustc_session::declare_lint_pass;
1313
use rustc_span::Span;
@@ -293,30 +293,6 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison {
293293
}
294294
}
295295

296-
struct ExpressionInfoWithSpan {
297-
one_side_is_unary_not: bool,
298-
left_span: Span,
299-
right_span: Span,
300-
}
301-
302-
fn is_unary_not(e: &Expr<'_>) -> (bool, Span) {
303-
if let ExprKind::Unary(UnOp::Not, operand) = e.kind {
304-
return (true, operand.span);
305-
}
306-
(false, e.span)
307-
}
308-
309-
fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> ExpressionInfoWithSpan {
310-
let left = is_unary_not(left_side);
311-
let right = is_unary_not(right_side);
312-
313-
ExpressionInfoWithSpan {
314-
one_side_is_unary_not: left.0 != right.0,
315-
left_span: left.1,
316-
right_span: right.1,
317-
}
318-
}
319-
320296
fn check_comparison<'a, 'tcx>(
321297
cx: &LateContext<'tcx>,
322298
e: &'tcx Expr<'_>,
@@ -326,41 +302,12 @@ fn check_comparison<'a, 'tcx>(
326302
right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
327303
no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &'static str)>,
328304
) {
329-
if let ExprKind::Binary(op, left_side, right_side) = e.kind {
305+
if let ExprKind::Binary(_, left_side, right_side) = e.kind {
330306
let mut applicability = Applicability::MachineApplicable;
331307
// Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs,
332308
// calling `source_callsite` make sure macros are handled correctly, see issue #9907
333309
let binop_span = left_side.span.source_callsite().to(right_side.span.source_callsite());
334310

335-
if op.node == BinOpKind::Eq {
336-
let expression_info = one_side_is_unary_not(left_side, right_side);
337-
if expression_info.one_side_is_unary_not {
338-
span_lint_and_sugg(
339-
cx,
340-
BOOL_COMPARISON,
341-
binop_span,
342-
"this comparison might be written more concisely",
343-
"try simplifying it",
344-
format!(
345-
"{} != {}",
346-
snippet_with_applicability(
347-
cx,
348-
expression_info.left_span.source_callsite(),
349-
"..",
350-
&mut applicability
351-
),
352-
snippet_with_applicability(
353-
cx,
354-
expression_info.right_span.source_callsite(),
355-
"..",
356-
&mut applicability
357-
)
358-
),
359-
applicability,
360-
);
361-
}
362-
}
363-
364311
match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
365312
(Some(true), None) => left_true.map_or((), |(h, m)| {
366313
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);

‎tests/ui/bool_comparison.fixed

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

55
fn main() {
66
let x = true;
@@ -72,26 +72,6 @@ fn issue3703() {
7272
if false < Foo {}
7373
}
7474

75-
#[allow(dead_code)]
76-
fn issue4983() {
77-
let a = true;
78-
let b = false;
79-
80-
if a != b {};
81-
//~^ bool_comparison
82-
if a != b {};
83-
//~^ bool_comparison
84-
if a == b {};
85-
if !a == !b {};
86-
87-
if b != a {};
88-
//~^ bool_comparison
89-
if b != a {};
90-
//~^ bool_comparison
91-
if b == a {};
92-
if !b == !a {};
93-
}
94-
9575
macro_rules! m {
9676
($func:ident) => {
9777
$func()
@@ -139,8 +119,27 @@ fn issue9907() {
139119
//~^ bool_comparison
140120
let _ = (!m!(func)) as usize;
141121
//~^ bool_comparison
142-
// This is not part of the issue, but an unexpected found when fixing the issue,
143-
// the provided span was inside of macro rather than the macro callsite.
144-
let _ = ((1 < 2) != m!(func)) as usize;
145-
//~^ bool_comparison
122+
123+
// NOTE: Here's what used to be here:
124+
//
125+
// > This is not part of the issue, but an unexpected found when fixing the issue,
126+
// > the provided span was inside of macro rather than the macro callsite.
127+
// > let _ = ((1 < 2) == !m!(func)) as usize;
128+
// > <error pattern for `bool_comparison`> (can't put it into a comment or ui_test complains)
129+
//
130+
// But we don't lint this case since #15367, and similar cases of `<` of `>` apparently still
131+
// have this bug (see #15497), so I can't just modify the example. Consider doing that when the
132+
// issue is fixed
133+
}
134+
135+
#[allow(dead_code, clippy::nonminimal_bool)]
136+
fn issue15367() {
137+
let a = true;
138+
let b = false;
139+
140+
// these cases are handled by `nonminimal_bool`, so don't double-lint
141+
if a == !b {};
142+
if !a == b {};
143+
if b == !a {};
144+
if !b == a {};
146145
}

‎tests/ui/bool_comparison.rs

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

55
fn main() {
66
let x = true;
@@ -72,26 +72,6 @@ fn issue3703() {
7272
if false < Foo {}
7373
}
7474

75-
#[allow(dead_code)]
76-
fn issue4983() {
77-
let a = true;
78-
let b = false;
79-
80-
if a == !b {};
81-
//~^ bool_comparison
82-
if !a == b {};
83-
//~^ bool_comparison
84-
if a == b {};
85-
if !a == !b {};
86-
87-
if b == !a {};
88-
//~^ bool_comparison
89-
if !b == a {};
90-
//~^ bool_comparison
91-
if b == a {};
92-
if !b == !a {};
93-
}
94-
9575
macro_rules! m {
9676
($func:ident) => {
9777
$func()
@@ -139,8 +119,27 @@ fn issue9907() {
139119
//~^ bool_comparison
140120
let _ = (false == m!(func)) as usize;
141121
//~^ bool_comparison
142-
// This is not part of the issue, but an unexpected found when fixing the issue,
143-
// the provided span was inside of macro rather than the macro callsite.
144-
let _ = ((1 < 2) == !m!(func)) as usize;
145-
//~^ bool_comparison
122+
123+
// NOTE: Here's what used to be here:
124+
//
125+
// > This is not part of the issue, but an unexpected found when fixing the issue,
126+
// > the provided span was inside of macro rather than the macro callsite.
127+
// > let _ = ((1 < 2) == !m!(func)) as usize;
128+
// > <error pattern for `bool_comparison`> (can't put it into a comment or ui_test complains)
129+
//
130+
// But we don't lint this case since #15367, and similar cases of `<` of `>` apparently still
131+
// have this bug (see #15497), so I can't just modify the example. Consider doing that when the
132+
// issue is fixed
133+
}
134+
135+
#[allow(dead_code, clippy::nonminimal_bool)]
136+
fn issue15367() {
137+
let a = true;
138+
let b = false;
139+
140+
// these cases are handled by `nonminimal_bool`, so don't double-lint
141+
if a == !b {};
142+
if !a == b {};
143+
if b == !a {};
144+
if !b == a {};
146145
}

‎tests/ui/bool_comparison.stderr

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -85,71 +85,41 @@ error: order comparisons between booleans can be simplified
8585
LL | let _ = if x > y { "yes" } else { "no" };
8686
| ^^^^^ help: try simplifying it: `x & !y`
8787

88-
error: this comparison might be written more concisely
89-
--> tests/ui/bool_comparison.rs:80:8
90-
|
91-
LL | if a == !b {};
92-
| ^^^^^^^ help: try simplifying it: `a != b`
93-
94-
error: this comparison might be written more concisely
95-
--> tests/ui/bool_comparison.rs:82:8
96-
|
97-
LL | if !a == b {};
98-
| ^^^^^^^ help: try simplifying it: `a != b`
99-
100-
error: this comparison might be written more concisely
101-
--> tests/ui/bool_comparison.rs:87:8
102-
|
103-
LL | if b == !a {};
104-
| ^^^^^^^ help: try simplifying it: `b != a`
105-
106-
error: this comparison might be written more concisely
107-
--> tests/ui/bool_comparison.rs:89:8
108-
|
109-
LL | if !b == a {};
110-
| ^^^^^^^ help: try simplifying it: `b != a`
111-
11288
error: equality checks against false can be replaced by a negation
113-
--> tests/ui/bool_comparison.rs:114:8
89+
--> tests/ui/bool_comparison.rs:94:8
11490
|
11591
LL | if false == m!(func) {}
11692
| ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
11793

11894
error: equality checks against false can be replaced by a negation
119-
--> tests/ui/bool_comparison.rs:116:8
95+
--> tests/ui/bool_comparison.rs:96:8
12096
|
12197
LL | if m!(func) == false {}
12298
| ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
12399

124100
error: equality checks against true are unnecessary
125-
--> tests/ui/bool_comparison.rs:118:8
101+
--> tests/ui/bool_comparison.rs:98:8
126102
|
127103
LL | if true == m!(func) {}
128104
| ^^^^^^^^^^^^^^^^ help: try simplifying it: `m!(func)`
129105

130106
error: equality checks against true are unnecessary
131-
--> tests/ui/bool_comparison.rs:120:8
107+
--> tests/ui/bool_comparison.rs:100:8
132108
|
133109
LL | if m!(func) == true {}
134110
| ^^^^^^^^^^^^^^^^ help: try simplifying it: `m!(func)`
135111

136112
error: equality checks against false can be replaced by a negation
137-
--> tests/ui/bool_comparison.rs:138:14
113+
--> tests/ui/bool_comparison.rs:118:14
138114
|
139115
LL | let _ = ((1 < 2) == false) as usize;
140116
| ^^^^^^^^^^^^^^^^ help: try simplifying it: `1 >= 2`
141117

142118
error: equality checks against false can be replaced by a negation
143-
--> tests/ui/bool_comparison.rs:140:14
119+
--> tests/ui/bool_comparison.rs:120:14
144120
|
145121
LL | let _ = (false == m!(func)) as usize;
146122
| ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
147123

148-
error: this comparison might be written more concisely
149-
--> tests/ui/bool_comparison.rs:144:14
150-
|
151-
LL | let _ = ((1 < 2) == !m!(func)) as usize;
152-
| ^^^^^^^^^^^^^^^^^^^^ help: try simplifying it: `(1 < 2) != m!(func)`
153-
154-
error: aborting due to 25 previous errors
124+
error: aborting due to 20 previous errors
155125

‎tests/ui/nonminimal_bool.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,12 @@ fn issue_5794() {
182182
if !b == true {}
183183
//~^ nonminimal_bool
184184
//~| bool_comparison
185-
//~| bool_comparison
186185
if !b != true {}
187186
//~^ nonminimal_bool
188187
//~| bool_comparison
189188
if true == !b {}
190189
//~^ nonminimal_bool
191190
//~| bool_comparison
192-
//~| bool_comparison
193191
if true != !b {}
194192
//~^ nonminimal_bool
195193
//~| bool_comparison

0 commit comments

Comments
 (0)