diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index c814c1abcd13..8816d50c5c26 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -4,7 +4,7 @@ use syntax::ast::{LitKind, NodeId, DUMMY_NODE_ID}; use syntax::codemap::{dummy_spanned, Span, DUMMY_SP}; use syntax::util::ThinVec; -use crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq}; +use crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq, get_trait_def_id, implements_trait}; /// **What it does:** Checks for boolean expressions that can be written more /// concisely. @@ -122,6 +122,12 @@ fn run(&mut self, e: &'v Expr) -> Result { } let negated = match e.node { ExprBinary(binop, ref lhs, ref rhs) => { + + match implements_ord(self.cx, lhs) { + Some(true) => (), + _ => continue, + }; + let mk_expr = |op| { Expr { id: DUMMY_NODE_ID, @@ -174,6 +180,12 @@ fn snip(&self, e: &Expr) -> Option { fn simplify_not(&self, expr: &Expr) -> Option { match expr.node { ExprBinary(binop, ref lhs, ref rhs) => { + + match implements_ord(self.cx, lhs) { + Some(true) => (), + _ => return None, + }; + match binop.node { BiEq => Some(" != "), BiNe => Some(" == "), @@ -444,3 +456,14 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::None } } + + +fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr) -> Option { + let ty = cx.tables.expr_ty(expr); + + return if let Some(id) = get_trait_def_id(cx, &paths::ORD) { + Some(implements_trait(cx, ty, id, &[])) + } else { + None + }; +} diff --git a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs index 6b89ded82558..8e70d0eeba0a 100644 --- a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs +++ b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs @@ -1,10 +1,7 @@ use rustc::hir::*; use rustc::lint::*; -use crate::utils; - -const ORD: [&str; 3] = ["core", "cmp", "Ord"]; -const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"]; +use crate::utils::{self, paths}; /// **What it does:** /// Checks for the usage of negated comparision operators on types which only implement @@ -65,7 +62,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { let ty = cx.tables.expr_ty(left); let implements_ord = { - if let Some(id) = utils::get_trait_def_id(cx, &ORD) { + if let Some(id) = utils::get_trait_def_id(cx, &paths::ORD) { utils::implements_trait(cx, ty, id, &[]) } else { return; @@ -73,7 +70,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { }; let implements_partial_ord = { - if let Some(id) = utils::get_trait_def_id(cx, &PARTIAL_ORD) { + if let Some(id) = utils::get_trait_def_id(cx, &paths::PARTIAL_ORD) { utils::implements_trait(cx, ty, id, &[]) } else { return; diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index a1cb6670bc5e..ab62346ea7ef 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -57,6 +57,8 @@ pub const OPTION: [&str; 3] = ["core", "option", "Option"]; pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"]; pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"]; +pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; +pub const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"]; pub const PTR_NULL: [&str; 2] = ["ptr", "null"]; pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"]; pub const RANGE: [&str; 3] = ["core", "ops", "Range"]; diff --git a/tests/ui/neg_cmp_op_on_partial_ord.rs b/tests/ui/neg_cmp_op_on_partial_ord.rs index f7fa09550e9e..214d627ba308 100644 --- a/tests/ui/neg_cmp_op_on_partial_ord.rs +++ b/tests/ui/neg_cmp_op_on_partial_ord.rs @@ -4,7 +4,6 @@ use std::cmp::Ordering; -#[allow(nonminimal_bool)] #[warn(neg_cmp_op_on_partial_ord)] fn main() { diff --git a/tests/ui/neg_cmp_op_on_partial_ord.stderr b/tests/ui/neg_cmp_op_on_partial_ord.stderr index 5f0c240b4596..5067ece87050 100644 --- a/tests/ui/neg_cmp_op_on_partial_ord.stderr +++ b/tests/ui/neg_cmp_op_on_partial_ord.stderr @@ -1,27 +1,27 @@ error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable. - --> $DIR/neg_cmp_op_on_partial_ord.rs:18:21 + --> $DIR/neg_cmp_op_on_partial_ord.rs:17:21 | -18 | let _not_less = !(a_value < another_value); +17 | let _not_less = !(a_value < another_value); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D neg-cmp-op-on-partial-ord` implied by `-D warnings` error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable. - --> $DIR/neg_cmp_op_on_partial_ord.rs:21:30 + --> $DIR/neg_cmp_op_on_partial_ord.rs:20:30 | -21 | let _not_less_or_equal = !(a_value <= another_value); +20 | let _not_less_or_equal = !(a_value <= another_value); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable. - --> $DIR/neg_cmp_op_on_partial_ord.rs:24:24 + --> $DIR/neg_cmp_op_on_partial_ord.rs:23:24 | -24 | let _not_greater = !(a_value > another_value); +23 | let _not_greater = !(a_value > another_value); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable. - --> $DIR/neg_cmp_op_on_partial_ord.rs:27:33 + --> $DIR/neg_cmp_op_on_partial_ord.rs:26:33 | -27 | let _not_greater_or_equal = !(a_value >= another_value); +26 | let _not_greater_or_equal = !(a_value >= another_value); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 4 previous errors