Added lint to avoid negated comparisions on partially ordered types.

Author: 0ndorio

clippy_lints/src/lib.rs

@@ -169,6 +169,7 @@ pub mod needless_borrowed_ref;
 pub mod needless_continue;
 pub mod needless_pass_by_value;
 pub mod needless_update;
+pub mod neg_cmp_op_on_partial_ord;
 pub mod neg_multiply;
 pub mod new_without_default;
 pub mod no_effect;
@@ -419,7 +420,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box map_unit_fn::Pass);
     reg.register_late_lint_pass(box infallible_destructuring_match::Pass);
     reg.register_late_lint_pass(box inherent_impl::Pass::default());
-
+    reg.register_late_lint_pass(box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
 
     reg.register_lint_group("clippy_restriction", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -501,6 +502,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         booleans::LOGIC_BUG,
         booleans::NONMINIMAL_BOOL,
         bytecount::NAIVE_BYTECOUNT,
+        neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD,
         collapsible_if::COLLAPSIBLE_IF,
         const_static_lifetime::CONST_STATIC_LIFETIME,
         copies::IF_SAME_THEN_ELSE,

clippy_lints/src/neg_cmp_op_on_partial_ord.rs

@@ -0,0 +1,95 @@
+use rustc::hir::*;
+use rustc::lint::*;
+
+use crate::utils;
+
+const ORD: [&str; 3] = ["core", "cmp", "Ord"];
+const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
+
+/// **What it does:**
+/// Checks for the usage of negated comparision operators on types which only implement
+/// `PartialOrd` (e.g. `f64`).
+///
+/// **Why is this bad?**
+/// These operators make it easy to forget that the underlying types actually allow not only three
+/// potential Orderings (Less, Equal, Greater) but also a forth one (Uncomparable). Escpeccially if
+/// the operator based comparision result is negated it is easy to miss that fact.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+///
+/// ```rust
+/// use core::cmp::Ordering;
+/// 
+/// // Bad
+/// let a = 1.0;
+/// let b = std::f64::NAN;
+/// 
+/// let _not_less_or_equal = !(a <= b);
+///
+/// // Good
+/// let a = 1.0;
+/// let b = std::f64::NAN;
+/// 
+/// let _not_less_or_equal = match a.partial_cmp(&b) {
+///     None | Some(Ordering::Greater) => true,
+///     _ => false, 
+/// };
+/// ```
+declare_lint! {
+    pub NEG_CMP_OP_ON_PARTIAL_ORD, Warn,
+    "The use of negated comparision operators on partially orded types may produce confusing code."
+}
+
+pub struct NoNegCompOpForPartialOrd;
+
+impl LintPass for NoNegCompOpForPartialOrd {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(NEG_CMP_OP_ON_PARTIAL_ORD)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NoNegCompOpForPartialOrd {
+
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if_chain! {
+
+            if let Expr_::ExprUnary(UnOp::UnNot, ref inner) = expr.node;
+            if let Expr_::ExprBinary(ref op, ref left, _) = inner.node;
+            if let BinOp_::BiLe | BinOp_::BiGe | BinOp_::BiLt | BinOp_::BiGt = op.node;
+
+            then {
+
+                let ty = cx.tables.expr_ty(left);
+
+                let implements_ord = {
+                    if let Some(id) = utils::get_trait_def_id(cx, &ORD) {
+                        utils::implements_trait(cx, ty, id, &[])
+                    } else {
+                        return;
+                    }
+                };
+
+                let implements_partial_ord = {
+                    if let Some(id) = utils::get_trait_def_id(cx, &PARTIAL_ORD) {
+                        utils::implements_trait(cx, ty, id, &[])
+                    } else {
+                        return;
+                    }
+                };
+
+                if implements_partial_ord && !implements_ord {
+                    cx.span_lint(
+                        NEG_CMP_OP_ON_PARTIAL_ORD,
+                        expr.span,
+                        "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."
+                    )
+                }
+            }
+        }
+    }
+}

tests/ui/neg_cmp_op_on_partial_ord.rs

@@ -0,0 +1,59 @@
+/// This test case utilizes `f64` an easy example for `PartialOrd` only types
+/// but the lint itself actually validates any expression where the left
+/// operand implements `PartialOrd` but not `Ord`.
+
+use std::cmp::Ordering;
+
+#[allow(nonminimal_bool)]
+#[warn(neg_cmp_op_on_partial_ord)]
+fn main() {
+
+    let a_value = 1.0;
+    let another_value = 7.0;
+
+    // --- Bad ---
+
+
+    // Not Less but potentially Greater, Equal or Uncomparable.
+    let _not_less = !(a_value < another_value);
+    
+    // Not Less or Equal but potentially Greater or Uncomparable.
+    let _not_less_or_equal = !(a_value <= another_value);
+
+    // Not Greater but potentially Less, Equal or Uncomparable.
+    let _not_greater = !(a_value > another_value);
+
+    // Not Greater or Equal but potentially Less or Uncomparable.
+    let _not_greater_or_equal = !(a_value >= another_value);
+
+
+    // --- Good ---
+
+
+    let _not_less = match a_value.partial_cmp(&another_value) {
+        None | Some(Ordering::Greater) | Some(Ordering::Equal)  => true,
+	_ => false,
+    };
+    let _not_less_or_equal = match a_value.partial_cmp(&another_value) {
+        None | Some(Ordering::Greater) => true,
+        _ => false,
+    };
+    let _not_greater = match a_value.partial_cmp(&another_value) {
+        None | Some(Ordering::Less) | Some(Ordering::Equal) => true,
+	_ => false,
+    };
+    let _not_greater_or_equal = match a_value.partial_cmp(&another_value) {
+        None | Some(Ordering::Less) => true,
+	_ => false,
+    };
+
+
+    // --- Should not trigger ---
+
+
+    let _ = a_value < another_value;
+    let _ = a_value <= another_value;
+    let _ = a_value > another_value;
+    let _ = a_value >= another_value;
+}
+

tests/ui/neg_cmp_op_on_partial_ord.stderr

@@ -0,0 +1,28 @@
+error: The use of negated comparision operators on partially ordedtypes produces code that is hard to read and refactor. Pleaseconsider to use the partial_cmp() instead, to make it clearthat the two values could be incomparable.
+  --> $DIR/neg_cmp_op_on_partial_ord.rs:18:21
+   |
+18 |     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 ordedtypes produces code that is hard to read and refactor. Pleaseconsider to use the partial_cmp() instead, to make it clearthat the two values could be incomparable.
+  --> $DIR/neg_cmp_op_on_partial_ord.rs:21:30
+   |
+21 |     let _not_less_or_equal = !(a_value <= another_value);
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: The use of negated comparision operators on partially ordedtypes produces code that is hard to read and refactor. Pleaseconsider to use the partial_cmp() instead, to make it clearthat the two values could be incomparable.
+  --> $DIR/neg_cmp_op_on_partial_ord.rs:24:24
+   |
+24 |     let _not_greater = !(a_value > another_value);
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: The use of negated comparision operators on partially ordedtypes produces code that is hard to read and refactor. Pleaseconsider to use the partial_cmp() instead, to make it clearthat the two values could be incomparable.
+  --> $DIR/neg_cmp_op_on_partial_ord.rs:27:33
+   |
+27 |     let _not_greater_or_equal = !(a_value >= another_value);
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+