diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 13ad8b41f96..4f99dff89b4 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/clippy_lints/src/should_assert_eq.rs b/clippy_lints/src/should_assert_eq.rs new file mode 100644 index 00000000000..9bacc64f61c --- /dev/null +++ b/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"); + } + }} + } +} diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 5edff76d996..6ee35e3bd96 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/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"]; diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index de897caca08..024dd9c242c 100644 --- a/tests/ui/float_cmp.rs +++ b/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 } diff --git a/tests/ui/should_assert_eq.rs b/tests/ui/should_assert_eq.rs new file mode 100644 index 00000000000..41e3a411383 --- /dev/null +++ b/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 +} diff --git a/tests/ui/should_assert_eq.stderr b/tests/ui/should_assert_eq.stderr new file mode 100644 index 00000000000..3c51dbe0a9c --- /dev/null +++ b/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 +