From c5d4b04dd51bc470469c72ee22816a2c634b8a3d Mon Sep 17 00:00:00 2001
From: "Samuel \"Sam\" Tardieu" <sam@rfc1149.net>
Date: Wed, 10 May 2023 21:06:36 +0200
Subject: [PATCH] needless_bool: do not simplify code if it loses comments
---
clippy_lints/src/needless_bool.rs | 5 ++---
tests/ui/needless_bool/fixable.fixed | 7 +++++++
tests/ui/needless_bool/fixable.rs | 7 +++++++
tests/ui/needless_bool/fixable.stderr | 24 ++++++++++++------------
4 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index 71281a0b40b..62af42a3961 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -146,7 +146,7 @@ fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool {
impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
use self::Expression::{Bool, RetBool};
- if e.span.from_expansion() {
+ if e.span.from_expansion() || !span_extract_comment(cx.tcx.sess.source_map(), e.span).is_empty() {
return;
}
if let Some(higher::If {
@@ -209,8 +209,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
}
if let Some((lhs_a, a)) = fetch_assign(then) &&
let Some((lhs_b, b)) = fetch_assign(r#else) &&
- SpanlessEq::new(cx).eq_expr(lhs_a, lhs_b) &&
- span_extract_comment(cx.tcx.sess.source_map(), e.span).is_empty()
+ SpanlessEq::new(cx).eq_expr(lhs_a, lhs_b)
{
let mut applicability = Applicability::MachineApplicable;
let cond = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability);
diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed
index f860852e7b7..bf1911881c8 100644
--- a/tests/ui/needless_bool/fixable.fixed
+++ b/tests/ui/needless_bool/fixable.fixed
@@ -63,6 +63,13 @@ fn main() {
needless_bool2(x);
needless_bool3(x);
needless_bool_condition();
+
+ if a == b {
+ true
+ } else {
+ // Do not lint as this comment might be important
+ false
+ };
}
fn bool_ret3(x: bool) -> bool {
diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs
index 6680dab5b6d..a6c465d4fbd 100644
--- a/tests/ui/needless_bool/fixable.rs
+++ b/tests/ui/needless_bool/fixable.rs
@@ -99,6 +99,13 @@ fn main() {
needless_bool2(x);
needless_bool3(x);
needless_bool_condition();
+
+ if a == b {
+ true
+ } else {
+ // Do not lint as this comment might be important
+ false
+ };
}
fn bool_ret3(x: bool) -> bool {
diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr
index d2c48376f76..fa906374fb3 100644
--- a/tests/ui/needless_bool/fixable.stderr
+++ b/tests/ui/needless_bool/fixable.stderr
@@ -91,7 +91,7 @@ LL | | };
| |_____^ help: you can reduce it to: `a < b`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:105:5
+ --> $DIR/fixable.rs:112:5
|
LL | / if x {
LL | | return true;
@@ -101,7 +101,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:113:5
+ --> $DIR/fixable.rs:120:5
|
LL | / if x {
LL | | return false;
@@ -111,7 +111,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return !x`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:121:5
+ --> $DIR/fixable.rs:128:5
|
LL | / if x && y {
LL | | return true;
@@ -121,7 +121,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x && y`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:129:5
+ --> $DIR/fixable.rs:136:5
|
LL | / if x && y {
LL | | return false;
@@ -131,7 +131,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return !(x && y)`
error: equality checks against true are unnecessary
- --> $DIR/fixable.rs:137:8
+ --> $DIR/fixable.rs:144:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`
@@ -139,25 +139,25 @@ LL | if x == true {};
= note: `-D clippy::bool-comparison` implied by `-D warnings`
error: equality checks against false can be replaced by a negation
- --> $DIR/fixable.rs:141:8
+ --> $DIR/fixable.rs:148:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: equality checks against true are unnecessary
- --> $DIR/fixable.rs:151:8
+ --> $DIR/fixable.rs:158:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`
error: equality checks against false can be replaced by a negation
- --> $DIR/fixable.rs:152:8
+ --> $DIR/fixable.rs:159:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:161:12
+ --> $DIR/fixable.rs:168:12
|
LL | } else if returns_bool() {
| ____________^
@@ -168,7 +168,7 @@ LL | | };
| |_____^ help: you can reduce it to: `{ !returns_bool() }`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:174:5
+ --> $DIR/fixable.rs:181:5
|
LL | / if unsafe { no(4) } & 1 != 0 {
LL | | true
@@ -178,13 +178,13 @@ LL | | };
| |_____^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:179:30
+ --> $DIR/fixable.rs:186:30
|
LL | let _brackets_unneeded = if unsafe { no(4) } & 1 != 0 { true } else { false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `unsafe { no(4) } & 1 != 0`
error: this if-then-else expression returns a bool literal
- --> $DIR/fixable.rs:182:9
+ --> $DIR/fixable.rs:189:9
|
LL | if unsafe { no(4) } & 1 != 0 { true } else { false }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)`