From 7b3847fad772fd52da1f91b791d449d0a7cdcf19 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Tue, 5 Oct 2021 12:59:24 -0700
Subject: [PATCH] attributes: remove unnecessary quote_spanned!  (#1617)

## Motivation

Apparently, using `quote_spanned!` can trigger a Clippy bug where the
text `else`, even inside a comment, _may_ cause the
`suspicious_else_formatting` lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the `#[instrument]` attribute is
used on `async fn`s. See issue #1613 for details.

## Solution

It turns out that some of the uses of `quote_spanned!` in the
`tracing-attributes` code generation are not needed. We really only need
`quote_spanned!` when actually interpolating the user provided code into
a block, not in the `tracing-attributes` code that inserts the generated
code for producing the span etc. Replacing some of these
`quote_spanned!` uses with the normal `quote!` macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.

I've added a few test cases that should reproduce the bug.

Fixes #1613

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-attributes/src/lib.rs        |  6 +++---
 tracing-attributes/tests/async_fn.rs | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs
index e3a706c21a..3b220522d9 100644
--- a/tracing-attributes/src/lib.rs
+++ b/tracing-attributes/src/lib.rs
@@ -730,7 +730,7 @@ fn gen_block(
             )
         };
 
-        return quote_spanned!(block.span()=>
+        return quote!(
             let __tracing_attr_span = #span;
             let __tracing_instrument_future = #mk_fut;
             if !__tracing_attr_span.is_disabled() {
@@ -745,7 +745,7 @@ fn gen_block(
         );
     }
 
-    let span = quote_spanned!(block.span()=>
+    let span = quote!(
         // These variables are left uninitialized and initialized only
         // if the tracing level is statically enabled at this point.
         // While the tracing level is also checked at span creation
@@ -779,7 +779,7 @@ fn gen_block(
         );
     }
 
-    quote_spanned!(block.span()=>
+    quote_spanned!(block.span() =>
         // 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.
diff --git a/tracing-attributes/tests/async_fn.rs b/tracing-attributes/tests/async_fn.rs
index 6830cf6239..1f5e3d0bf6 100644
--- a/tracing-attributes/tests/async_fn.rs
+++ b/tracing-attributes/tests/async_fn.rs
@@ -33,6 +33,28 @@ async fn test_ret_impl_trait_err(n: i32) -> Result<impl Iterator<Item = i32>, &'
 #[instrument]
 async fn test_async_fn_empty() {}
 
+// Reproduces https://github.com/tokio-rs/tracing/issues/1613
+#[instrument]
+// LOAD-BEARING `#[rustfmt::skip]`! This is necessary to reproduce the bug;
+// with the rustfmt-generated formatting, the lint will not be triggered!
+#[rustfmt::skip]
+#[deny(clippy::suspicious_else_formatting)]
+async fn repro_1613(var: bool) {
+    println!(
+        "{}",
+        if var { "true" } else { "false" }
+    );
+}
+
+// Reproduces https://github.com/tokio-rs/tracing/issues/1613
+// and https://github.com/rust-lang/rust-clippy/issues/7760
+#[instrument]
+#[deny(clippy::suspicious_else_formatting)]
+async fn repro_1613_2() {
+    // hello world
+    // else
+}
+
 #[test]
 fn async_fn_only_enters_for_polls() {
     let (subscriber, handle) = subscriber::mock()