tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.19k stars 677 forks source link

instrument triggers clippy::blocks_in_conditions #2876

Open ShahakShama opened 5 months ago

ShahakShama commented 5 months ago

Bug Report

Version

tracing v0.1.40 tracing-attributes v0.1.27 (proc-macro) tracing-core v0.1.32 tracing-futures v0.2.5 tracing-log v0.2.0 tracing-subscriber v0.3.18

Platform

22.04.1-Ubuntu

Description

Running the following code with clippy version 1.76.0 (2024-02-04) caused an error

#[async_trait]
pub trait Bar {
    async fn foo(&self) -> Result<usize, usize>;
}

#[derive(Debug)]
struct Foo;

#[async_trait]
impl Bar for Foo {
    #[instrument(skip(self), level = "debug", err)]
    async fn foo(&self) -> Result<usize, usize> {
        Ok(8)
    }
}

The error is

error: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
    --> ...:1515:49
     |
1515 |       async fn foo(&self) -> Result<usize, usize> {
     |  _________________________________________________^
1516 | |         Ok(8)
1517 | |     }
     | |_____^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions

This only happens with async traits. The following caused no errors:

#[instrument(skip(bar), level = "debug", err)]
async fn foo1(bar: usize) -> Result<usize, usize> {
    Ok(bar)
}

struct Foo2;

impl Foo2 {
    #[instrument(skip(self), level = "debug", err)]
    async fn foo2(&self) -> Result<usize, usize> {
        Ok(8)
    }
}

pub trait Bar3 {
    fn foo3(&self) -> Result<usize, usize>;
}

#[derive(Debug)]
struct Foo3;

impl Bar3 for Foo3 {
    #[instrument(skip(self), level = "debug", err)]
    fn foo3(&self) -> Result<usize, usize> {
        Ok(8)
    }
}
seanpianka commented 4 months ago

2880 is merged. Once released, it should close this issue.

davidbarsky commented 4 months ago

2880 is merged. Once released, it should close this issue.

We'd need to port it to master as well, yes?

EDIT: yes, we will. 2880 only targeted v0.1.x.