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 1/6] 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 `()`
From 288f02da4439602d831c62082728ced10fdfe5bd Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Thu, 3 Oct 2019 11:43:39 +0200
Subject: [PATCH 2/6] Print assert macro name in backticks
Co-Authored-By: Philipp Krones <hello@philkrones.com>
---
clippy_lints/src/types.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index b235b3eab2a..a42a50e553a 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -544,7 +544,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitCmp {
UNIT_CMP,
expr.span,
&format!(
- "{} of unit values detected. This will always {}",
+ "`{}` of unit values detected. This will always {}",
symbol.as_str(),
result
),
From 320d17aa63f1516041beb9d42588dfd0bde90136 Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Thu, 3 Oct 2019 12:01:02 +0200
Subject: [PATCH 3/6] Update the .stderr to include the backticks
---
tests/ui/unit_cmp.stderr | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/ui/unit_cmp.stderr b/tests/ui/unit_cmp.stderr
index e2bab3eab60..eeeef05d002 100644
--- a/tests/ui/unit_cmp.stderr
+++ b/tests/ui/unit_cmp.stderr
@@ -22,7 +22,7 @@ LL | | false;
LL | | } {}
| |_____^
-error: assert_eq of unit values detected. This will always succeed
+error: `assert_eq` of unit values detected. This will always succeed
--> $DIR/unit_cmp.rs:24:5
|
LL | assert_eq!((), ());
@@ -30,7 +30,7 @@ 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
+error: `debug_assert_eq` of unit values detected. This will always succeed
--> $DIR/unit_cmp.rs:25:5
|
LL | debug_assert_eq!((), ());
@@ -38,7 +38,7 @@ 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
+error: `assert_ne` of unit values detected. This will always fail
--> $DIR/unit_cmp.rs:27:5
|
LL | assert_ne!((), ());
@@ -46,7 +46,7 @@ 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
+error: `debug_assert_ne` of unit values detected. This will always fail
--> $DIR/unit_cmp.rs:28:5
|
LL | debug_assert_ne!((), ());
From fb25d5679949e26621f43cfbbd7f5864a60e2bd9 Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Thu, 3 Oct 2019 14:35:05 +0200
Subject: [PATCH 4/6] Mention asserts in doc for unit_cmp lint
---
clippy_lints/src/types.rs | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index a42a50e553a..2b3c02ce74e 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -487,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
@@ -519,6 +520,20 @@ declare_clippy_lint! {
/// baz();
/// }
/// ```
+ ///
+ /// For asserts:
+ /// ```rust
+ /// # fn foo() {};
+ /// # fn bar() {};
+ /// assert_eq!({ foo(); }, { bar(); });
+ /// ```
+ /// will always succeed
+ /// ```rust
+ /// # fn foo() {};
+ /// # fn bar() {};
+ /// assert_ne!({ foo(); }, { bar(); });
+ /// ```
+ /// will always fail
pub UNIT_CMP,
correctness,
"comparing unit values"
From 024dfee33c89297ef862fefdf8e1db78574c5b5c Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Thu, 3 Oct 2019 14:38:04 +0200
Subject: [PATCH 5/6] Update unit_cmp tests to include blocks for asserts
---
tests/ui/unit_cmp.rs | 36 +++++++++++++++++++++++++++----
tests/ui/unit_cmp.stderr | 46 ++++++++++++++++++++++++++++++----------
2 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/tests/ui/unit_cmp.rs b/tests/ui/unit_cmp.rs
index 71c4348a2a1..8d3a4eed82e 100644
--- a/tests/ui/unit_cmp.rs
+++ b/tests/ui/unit_cmp.rs
@@ -21,9 +21,37 @@ fn main() {
false;
} {}
- assert_eq!((), ());
- debug_assert_eq!((), ());
+ assert_eq!(
+ {
+ true;
+ },
+ {
+ false;
+ }
+ );
+ debug_assert_eq!(
+ {
+ true;
+ },
+ {
+ false;
+ }
+ );
- assert_ne!((), ());
- debug_assert_ne!((), ());
+ assert_ne!(
+ {
+ true;
+ },
+ {
+ false;
+ }
+ );
+ debug_assert_ne!(
+ {
+ true;
+ },
+ {
+ false;
+ }
+ );
}
diff --git a/tests/ui/unit_cmp.stderr b/tests/ui/unit_cmp.stderr
index eeeef05d002..578a6218ee0 100644
--- a/tests/ui/unit_cmp.stderr
+++ b/tests/ui/unit_cmp.stderr
@@ -25,32 +25,56 @@ LL | | } {}
error: `assert_eq` of unit values detected. This will always succeed
--> $DIR/unit_cmp.rs:24:5
|
-LL | assert_eq!((), ());
- | ^^^^^^^^^^^^^^^^^^^
+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:25:5
+ --> $DIR/unit_cmp.rs:32:5
|
-LL | debug_assert_eq!((), ());
- | ^^^^^^^^^^^^^^^^^^^^^^^^^
+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:27:5
+ --> $DIR/unit_cmp.rs:41:5
|
-LL | assert_ne!((), ());
- | ^^^^^^^^^^^^^^^^^^^
+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:28:5
+ --> $DIR/unit_cmp.rs:49:5
|
-LL | debug_assert_ne!((), ());
- | ^^^^^^^^^^^^^^^^^^^^^^^^^
+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)
From 5a0a2b383c71931bcb4a0431ead16e075c057b0a Mon Sep 17 00:00:00 2001
From: Matthias Seiffert <matthias-seiffert@hotmail.de>
Date: Thu, 3 Oct 2019 19:53:41 +0200
Subject: [PATCH 6/6] Remove assert_ne example from doc
---
clippy_lints/src/types.rs | 6 ------
1 file changed, 6 deletions(-)
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 2b3c02ce74e..2e960952e69 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -528,12 +528,6 @@ declare_clippy_lint! {
/// assert_eq!({ foo(); }, { bar(); });
/// ```
/// will always succeed
- /// ```rust
- /// # fn foo() {};
- /// # fn bar() {};
- /// assert_ne!({ foo(); }, { bar(); });
- /// ```
- /// will always fail
pub UNIT_CMP,
correctness,
"comparing unit values"