diff --git a/CHANGELOG.md b/CHANGELOG.md
index 90f67e8aec7..1192195cd6a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -423,6 +423,7 @@ All notable changes to this project will be documented in this file.
[`shadow_same`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_same
[`shadow_unrelated`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated
[`short_circuit_statement`]: https://github.com/Manishearth/rust-clippy/wiki#short_circuit_statement
+[`should_assert_eq`]: https://github.com/Manishearth/rust-clippy/wiki#should_assert_eq
[`should_implement_trait`]: https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait
[`similar_names`]: https://github.com/Manishearth/rust-clippy/wiki#similar_names
[`single_char_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern
diff --git a/README.md b/README.md
index 9e4b9fba1b6..842e654678b 100644
--- a/README.md
+++ b/README.md
@@ -180,7 +180,7 @@ transparently:
## Lints
-There are 190 lints included in this crate:
+There are 191 lints included in this crate:
name | default | triggers on
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -329,6 +329,7 @@ name
[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | rebinding a name without even using the original value
[short_circuit_statement](https://github.com/Manishearth/rust-clippy/wiki#short_circuit_statement) | warn | using a short circuit boolean condition as a statement
+[should_assert_eq](https://github.com/Manishearth/rust-clippy/wiki#should_assert_eq) | warn | using `assert` macro for asserting equality
[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait
[similar_names](https://github.com/Manishearth/rust-clippy/wiki#similar_names) | allow | similarly named items and bindings
[single_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")`
diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
index 71ebb996fc5..ea3e29cff12 100644
--- a/clippy_lints/src/assign_ops.rs
+++ b/clippy_lints/src/assign_ops.rs
@@ -152,7 +152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
let hir::Item_::ItemImpl(_, _, _, Some(ref trait_ref), _, _) = item.node,
trait_ref.path.def.def_id() == trait_id
], { return; }}
- implements_trait($cx, $ty, trait_id, vec![$rty])
+ implements_trait($cx, $ty, trait_id, &[$rty], None)
},)*
_ => false,
}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 13ad8b41f96..814daf7d645 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,
@@ -479,6 +481,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
serde::SERDE_API_MISUSE,
+ should_assert_eq::SHOULD_ASSERT_EQ,
strings::STRING_LIT_AS_BYTES,
swap::ALMOST_SWAPPED,
swap::MANUAL_SWAP,
diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index b74bd9266a4..9acf4c7e973 100644
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -724,7 +724,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir:
return false;
};
- if implements_trait(cx, arg_ty, default_trait_id, Vec::new()) {
+ if implements_trait(cx, arg_ty, default_trait_id, &[], None) {
span_lint_and_then(cx,
OR_FUN_CALL,
span,
@@ -1268,7 +1268,7 @@ fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
/// This checks whether a given type is known to implement Debug.
fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool {
match cx.tcx.lang_items.debug_trait() {
- Some(debug) => implements_trait(cx, ty, debug, Vec::new()),
+ Some(debug) => implements_trait(cx, ty, debug, &[], None),
None => false,
}
}
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index d397d425111..d2f794def0c 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -420,7 +420,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: S
None => return,
};
- if !implements_trait(cx, arg_ty, partial_eq_trait_id, vec![other_ty]) {
+ if !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty], None) {
return;
}
diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs
index e4e98c26038..ece7e130c5f 100644
--- a/clippy_lints/src/new_without_default.rs
+++ b/clippy_lints/src/new_without_default.rs
@@ -115,7 +115,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
self_ty.walk_shallow().next().is_none(), // implements_trait does not work with generics
same_tys(cx, self_ty, return_ty(cx, id), id),
let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT),
- !implements_trait(cx, self_ty, default_trait_id, Vec::new())
+ !implements_trait(cx, self_ty, default_trait_id, &[], None)
], {
if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) {
span_lint_and_then(cx,
@@ -156,7 +156,7 @@ fn can_derive_default<'t, 'c>(ty: ty::Ty<'t>, cx: &LateContext<'c, 't>, default_
ty::TyAdt(adt_def, substs) if adt_def.is_struct() => {
for field in adt_def.all_fields() {
let f_ty = field.ty(cx.tcx, substs);
- if !implements_trait(cx, f_ty, default_trait_id, Vec::new()) {
+ if !implements_trait(cx, f_ty, default_trait_id, &[], None) {
return None;
}
}
diff --git a/clippy_lints/src/should_assert_eq.rs b/clippy_lints/src/should_assert_eq.rs
new file mode 100644
index 00000000000..e6296154126
--- /dev/null
+++ b/clippy_lints/src/should_assert_eq.rs
@@ -0,0 +1,54 @@
+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.
+///
+/// **Why is this bad?** `assert_eq` provides better assertion failure reporting.
+///
+/// **Known problems:** Hopefully 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,
+ 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 ty1 = cx.tables.expr_ty(expr1);
+ let ty2 = cx.tables.expr_ty(expr2);
+
+ let parent = cx.tcx.hir.get_parent(e.id);
+
+ 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");
+ }
+ }}
+ }
+}
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 1211b77ddb8..ce581febfe4 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -317,13 +317,19 @@ pub fn implements_trait<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
ty: ty::Ty<'tcx>,
trait_id: DefId,
- ty_params: Vec<ty::Ty<'tcx>>
+ ty_params: &[ty::Ty<'tcx>],
+ parent_node_id: Option<NodeId>
) -> bool {
cx.tcx.populate_implementations_for_trait_if_necessary(trait_id);
let ty = cx.tcx.erase_regions(&ty);
- cx.tcx.infer_ctxt((), Reveal::All).enter(|infcx| {
- let obligation = cx.tcx.predicate_for_trait_def(traits::ObligationCause::dummy(), trait_id, 0, ty, &ty_params);
+ let mut b = if let Some(id) = parent_node_id {
+ cx.tcx.infer_ctxt(BodyId { node_id: id }, Reveal::All)
+ } else {
+ cx.tcx.infer_ctxt((), Reveal::All)
+ };
+ b.enter(|infcx| {
+ let obligation = cx.tcx.predicate_for_trait_def(traits::ObligationCause::dummy(), trait_id, 0, ty, ty_params);
traits::SelectionContext::new(&infcx).evaluate_obligation_conservatively(&obligation)
})
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..5abf3566425
--- /dev/null
+++ b/tests/ui/should_assert_eq.rs
@@ -0,0 +1,23 @@
+#![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
+
+ test_generic(1, 2, 3, 4);
+}
+
+fn test_generic<T: std::fmt::Debug + Eq, U: Eq>(x: T, y: T, z: U, w: U) {
+ assert!(x == y);
+ assert!(z == w); // ok
+}
diff --git a/tests/ui/should_assert_eq.stderr b/tests/ui/should_assert_eq.stderr
new file mode 100644
index 00000000000..68d287fda8d
--- /dev/null
+++ b/tests/ui/should_assert_eq.stderr
@@ -0,0 +1,31 @@
+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: use `assert_eq` for better reporting
+ --> $DIR/should_assert_eq.rs:21:5
+ |
+21 | assert!(x == y);
+ | ^^^^^^^^^^^^^^^^
+ |
+ = note: this error originates in a macro outside of the current crate
+
+error: aborting due to 3 previous errors
+