`tracing::instrument` triggers clippy warning

[malobre]

Bug Report

Version

├── tracing v0.1.28
└── tracing-subscriber v0.2.24
    ├── tracing-core v0.1.21
    └── tracing-serde v0.1.2
        └── tracing-core v0.1.21

Platform

Linux 5.14.8-arch1-1 #1 SMP PREEMPT Sun, 26 Sep 2021 19:36:15 +0000 x86_64 GNU/Linux

Description

This code:

#[tracing::instrument]
pub async fn print_bool(var: bool) {
    println!(
        "{}",
        if var { "true" } else { "false" }
    );
}

triggers a clippy warning:

warning: this is an `else {..}` but the formatting might hide it
 --> src/lib.rs:2:36
  |
2 |   pub async fn print_bool(var: bool) {
  |  ____________________________________^
3 | |     println!(
4 | |         "{}",
5 | |         if var { "true" } else { "false" }
6 | |     );
7 | | }
  | |_^
  |
  = note: `#[warn(clippy::suspicious_else_formatting)]` on by default
  = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

Playground

[hawkw]

Thanks, can you also provide the version of the tracing-attributes crate? The instrument macro is provided by tracing-attributes and re-exported by tracing.

[hawkw]

Hmm, interesting --- I can't seem to reproduce this issue with the exact code you've provided. If I create a new crate and paste in your code:

lib.rs:

#[tracing::instrument]
pub async fn print_bool(var: bool) {
    println!("{}", if var { "true" } else { "false" });
}

Cargo.toml:

[package]
name = "clippy-repro"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tracing = "0.1.28"

cargo clippy doesn't return any warnings:

:; cargo clippy
   Compiling proc-macro2 v1.0.29
   Compiling unicode-xid v0.2.2
   Compiling syn v1.0.78
    Checking lazy_static v1.4.0
    Checking pin-project-lite v0.2.7
    Checking cfg-if v1.0.0
    Checking tracing-core v0.1.21
   Compiling quote v1.0.9
   Compiling tracing-attributes v0.1.17
    Checking tracing v0.1.28
    Checking clippy-repro v0.1.0 (/home/eliza/Code/clippy-repro)
    Finished dev [unoptimized + debuginfo] target(s) in 5.55s

However, I see that it does produce a warning in the playground. Strange...

[hawkw]

Oh, interesting! It turns out that this is warning is specifically a result of the particular way the code you provided is formatted. When I was trying to reproduce locally, my editor ran rustfmt automatically, and re-formatted the if to be all on one line:

#[tracing::instrument]
pub async fn print_bool(var: bool) {
    println!("{}", if var { "true" } else { "false" });
}

If I change the playground to have that formatting, it doesn't produce the warning!

[malobre]

tracing-attributes v0.1.17, what a weird bug..

[hawkw]

I may have a fix, now that I know how to reproduce it...

[hawkw]

Okay, so #1614, which fixes some instances of this warning, does not fix this. I'm honestly quite unsure why the formatting of the original source effects this at all, since the cargo expand output is identical regardless of how the println is formatted...this is very strange!

[davidpdrsn]

At Embark we hit this as well but also hit clippy::let_unit_value. I'll ask tomorrow if someone has a reproduction.

[davidpdrsn]

Actually making a repro turned out to be quite easy:

#![deny(clippy::let_unit_value)]

#[tracing::instrument(level = "debug")]
fn foo() {}

with

[package]
name = "name"
version = "0.1.0"
edition = "2018"

[dependencies]
tracing = "=0.1.28"

That hits the warning

error: this let-binding has unit value
 --> src/lib.rs:4:10
  |
4 | fn foo() {}
  |          ^^ help: omit the `let` binding: `{};`
  |
note: the lint level is defined here
 --> src/lib.rs:1:9
  |
1 | #![deny(clippy::let_unit_value)]
  |         ^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value

error: could not compile `name` due to previous error

[dcormier]

I was bit by this as well. It looks like caused some additional traffic on rust-lang/rust-clippy#6249, too. I had to lock tracing-attributes to the previous release to continue with my day. For anyone else just trying to get some work done, just put this in the appropriate place in your Cargo.toml for now:

# trancing-attributes is only here because of:
# https://github.com/rust-lang/rust-clippy/issues/6249
# https://github.com/tokio-rs/tracing/issues/1613
# TODO: Remove it when that's fixed.
tracing-attributes = "=0.1.16"

Maybe 0.1.17 should be pulled?

[sehz]

In our case, this triggered compilation error. It worked on 0.1.16 or by removing instrument

   --> crates/fluvio/src/consumer.rs:304:79
    |
304 |       ) -> Result<impl Stream<Item = Result<Record, FluvioError>>, FluvioError> {
    |  _______________________________________________________________________________^
305 | |         let stream = self.stream_batches_with_config(offset, config).await?;
306 | |         let partition = self.partition;
307 | |         let flattened =
...   |
325 | |         Ok(flattened)
326 | |     }
    | |_____^ expected closure, found a different closure
...
358 |       ) -> Result<impl Stream<Item = Result<Batch, FluvioError>>, FluvioError> {
    |                   ----------------------------------------------
    |                   |
    |                   the expected opaque type
    |                   the found opaque type
    |
    = note: expected enum `Result<futures_util::stream::FlatMap<_, Either<futures_util::stream::Iter<std::iter::Map<_, [closure@crates/fluvio/src/consumer.rs:313:25: 319:26]>>, _>, [closure@crates/fluvio/src/consumer.rs:308:29: 323:14]>, FluvioError>`
               found enum `Result<futures_util::stream::FlatMap<_, Either<futures_util::stream::Iter<std::iter::Map<_, [closure@crates/fluvio/src/consumer.rs:313:25: 319:26]>>, _>, [closure@crates/fluvio/src/consumer.rs:308:29: 323:14]>, _>`
    = note: no two closures, even if identical, have the same type
    = help: consider boxing your closure and/or using it as a trait object

[hawkw]

At Embark we hit this as well but also hit clippy::let_unit_value. I'll ask tomorrow if someone has a reproduction.

This is a different, unrelated issue, which is fixed by #1614.

[hawkw]

In our case, this triggered compilation error. It worked on 0.1.16 or by removing instrument

   --> crates/fluvio/src/consumer.rs:304:79
    |
304 |       ) -> Result<impl Stream<Item = Result<Record, FluvioError>>, FluvioError> {
    |  _______________________________________________________________________________^
305 | |         let stream = self.stream_batches_with_config(offset, config).await?;
306 | |         let partition = self.partition;
307 | |         let flattened =
...   |
325 | |         Ok(flattened)
326 | |     }
    | |_____^ expected closure, found a different closure
...
358 |       ) -> Result<impl Stream<Item = Result<Batch, FluvioError>>, FluvioError> {
    |                   ----------------------------------------------
    |                   |
    |                   the expected opaque type
    |                   the found opaque type
    |
    = note: expected enum `Result<futures_util::stream::FlatMap<_, Either<futures_util::stream::Iter<std::iter::Map<_, [closure@crates/fluvio/src/consumer.rs:313:25: 319:26]>>, _>, [closure@crates/fluvio/src/consumer.rs:308:29: 323:14]>, FluvioError>`
               found enum `Result<futures_util::stream::FlatMap<_, Either<futures_util::stream::Iter<std::iter::Map<_, [closure@crates/fluvio/src/consumer.rs:313:25: 319:26]>>, _>, [closure@crates/fluvio/src/consumer.rs:308:29: 323:14]>, _>`
    = note: no two closures, even if identical, have the same type
    = help: consider boxing your closure and/or using it as a trait object

This probably deserves its own issue, as I don't think the compiler error is related to the issue that causes the clippy warning.

[hawkw]

I was bit by this as well. It looks like caused some additional traffic on rust-lang/rust-clippy#6249, too. I had to lock tracing-attributes to the previous release to continue with my day. For anyone else just trying to get some work done, just put this in the appropriate place in your Cargo.toml for now:

# trancing-attributes is only here because of:
# https://github.com/rust-lang/rust-clippy/issues/6249
# https://github.com/tokio-rs/tracing/issues/1613
# TODO: Remove it when that's fixed.
tracing-attributes = "=0.1.16"

Maybe 0.1.17 should be pulled?

I've yanked v0.1.17 to hopefully avoid breaking anyone else's builds. We'll try to put together a new version that still makes the same optimizations added in #1600, without causing build breakage.