From c0c0686a6571492da6b6fdf8bed0e8527c2d19c5 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 25 Feb 2019 21:48:20 +0100 Subject: [PATCH] Fix `bool_comparison` with non-`bool` expressions --- clippy_lints/src/needless_bool.rs | 40 +++++++++++++++---------------- tests/ui/bool_comparison.rs | 36 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 49f607f52..eca70c6a4 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -225,23 +225,23 @@ fn check_comparison<'a, 'tcx>( use self::Expression::*; if let ExprKind::Binary(_, ref left_side, ref right_side) = e.node { - let mut applicability = Applicability::MachineApplicable; - 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) - }), - (Other, Bool(true)) => right_true.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, left_side, applicability, m, h) - }), - (Bool(false), Other) => left_false.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, right_side, applicability, m, h) - }), - (Other, Bool(false)) => right_false.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, left_side, applicability, m, h) - }), - (Other, Other) => no_literal.map_or((), |(h, m)| { - 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() { + 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() { + let mut applicability = Applicability::MachineApplicable; + 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) + }), + (Other, Bool(true)) => right_true.map_or((), |(h, m)| { + suggest_bool_comparison(cx, e, left_side, applicability, m, h) + }), + (Bool(false), Other) => left_false.map_or((), |(h, m)| { + suggest_bool_comparison(cx, e, right_side, applicability, m, h) + }), + (Other, Bool(false)) => right_false.map_or((), |(h, m)| { + suggest_bool_comparison(cx, e, left_side, applicability, m, h) + }), + (Other, Other) => no_literal.map_or((), |(h, m)| { let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability); let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability); span_lint_and_sugg( @@ -253,9 +253,9 @@ fn check_comparison<'a, 'tcx>( h(left_side, right_side).to_string(), applicability, ) - } - }), - _ => (), + }), + _ => (), + } } } } diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index 48c5e9d6d..36d31aa04 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -73,3 +73,39 @@ fn main() { "no" }; } + +#[allow(dead_code)] +fn issue3703() { + struct Foo; + impl PartialEq for Foo { + fn eq(&self, _: &bool) -> bool { + true + } + } + impl PartialEq for bool { + fn eq(&self, _: &Foo) -> bool { + true + } + } + impl PartialOrd for Foo { + fn partial_cmp(&self, _: &bool) -> Option { + None + } + } + impl PartialOrd for bool { + fn partial_cmp(&self, _: &Foo) -> Option { + None + } + } + + if Foo == true {} + if true == Foo {} + if Foo != true {} + if true != Foo {} + if Foo == false {} + if false == Foo {} + if Foo != false {} + if false != Foo {} + if Foo < false {} + if false < Foo {} +}