From 1e26c4401049fed53b9b1aee44a1084f73d50104 Mon Sep 17 00:00:00 2001 From: sinkuu Date: Wed, 22 Feb 2017 16:47:18 +0900 Subject: [PATCH] Suggest `assert_ne` for `assert!(x != y)` --- clippy_lints/src/should_assert_eq.rs | 15 ++++++++++----- tests/ui/should_assert_eq.rs | 5 +++++ tests/ui/should_assert_eq.stderr | 24 ++++++++++++++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/should_assert_eq.rs b/clippy_lints/src/should_assert_eq.rs index e6296154126..372ee210336 100644 --- a/clippy_lints/src/should_assert_eq.rs +++ b/clippy_lints/src/should_assert_eq.rs @@ -2,10 +2,10 @@ use rustc::lint::*; use rustc::hir::*; use utils::{is_direct_expn_of, implements_trait, span_lint}; -/// **What it does:** Checks for `assert!(x == y)` which can be better written -/// as `assert_eq!(x, y)` if `x` and `y` implement `Debug` trait. +/// **What it does:** Checks for `assert!(x == y)` or `assert!(x != y)` which can be better written +/// using `assert_eq` or `assert_ne` if `x` and `y` implement `Debug` trait. /// -/// **Why is this bad?** `assert_eq` provides better assertion failure reporting. +/// **Why is this bad?** `assert_eq` and `assert_ne` provide better assertion failure reporting. /// /// **Known problems:** Hopefully none. /// @@ -36,10 +36,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ShouldAssertEq { let ExprIf(ref cond, ..) = e.node, let ExprUnary(UnOp::UnNot, ref cond) = cond.node, let ExprBinary(ref binop, ref expr1, ref expr2) = cond.node, - binop.node == BinOp_::BiEq, is_direct_expn_of(cx, e.span, "assert").is_some(), let Some(debug_trait) = cx.tcx.lang_items.debug_trait(), ], { + let sugg = match binop.node { + BinOp_::BiEq => "assert_eq", + BinOp_::BiNe => "assert_ne", + _ => return, + }; + let ty1 = cx.tables.expr_ty(expr1); let ty2 = cx.tables.expr_ty(expr2); @@ -47,7 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ShouldAssertEq { if implements_trait(cx, ty1, debug_trait, &[], Some(parent)) && implements_trait(cx, ty2, debug_trait, &[], Some(parent)) { - span_lint(cx, SHOULD_ASSERT_EQ, e.span, "use `assert_eq` for better reporting"); + span_lint(cx, SHOULD_ASSERT_EQ, e.span, &format!("use `{}` for better reporting", sugg)); } }} } diff --git a/tests/ui/should_assert_eq.rs b/tests/ui/should_assert_eq.rs index e6144f0027a..f00278bfc7e 100644 --- a/tests/ui/should_assert_eq.rs +++ b/tests/ui/should_assert_eq.rs @@ -14,6 +14,8 @@ fn main() { assert!(1 == 2); assert!(Debug(1) == Debug(2)); assert!(NonDebug(1) == NonDebug(1)); // ok + assert!(Debug(1) != Debug(2)); + assert!(NonDebug(1) != NonDebug(2)); // ok test_generic(1, 2, 3, 4); } @@ -21,4 +23,7 @@ fn main() { fn test_generic(x: T, y: T, z: U, w: U) { assert!(x == y); assert!(z == w); // ok + + assert!(x != y); + assert!(z != w); // ok } diff --git a/tests/ui/should_assert_eq.stderr b/tests/ui/should_assert_eq.stderr index 6cd729e4035..8dcf71698df 100644 --- a/tests/ui/should_assert_eq.stderr +++ b/tests/ui/should_assert_eq.stderr @@ -19,13 +19,29 @@ error: use `assert_eq` for better reporting | = note: this error originates in a macro outside of the current crate -error: use `assert_eq` for better reporting - --> $DIR/should_assert_eq.rs:22:5 +error: use `assert_ne` for better reporting + --> $DIR/should_assert_eq.rs:17:5 | -22 | assert!(x == y); +17 | assert!(Debug(1) != Debug(2)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate + +error: use `assert_eq` for better reporting + --> $DIR/should_assert_eq.rs:24:5 + | +24 | assert!(x == y); | ^^^^^^^^^^^^^^^^ | = note: this error originates in a macro outside of the current crate -error: aborting due to 3 previous errors +error: use `assert_ne` for better reporting + --> $DIR/should_assert_eq.rs:27:5 + | +27 | assert!(x != y); + | ^^^^^^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate + +error: aborting due to 5 previous errors