OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix #3335: bool_comparison triggers 3 times on same code

Browse Source
This commit is contained in:
Giorgio Gambino 2018-10-28 15:37:39 +01:00
parent 457e7f12e9
commit 7cfde9cfa9
3 changed files with 158 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
clippy_lints/src/needless_bool.rs
View File

@ -17,7 +17,7 @@ use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc::hir::*; use crate::rustc::hir::*;
use crate::syntax::ast::LitKind; use crate::syntax::ast::LitKind;
use crate::syntax::source_map::Spanned; use crate::syntax::source_map::Spanned;
use crate::utils::{snippet, span_lint, span_lint_and_sugg}; use crate::utils::{in_macro, snippet, span_lint, span_lint_and_sugg};
use crate::utils::sugg::Sugg; use crate::utils::sugg::Sugg;
/// **What it does:** Checks for expressions of the form `if c { true } else { /// **What it does:** Checks for expressions of the form `if c { true } else {
@ -133,54 +133,56 @@ impl LintPass for BoolComparison {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
use self::Expression::*; if !in_macro(e.span) {
if let ExprKind::Binary(Spanned { node: BinOpKind::Eq, .. }, ref left_side, ref right_side) = e.node { use self::Expression::*;
match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) { if let ExprKind::Binary(Spanned { node: BinOpKind::Eq, .. }, ref left_side, ref right_side) = e.node {
(Bool(true), Other) => { match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
let hint = snippet(cx, right_side.span, "..").into_owned(); (Bool(true), Other) => {
span_lint_and_sugg( let hint = snippet(cx, right_side.span, "..").into_owned();
cx, span_lint_and_sugg(
BOOL_COMPARISON, cx,
e.span, BOOL_COMPARISON,
"equality checks against true are unnecessary", e.span,
"try simplifying it as shown", "equality checks against true are unnecessary",
hint, "try simplifying it as shown",
); hint,
}, );
(Other, Bool(true)) => { },
let hint = snippet(cx, left_side.span, "..").into_owned(); (Other, Bool(true)) => {
span_lint_and_sugg( let hint = snippet(cx, left_side.span, "..").into_owned();
cx, span_lint_and_sugg(
BOOL_COMPARISON, cx,
e.span, BOOL_COMPARISON,
"equality checks against true are unnecessary", e.span,
"try simplifying it as shown", "equality checks against true are unnecessary",
hint, "try simplifying it as shown",
); hint,
}, );
(Bool(false), Other) => { },
let hint = Sugg::hir(cx, right_side, ".."); (Bool(false), Other) => {
span_lint_and_sugg( let hint = Sugg::hir(cx, right_side, "..");
cx, span_lint_and_sugg(
BOOL_COMPARISON, cx,
e.span, BOOL_COMPARISON,
"equality checks against false can be replaced by a negation", e.span,
"try simplifying it as shown", "equality checks against false can be replaced by a negation",
(!hint).to_string(), "try simplifying it as shown",
); (!hint).to_string(),
}, );
(Other, Bool(false)) => { },
let hint = Sugg::hir(cx, left_side, ".."); (Other, Bool(false)) => {
span_lint_and_sugg( let hint = Sugg::hir(cx, left_side, "..");
cx, span_lint_and_sugg(
BOOL_COMPARISON, cx,
e.span, BOOL_COMPARISON,
"equality checks against false can be replaced by a negation", e.span,
"try simplifying it as shown", "equality checks against false can be replaced by a negation",
(!hint).to_string(), "try simplifying it as shown",
); (!hint).to_string(),
}, );
_ => (), },
_ => (),
}
} }
} }
} }

49
tests/ui/needless_bool.rs
View File

@ -8,10 +8,32 @@
// except according to those terms. // except according to those terms.
#![warn(clippy::needless_bool)] #![warn(clippy::needless_bool)]
use std::cell::Cell;
macro_rules! bool_comparison_trigger {
($($i:ident: $def:expr, $stb:expr );+ $(;)*) => (
#[derive(Clone)]
pub struct Trigger {
$($i: (Cell<bool>, bool, bool)),+
}
#[allow(dead_code)]
impl Trigger {
pub fn trigger(&self, key: &str) -> bool {
$(
if let stringify!($i) = key {
return self.$i.1 && self.$i.2 == $def;
}
)+
false
}
}
)
}
#[allow(clippy::if_same_then_else)] #[allow(clippy::if_same_then_else)]
fn main() { fn main() {
let x = true; let x = true;
@ -28,6 +50,9 @@ fn main() {
bool_ret5(x, x); bool_ret5(x, x);
bool_ret4(x); bool_ret4(x);
bool_ret6(x, x); bool_ret6(x, x);
needless_bool(x);
needless_bool2(x);
needless_bool3(x);
} }
#[allow(clippy::if_same_then_else, clippy::needless_return)] #[allow(clippy::if_same_then_else, clippy::needless_return)]
@ -59,3 +84,23 @@ fn bool_ret4(x: bool) -> bool {
fn bool_ret6(x: bool, y: bool) -> bool { fn bool_ret6(x: bool, y: bool) -> bool {
if x && y { return false } else { return true }; if x && y { return false } else { return true };
} }
fn needless_bool(x: bool) {
if x == true { };
}
fn needless_bool2(x: bool) {
if x == false { };
}
fn needless_bool3(x: bool) {
bool_comparison_trigger! {
test_one: false, false;
test_three: false, false;
test_two: true, true;
}
if x == true { };
if x == false { };
}

94
tests/ui/needless_bool.stderr
View File

@ -1,70 +1,96 @@
error: this if-then-else expression will always return true error: this if-then-else expression will always return true
--> $DIR/needless_bool.rs:19:5 --> $DIR/needless_bool.rs:41:5
| |
19 | if x { true } else { true }; 41 | if x { true } else { true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
= note: `-D clippy::needless-bool` implied by `-D warnings` = note: `-D clippy::needless-bool` implied by `-D warnings`
error: this if-then-else expression will always return false error: this if-then-else expression will always return false
--> $DIR/needless_bool.rs:20:5 --> $DIR/needless_bool.rs:42:5
| |
20 | if x { false } else { false }; 42 | if x { false } else { false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:21:5 --> $DIR/needless_bool.rs:43:5
| |
21 | if x { true } else { false }; 43 | if x { true } else { false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `x` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `x`
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:22:5 --> $DIR/needless_bool.rs:44:5
| |
22 | if x { false } else { true }; 44 | if x { false } else { true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `!x` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `!x`
error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:23:5
|
23 | if x && y { false } else { true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `!(x && y)`
error: this if-then-else expression will always return true
--> $DIR/needless_bool.rs:35:5
|
35 | if x { return true } else { return true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this if-then-else expression will always return false
--> $DIR/needless_bool.rs:40:5
|
40 | if x { return false } else { return false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:45:5 --> $DIR/needless_bool.rs:45:5
| |
45 | if x { return true } else { return false }; 45 | if x && y { false } else { true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `!(x && y)`
error: this if-then-else expression will always return true
--> $DIR/needless_bool.rs:60:5
|
60 | if x { return true } else { return true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this if-then-else expression will always return false
--> $DIR/needless_bool.rs:65:5
|
65 | if x { return false } else { return false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:70:5
|
70 | if x { return true } else { return false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return x` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return x`
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:50:5 --> $DIR/needless_bool.rs:75:5
| |
50 | if x && y { return true } else { return false }; 75 | if x && y { return true } else { return false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return x && y` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return x && y`
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:55:5 --> $DIR/needless_bool.rs:80:5
| |
55 | if x { return false } else { return true }; 80 | if x { return false } else { return true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return !x` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return !x`
error: this if-then-else expression returns a bool literal error: this if-then-else expression returns a bool literal
--> $DIR/needless_bool.rs:60:5 --> $DIR/needless_bool.rs:85:5
| |
60 | if x && y { return false } else { return true }; 85 | if x && y { return false } else { return true };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return !(x && y)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `return !(x && y)`
error: aborting due to 11 previous errors error: equality checks against true are unnecessary
--> $DIR/needless_bool.rs:89:7
|
89 | if x == true { };
| ^^^^^^^^^^ help: try simplifying it as shown: `x`
|
= note: `-D clippy::bool-comparison` implied by `-D warnings`
error: equality checks against false can be replaced by a negation
--> $DIR/needless_bool.rs:93:7
|
93 | if x == false { };
| ^^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: equality checks against true are unnecessary
--> $DIR/needless_bool.rs:104:8
|
104 | if x == true { };
| ^^^^^^^^^ help: try simplifying it as shown: `x`
error: equality checks against false can be replaced by a negation
--> $DIR/needless_bool.rs:105:8
|
105 | if x == false { };
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: aborting due to 15 previous errors