Add should_assert_eq lint (fixes #641)

clippy_lints/src/lib.rs

@@ -126,6 +126,7 @@ pub mod regex;
 pub mod returns;
 pub mod serde;
 pub mod shadow;
+pub mod should_assert_eq;
 pub mod strings;
 pub mod swap;
 pub mod temporary_assignment;
@@ -297,6 +298,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_early_lint_pass(box double_parens::DoubleParens);
     reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount);
     reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold));
+    reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,

clippy_lints/src/should_assert_eq.rs

@@ -0,0 +1,59 @@
+use rustc::lint::*;
+use rustc::hir::*;
+use utils::{paths, is_direct_expn_of, get_trait_def_id, implements_trait, span_lint};
+
+/// **What it does:** Checks for `assert!(x == y)` which can be written better
+/// as `assert_eq!(x, y)` if `x` and `y` implement `Debug` trait.
+///
+/// **Why is this bad?** `assert_eq` provides better assertion failure reporting.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// let (x, y) = (1, 2);
+///
+/// assert!(x == y);  // assertion failed: x == y
+/// assert_eq!(x, y); // assertion failed: `(left == right)` (left: `1`, right: `2`)
+/// ```
+declare_lint! {
+    pub SHOULD_ASSERT_EQ,
+    Warn,
+    "using `assert` macro for asserting equality"
+}
+
+pub struct ShouldAssertEq;
+
+impl LintPass for ShouldAssertEq {
+    fn get_lints(&self) -> LintArray {
+        lint_array![SHOULD_ASSERT_EQ]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ShouldAssertEq {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
+        if_let_chain! {[
+            let ExprIf(ref cond, ..) = e.node,
+            let ExprUnary(UnOp::UnNot, ref cond) = cond.node,
+            let ExprBinary(ref binop, ref expr1, ref expr2) = cond.node,
+        ], {
+            if is_direct_expn_of(cx, e.span, "assert").is_none() {
+                return;
+            }
+
+            if binop.node != BinOp_::BiEq {
+                return;
+            }
+
+            let debug_trait = get_trait_def_id(cx, &paths::DEBUG_TRAIT)
+                .expect("cannot find Debug trait");
+
+            let ty1 = cx.tables.expr_ty(expr1);
+            let ty2 = cx.tables.expr_ty(expr2);
+            if implements_trait(cx, ty1, debug_trait, vec![]) &&
+                implements_trait(cx, ty2, debug_trait, vec![]) {
+                span_lint(cx, SHOULD_ASSERT_EQ, e.span, "use `assert_eq` for better reporting");
+            }
+        }}
+    }
+}

clippy_lints/src/utils/paths.rs

@@ -15,6 +15,7 @@ pub const CMP_MIN: [&'static str; 3] = ["core", "cmp", "min"];
 pub const COW: [&'static str; 3] = ["collections", "borrow", "Cow"];
 pub const CSTRING_NEW: [&'static str; 5] = ["std", "ffi", "c_str", "CString", "new"];
 pub const DEBUG_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Debug", "fmt"];
+pub const DEBUG_TRAIT: [&'static str; 3] = ["core", "fmt", "Debug"];
 pub const DEFAULT_TRAIT: [&'static str; 3] = ["core", "default", "Default"];
 pub const DISPLAY_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Display", "fmt"];
 pub const DROP: [&'static str; 3] = ["core", "mem", "drop"];

tests/ui/float_cmp.rs

@@ -92,5 +92,5 @@ fn main() {
     let a: *const f32 = xs.as_ptr();
     let b: *const f32 = xs.as_ptr();
 
-    assert!(a == b); // no errors
+    assert_eq!(a, b); // no errors
 }

tests/ui/should_assert_eq.rs

@@ -0,0 +1,16 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(should_assert_eq)]
+
+#[derive(PartialEq, Eq)]
+struct NonDebug(i32);
+
+#[derive(Debug, PartialEq, Eq)]
+struct Debug(i32);
+
+fn main() {
+    assert!(1 == 2);
+    assert!(Debug(1) == Debug(2));
+    assert!(NonDebug(1) == NonDebug(1)); // ok
+}

tests/ui/should_assert_eq.stderr

@@ -0,0 +1,23 @@
+error: use `assert_eq` for better reporting
+  --> $DIR/should_assert_eq.rs:13:5
+   |
+13 |     assert!(1 == 2);
+   |     ^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/should_assert_eq.rs:4:9
+   |
+4  | #![deny(should_assert_eq)]
+   |         ^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro outside of the current crate
+
+error: use `assert_eq` for better reporting
+  --> $DIR/should_assert_eq.rs:14:5
+   |
+14 |     assert!(Debug(1) == Debug(2));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate
+
+error: aborting due to 2 previous errors
+