From 9b79b1022c68b79dbcdf2e51d46843c6e801d1f7 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 4 Jul 2016 01:17:31 +0200 Subject: [PATCH] Fix suggestions for `needless_bool` --- clippy_lints/src/needless_bool.rs | 36 ++++++++++++++++++----------- tests/compile-fail/needless_bool.rs | 31 +++++++++++++++++++++---- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 5f912366770..fa2a350c36f 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -6,7 +6,8 @@ use rustc::lint::*; use rustc::hir::*; use syntax::ast::LitKind; use syntax::codemap::Spanned; -use utils::{span_lint, span_lint_and_then, snippet, snippet_opt}; +use utils::{span_lint, span_lint_and_then, snippet}; +use utils::sugg::Sugg; /// **What it does:** This lint checks for expressions of the form `if c { true } else { false }` (or vice versa) and suggest using the condition directly. /// @@ -49,11 +50,20 @@ impl LateLintPass for NeedlessBool { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { use self::Expression::*; if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node { - let reduce = |hint: &str, not| { - let hint = match snippet_opt(cx, pred.span) { - Some(pred_snip) => format!("`{}{}`", not, pred_snip), - None => hint.into(), + let reduce = |ret, not| { + let snip = Sugg::hir(cx, pred, ""); + let snip = if not { + !snip + } else { + snip }; + + let hint = if ret { + format!("return {};", snip) + } else { + snip.to_string() + }; + span_lint_and_then(cx, NEEDLESS_BOOL, e.span, @@ -77,10 +87,10 @@ impl LateLintPass for NeedlessBool { e.span, "this if-then-else expression will always return false"); } - (RetBool(true), RetBool(false)) => reduce("its predicate", "return "), - (Bool(true), Bool(false)) => reduce("its predicate", ""), - (RetBool(false), RetBool(true)) => reduce("`!` and its predicate", "return !"), - (Bool(false), Bool(true)) => reduce("`!` and its predicate", "!"), + (RetBool(true), RetBool(false)) => reduce(true, false), + (Bool(true), Bool(false)) => reduce(false, false), + (RetBool(false), RetBool(true)) => reduce(true, true), + (Bool(false), Bool(true)) => reduce(false, true), _ => (), } } @@ -122,23 +132,23 @@ impl LateLintPass for BoolComparison { }); } (Bool(false), Other) => { - let hint = format!("!{}", snippet(cx, right_side.span, "..")); + let hint = Sugg::hir(cx, right_side, ".."); span_lint_and_then(cx, BOOL_COMPARISON, e.span, "equality checks against false can be replaced by a negation", |db| { - db.span_suggestion(e.span, "try simplifying it as shown:", hint); + db.span_suggestion(e.span, "try simplifying it as shown:", (!hint).to_string()); }); } (Other, Bool(false)) => { - let hint = format!("!{}", snippet(cx, left_side.span, "..")); + let hint = Sugg::hir(cx, left_side, ".."); span_lint_and_then(cx, BOOL_COMPARISON, e.span, "equality checks against false can be replaced by a negation", |db| { - db.span_suggestion(e.span, "try simplifying it as shown:", hint); + db.span_suggestion(e.span, "try simplifying it as shown:", (!hint).to_string()); }); } _ => (), diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs index 480c16f1666..fb81d44308a 100644 --- a/tests/compile-fail/needless_bool.rs +++ b/tests/compile-fail/needless_bool.rs @@ -5,21 +5,28 @@ #[allow(if_same_then_else)] fn main() { let x = true; + let y = false; if x { true } else { true }; //~ERROR this if-then-else expression will always return true if x { false } else { false }; //~ERROR this if-then-else expression will always return false if x { true } else { false }; //~^ ERROR this if-then-else expression returns a bool literal //~| HELP you can reduce it to - //~| SUGGESTION `x` + //~| SUGGESTION x if x { false } else { true }; //~^ ERROR this if-then-else expression returns a bool literal //~| HELP you can reduce it to - //~| SUGGESTION `!x` + //~| SUGGESTION !x + if x && y { false } else { true }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION !(x && y) if x { x } else { false }; // would also be questionable, but we don't catch this yet bool_ret(x); bool_ret2(x); bool_ret3(x); + bool_ret5(x, x); bool_ret4(x); + bool_ret6(x, x); } #[allow(if_same_then_else, needless_return)] @@ -39,7 +46,15 @@ fn bool_ret3(x: bool) -> bool { if x { return true } else { return false }; //~^ ERROR this if-then-else expression returns a bool literal //~| HELP you can reduce it to - //~| SUGGESTION `return x` + //~| SUGGESTION return x +} + +#[allow(needless_return)] +fn bool_ret5(x: bool, y: bool) -> bool { + if x && y { return true } else { return false }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION return x && y } #[allow(needless_return)] @@ -47,5 +62,13 @@ fn bool_ret4(x: bool) -> bool { if x { return false } else { return true }; //~^ ERROR this if-then-else expression returns a bool literal //~| HELP you can reduce it to - //~| SUGGESTION `return !x` + //~| SUGGESTION return !x +} + +#[allow(needless_return)] +fn bool_ret6(x: bool, y: bool) -> bool { + if x && y { return false } else { return true }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION return !(x && y) }