Skip to content

Commit dc97526

Browse files
committed
Auto merge of rust-lang#11991 - J-ZhengLi:issue9911, r=llogiq
stop [`bool_comparison`]'s suggestion from consuming parentheses fixes: rust-lang#9907 --- changelog: stop [`bool_comparison`]'s suggestion from consuming parentheses
2 parents 7e650b7 + a556d00 commit dc97526

File tree

4 files changed

+64
-12
lines changed

4 files changed

+64
-12
lines changed

clippy_lints/src/needless_bool.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -340,20 +340,36 @@ fn check_comparison<'a, 'tcx>(
340340
}
341341
if l_ty.is_bool() && r_ty.is_bool() {
342342
let mut applicability = Applicability::MachineApplicable;
343+
// Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs,
344+
// calling `source_callsite` make sure macros are handled correctly, see issue #9907
345+
let binop_span = left_side
346+
.span
347+
.source_callsite()
348+
.with_hi(right_side.span.source_callsite().hi());
343349

344350
if op.node == BinOpKind::Eq {
345351
let expression_info = one_side_is_unary_not(left_side, right_side);
346352
if expression_info.one_side_is_unary_not {
347353
span_lint_and_sugg(
348354
cx,
349355
BOOL_COMPARISON,
350-
e.span,
356+
binop_span,
351357
"this comparison might be written more concisely",
352358
"try simplifying it as shown",
353359
format!(
354360
"{} != {}",
355-
snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability),
356-
snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability)
361+
snippet_with_applicability(
362+
cx,
363+
expression_info.left_span.source_callsite(),
364+
"..",
365+
&mut applicability
366+
),
367+
snippet_with_applicability(
368+
cx,
369+
expression_info.right_span.source_callsite(),
370+
"..",
371+
&mut applicability
372+
)
357373
),
358374
applicability,
359375
);
@@ -362,24 +378,24 @@ fn check_comparison<'a, 'tcx>(
362378

363379
match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
364380
(Some(true), None) => left_true.map_or((), |(h, m)| {
365-
suggest_bool_comparison(cx, e, right_side, applicability, m, h);
381+
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
366382
}),
367383
(None, Some(true)) => right_true.map_or((), |(h, m)| {
368-
suggest_bool_comparison(cx, e, left_side, applicability, m, h);
384+
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
369385
}),
370386
(Some(false), None) => left_false.map_or((), |(h, m)| {
371-
suggest_bool_comparison(cx, e, right_side, applicability, m, h);
387+
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
372388
}),
373389
(None, Some(false)) => right_false.map_or((), |(h, m)| {
374-
suggest_bool_comparison(cx, e, left_side, applicability, m, h);
390+
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
375391
}),
376392
(None, None) => no_literal.map_or((), |(h, m)| {
377393
let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability);
378394
let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability);
379395
span_lint_and_sugg(
380396
cx,
381397
BOOL_COMPARISON,
382-
e.span,
398+
binop_span,
383399
m,
384400
"try simplifying it as shown",
385401
h(left_side, right_side).to_string(),
@@ -394,17 +410,17 @@ fn check_comparison<'a, 'tcx>(
394410

395411
fn suggest_bool_comparison<'a, 'tcx>(
396412
cx: &LateContext<'tcx>,
397-
e: &'tcx Expr<'_>,
413+
span: Span,
398414
expr: &Expr<'_>,
399415
mut app: Applicability,
400416
message: &str,
401417
conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
402418
) {
403-
let hint = Sugg::hir_with_context(cx, expr, e.span.ctxt(), "..", &mut app);
419+
let hint = Sugg::hir_with_context(cx, expr, span.ctxt(), "..", &mut app);
404420
span_lint_and_sugg(
405421
cx,
406422
BOOL_COMPARISON,
407-
e.span,
423+
span,
408424
message,
409425
"try simplifying it as shown",
410426
conv_hint(hint).to_string(),

tests/ui/bool_comparison.fixed

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,12 @@ fn issue3973() {
165165
if is_debug == m!(func) {}
166166
if m!(func) == is_debug {}
167167
}
168+
169+
#[allow(clippy::unnecessary_cast)]
170+
fn issue9907() {
171+
let _ = (1 >= 2) as usize;
172+
let _ = (!m!(func)) as usize;
173+
// This is not part of the issue, but an unexpected found when fixing the issue,
174+
// the provided span was inside of macro rather than the macro callsite.
175+
let _ = ((1 < 2) != m!(func)) as usize;
176+
}

tests/ui/bool_comparison.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,12 @@ fn issue3973() {
165165
if is_debug == m!(func) {}
166166
if m!(func) == is_debug {}
167167
}
168+
169+
#[allow(clippy::unnecessary_cast)]
170+
fn issue9907() {
171+
let _ = ((1 < 2) == false) as usize;
172+
let _ = (false == m!(func)) as usize;
173+
// This is not part of the issue, but an unexpected found when fixing the issue,
174+
// the provided span was inside of macro rather than the macro callsite.
175+
let _ = ((1 < 2) == !m!(func)) as usize;
176+
}

tests/ui/bool_comparison.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,5 +133,23 @@ error: equality checks against true are unnecessary
133133
LL | if m!(func) == true {}
134134
| ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)`
135135

136-
error: aborting due to 22 previous errors
136+
error: equality checks against false can be replaced by a negation
137+
--> $DIR/bool_comparison.rs:171:14
138+
|
139+
LL | let _ = ((1 < 2) == false) as usize;
140+
| ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `1 >= 2`
141+
142+
error: equality checks against false can be replaced by a negation
143+
--> $DIR/bool_comparison.rs:172:14
144+
|
145+
LL | let _ = (false == m!(func)) as usize;
146+
| ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)`
147+
148+
error: this comparison might be written more concisely
149+
--> $DIR/bool_comparison.rs:175:14
150+
|
151+
LL | let _ = ((1 < 2) == !m!(func)) as usize;
152+
| ^^^^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `(1 < 2) != m!(func)`
153+
154+
error: aborting due to 25 previous errors
137155

0 commit comments

Comments
 (0)