From 3557084b0137f04c285197c2253e097c772cf047 Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Wed, 2 Oct 2019 22:48:19 +0200
Subject: [PATCH] Add check for assert_eq macros to unit_cmp lint

---
 clippy_lints/src/types.rs   | 26 ++++++++++++++++++++++++++
 tests/ui/unit_cmp.rs        |  6 ++++++
 tests/ui/unit_cmp.stderr    | 34 +++++++++++++++++++++++++++++++++-
 tests/ui/unused_unit.fixed  |  1 +
 tests/ui/unused_unit.rs     |  1 +
 tests/ui/unused_unit.stderr |  4 ++--
 6 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index e976b055791..b235b3eab2a 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -17,6 +17,8 @@ use rustc_target::spec::abi::Abi;
 use rustc_typeck::hir_ty_to_ty;
 use syntax::ast::{FloatTy, IntTy, LitIntType, LitKind, UintTy};
 use syntax::errors::DiagnosticBuilder;
+use syntax::ext::base::MacroKind;
+use syntax::ext::hygiene::ExpnKind;
 use syntax::source_map::Span;
 use syntax::symbol::{sym, Symbol};
 
@@ -527,6 +529,30 @@ declare_lint_pass!(UnitCmp => [UNIT_CMP]);
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitCmp {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         if expr.span.from_expansion() {
+            if let Some(callee) = expr.span.source_callee() {
+                if let ExpnKind::Macro(MacroKind::Bang, symbol) = callee.kind {
+                    if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
+                        let op = cmp.node;
+                        if op.is_comparison() && is_unit(cx.tables.expr_ty(left)) {
+                            let result = match &*symbol.as_str() {
+                                "assert_eq" | "debug_assert_eq" => "succeed",
+                                "assert_ne" | "debug_assert_ne" => "fail",
+                                _ => return,
+                            };
+                            span_lint(
+                                cx,
+                                UNIT_CMP,
+                                expr.span,
+                                &format!(
+                                    "{} of unit values detected. This will always {}",
+                                    symbol.as_str(),
+                                    result
+                                ),
+                            );
+                        }
+                    }
+                }
+            }
             return;
         }
         if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
diff --git a/tests/ui/unit_cmp.rs b/tests/ui/unit_cmp.rs
index 48c22f7f875..71c4348a2a1 100644
--- a/tests/ui/unit_cmp.rs
+++ b/tests/ui/unit_cmp.rs
@@ -20,4 +20,10 @@ fn main() {
     } > {
         false;
     } {}
+
+    assert_eq!((), ());
+    debug_assert_eq!((), ());
+
+    assert_ne!((), ());
+    debug_assert_ne!((), ());
 }
diff --git a/tests/ui/unit_cmp.stderr b/tests/ui/unit_cmp.stderr
index 56293403043..e2bab3eab60 100644
--- a/tests/ui/unit_cmp.stderr
+++ b/tests/ui/unit_cmp.stderr
@@ -22,5 +22,37 @@ LL | |         false;
 LL | |     } {}
    | |_____^
 
-error: aborting due to 2 previous errors
+error: assert_eq of unit values detected. This will always succeed
+  --> $DIR/unit_cmp.rs:24:5
+   |
+LL |     assert_eq!((), ());
+   |     ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: debug_assert_eq of unit values detected. This will always succeed
+  --> $DIR/unit_cmp.rs:25:5
+   |
+LL |     debug_assert_eq!((), ());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: assert_ne of unit values detected. This will always fail
+  --> $DIR/unit_cmp.rs:27:5
+   |
+LL |     assert_ne!((), ());
+   |     ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: debug_assert_ne of unit values detected. This will always fail
+  --> $DIR/unit_cmp.rs:28:5
+   |
+LL |     debug_assert_ne!((), ());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: aborting due to 6 previous errors
 
diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed
index 17c1a5de597..3f63624720f 100644
--- a/tests/ui/unused_unit.fixed
+++ b/tests/ui/unused_unit.fixed
@@ -34,6 +34,7 @@ fn return_unit()  {  }
 
 #[allow(clippy::needless_return)]
 #[allow(clippy::never_loop)]
+#[allow(clippy::unit_cmp)]
 fn main() {
     let u = Unitter;
     assert_eq!(u.get_unit(|| {}, return_unit), u.into());
diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs
index e04c5257337..8fc072ebd69 100644
--- a/tests/ui/unused_unit.rs
+++ b/tests/ui/unused_unit.rs
@@ -35,6 +35,7 @@ fn return_unit() -> () { () }
 
 #[allow(clippy::needless_return)]
 #[allow(clippy::never_loop)]
+#[allow(clippy::unit_cmp)]
 fn main() {
     let u = Unitter;
     assert_eq!(u.get_unit(|| {}, return_unit), u.into());
diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr
index 6ef6dc4f5d6..c489b13bf27 100644
--- a/tests/ui/unused_unit.stderr
+++ b/tests/ui/unused_unit.stderr
@@ -37,13 +37,13 @@ LL | fn return_unit() -> () { () }
    |                          ^^ help: remove the final `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:43:14
+  --> $DIR/unused_unit.rs:44:14
    |
 LL |         break();
    |              ^^ help: remove the `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:45:11
+  --> $DIR/unused_unit.rs:46:11
    |
 LL |     return();
    |           ^^ help: remove the `()`