Auto merge of #4613 - Lythenas:lint-assert_eq-unit_exprs, r=flip1995

Add check for assert_eq macros to unit_cmp lint

changelog: Add check for unit comparisons through `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` macros to unit_cmp lint.

fixes #4481

Author: bors

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};
 
@@ -485,7 +487,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnitValue {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for comparisons to unit.
+    /// **What it does:** Checks for comparisons to unit. This includes all binary
+    /// comparisons (like `==` and `<`) and asserts.
     ///
     /// **Why is this bad?** Unit is always equal to itself, and thus is just a
     /// clumsily written constant. Mostly this happens when someone accidentally
@@ -517,6 +520,14 @@ declare_clippy_lint! {
     ///     baz();
     /// }
     /// ```
+    ///
+    /// For asserts:
+    /// ```rust
+    /// # fn foo() {};
+    /// # fn bar() {};
+    /// assert_eq!({ foo(); }, { bar(); });
+    /// ```
+    /// will always succeed
     pub UNIT_CMP,
     correctness,
     "comparing unit values"
@@ -527,6 +538,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 {

tests/ui/unit_cmp.rs

@@ -20,4 +20,38 @@ fn main() {
     } > {
         false;
     } {}
+
+    assert_eq!(
+        {
+            true;
+        },
+        {
+            false;
+        }
+    );
+    debug_assert_eq!(
+        {
+            true;
+        },
+        {
+            false;
+        }
+    );
+
+    assert_ne!(
+        {
+            true;
+        },
+        {
+            false;
+        }
+    );
+    debug_assert_ne!(
+        {
+            true;
+        },
+        {
+            false;
+        }
+    );
 }

tests/ui/unit_cmp.stderr

@@ -22,5 +22,61 @@ 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!(
+LL | |         {
+LL | |             true;
+LL | |         },
+...  |
+LL | |         }
+LL | |     );
+   | |______^
+   |
+   = 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:32:5
+   |
+LL | /     debug_assert_eq!(
+LL | |         {
+LL | |             true;
+LL | |         },
+...  |
+LL | |         }
+LL | |     );
+   | |______^
+   |
+   = 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:41:5
+   |
+LL | /     assert_ne!(
+LL | |         {
+LL | |             true;
+LL | |         },
+...  |
+LL | |         }
+LL | |     );
+   | |______^
+   |
+   = 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:49:5
+   |
+LL | /     debug_assert_ne!(
+LL | |         {
+LL | |             true;
+LL | |         },
+...  |
+LL | |         }
+LL | |     );
+   | |______^
+   |
+   = 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
 

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());

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());

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 `()`