From aecf7f42174a6feeb6e2080c27e7f1975ba0caca Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Mon, 4 Oct 2021 09:28:18 -0700
Subject: [PATCH] attributes: suppress `clippy::suspicious_else` without nop
 let

Currently, `tracing-attributes` generates a `let _ = ();` in between the
`if tracing::level_enabled!(...) {}` and the function body. This is
intended to suppress the `clippy::suspicious_else_formatting` lint,
which is generated when an `if` is followed immediately by a block with
no whitespace in between. Since we can't add whitespace in the generated
code (as `quote` produces a stream of _rust tokens_, not text), we
can't suppress the lint without adding a no-op statement.

However, unfortunately, the no-op let triggers a _different_ lint
(`clippy::let_unit_value`), when `clippy::pedantic` is enabled. This is
kind of annoying for some users.

This branch changes the code to suppress the
`suspicious_else_formatting` lint using `#[allow(...)]` attributes,
instead. The warning is turned back on inside of user code, since users
probably want the warning.
---
 tracing-attributes/src/lib.rs        | 15 +++++++++++----
 tracing-attributes/tests/async_fn.rs |  3 +++
 tracing-attributes/tests/err.rs      |  6 ++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs
index b3029341c3..f02ec21a3c 100644
--- a/tracing-attributes/src/lib.rs
+++ b/tracing-attributes/src/lib.rs
@@ -563,8 +563,6 @@ fn gen_block(
             __tracing_attr_span = #span;
             __tracing_attr_guard = __tracing_attr_span.enter();
         }
-        // pacify clippy::suspicious_else_formatting
-        let _ = ();
     );
 
     if err {
@@ -583,8 +581,17 @@ fn gen_block(
     }
 
     quote_spanned!(block.span()=>
-        #span
-        #block
+        // Because `quote` produces a stream of tokens _without_ whitespace, the
+        // `if` and the block will appear directly next to each other. This
+        // generates a clippy lint about suspicious `if/else` formatting.
+        // Therefore, suppress the lint inside the generated code...
+        #[allow(clippy::suspicious_else_formatting)]
+        {
+            #span
+            // ...but turn the lint back on inside the function body.
+            #[warn(clippy::suspicious_else_formatting)]
+            #block
+        }
     )
 }
 
diff --git a/tracing-attributes/tests/async_fn.rs b/tracing-attributes/tests/async_fn.rs
index 816795946e..e7627fbbb2 100644
--- a/tracing-attributes/tests/async_fn.rs
+++ b/tracing-attributes/tests/async_fn.rs
@@ -15,6 +15,9 @@ async fn test_async_fn(polls: usize) -> Result<(), ()> {
     future.await
 }
 
+#[instrument]
+async fn test_async_fn_empty() {}
+
 #[test]
 fn async_fn_only_enters_for_polls() {
     let (collector, handle) = collector::mock()
diff --git a/tracing-attributes/tests/err.rs b/tracing-attributes/tests/err.rs
index f78b69518d..808134d101 100644
--- a/tracing-attributes/tests/err.rs
+++ b/tracing-attributes/tests/err.rs
@@ -16,6 +16,12 @@ fn err() -> Result<u8, TryFromIntError> {
     u8::try_from(1234)
 }
 
+#[instrument(err)]
+fn err_suspicious_else() -> Result<u8, TryFromIntError> {
+    {}
+    u8::try_from(1234)
+}
+
 #[test]
 fn test() {
     let span = span::mock().named("err");