From 3d3af078454bcd10980af28a04e172cc1f6bc9c8 Mon Sep 17 00:00:00 2001 From: mgr-inz-rafal Date: Mon, 23 Mar 2020 21:00:02 +0100 Subject: [PATCH] Provide appropriate suggestion --- clippy_lints/src/needless_bool.rs | 45 +++++++++++++++++++++---------- tests/ui/bool_comparison.fixed | 4 +-- tests/ui/bool_comparison.rs | 5 ++++ tests/ui/bool_comparison.stderr | 12 +++------ 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 8de2fe2f3ba..a93c55f2d59 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -3,7 +3,7 @@ //! This lint is **warn** by default use crate::utils::sugg::Sugg; -use crate::utils::{higher, parent_node_is_if_expr, span_lint, span_lint_and_help, span_lint_and_sugg}; +use crate::utils::{higher, parent_node_is_if_expr, span_lint, span_lint_and_help, span_lint_and_sugg, snippet_with_applicability}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -189,19 +189,23 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) { } } -fn is_unary_not<'tcx>(e: &'tcx Expr<'_>) -> bool { +fn is_unary_not<'tcx>(e: &'tcx Expr<'_>) -> (bool, rustc_span::Span) { if_chain! { - if let ExprKind::Unary(unop, _) = e.kind; + if let ExprKind::Unary(unop, operand) = e.kind; if let UnOp::UnNot = unop; then { - return true; + return (true, operand.span); } }; - false + (false, e.span) } -fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> bool { - is_unary_not(left_side) ^ is_unary_not(right_side) +fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> (bool, rustc_span::Span, rustc_span::Span) { + let left = is_unary_not(left_side); + let right = is_unary_not(right_side); + + let retval = left.0 ^ right.0; + (retval, left.1, right.1) } fn check_comparison<'a, 'tcx>( @@ -218,14 +222,27 @@ fn check_comparison<'a, 'tcx>( if let ExprKind::Binary(op, ref left_side, ref right_side) = e.kind { let (l_ty, r_ty) = (cx.tables.expr_ty(left_side), cx.tables.expr_ty(right_side)); if l_ty.is_bool() && r_ty.is_bool() { - if_chain! { - if let BinOpKind::Eq = op.node; - if one_side_is_unary_not(&left_side, &right_side); - then { - span_lint_and_help(cx, BOOL_COMPARISON, e.span, "Here comes", "the suggestion"); - } - }; let mut applicability = Applicability::MachineApplicable; + + if let BinOpKind::Eq = op.node + { + let xxx = one_side_is_unary_not(&left_side, &right_side); + if xxx.0 + { + span_lint_and_sugg( + cx, + BOOL_COMPARISON, + e.span, + "This comparison might be written more concisely", + "try simplifying it as shown", + format!("{} != {}", + snippet_with_applicability(cx, xxx.1, "..", &mut applicability), + snippet_with_applicability(cx, xxx.2, "..", &mut applicability)), + applicability, + ) + } + } + match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) { (Bool(true), Other) => left_true.map_or((), |(h, m)| { suggest_bool_comparison(cx, e, right_side, applicability, m, h) diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed index d217d03ead4..2081dea36a7 100644 --- a/tests/ui/bool_comparison.fixed +++ b/tests/ui/bool_comparison.fixed @@ -116,8 +116,8 @@ fn issue4983() { let a = true; let b = false; - if a == !b {}; - if !a == b {}; + if a != b {}; + if a != b {}; if a == b {}; if !a == !b {}; } diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index c13575eae71..f463ac1c883 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -120,4 +120,9 @@ fn issue4983() { if !a == b {}; if a == b {}; if !a == !b {}; + + if b == !a {}; + if !b == a {}; + if b == a {}; + if !b == !a {}; } diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr index cec7b196a23..a2a89fc3287 100644 --- a/tests/ui/bool_comparison.stderr +++ b/tests/ui/bool_comparison.stderr @@ -84,21 +84,17 @@ error: order comparisons between booleans can be simplified LL | if x > y { | ^^^^^ help: try simplifying it as shown: `x & !y` -error: Here comes +error: This comparison might be written more concisely --> $DIR/bool_comparison.rs:119:8 | LL | if a == !b {}; - | ^^^^^^^ - | - = help: the suggestion + | ^^^^^^^ help: try simplifying it as shown: `a != b` -error: Here comes +error: This comparison might be written more concisely --> $DIR/bool_comparison.rs:120:8 | LL | if !a == b {}; - | ^^^^^^^ - | - = help: the suggestion + | ^^^^^^^ help: try simplifying it as shown: `a != b` error: aborting due to 16 previous errors