Skip to content

Commit

Permalink
attributes: suppress clippy::suspicious_else without nop let (#1614)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hawkw committed Oct 4, 2021
1 parent 0fa74b9 commit fb45ba9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
15 changes: 11 additions & 4 deletions tracing-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
)
}

Expand Down
3 changes: 3 additions & 0 deletions tracing-attributes/tests/async_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 6 additions & 0 deletions tracing-attributes/tests/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit fb45ba9

Please sign in to comment.