Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn suspicious_else_formatting when a function using "tracing crate" has a comment containing "else". #7760

Closed
hotate29 opened this issue Oct 4, 2021 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@hotate29
Copy link
Contributor

hotate29 commented Oct 4, 2021

Lint name: suspicious_else_formatting

I tried this code:

use tracing::instrument;

#[instrument]
async fn func1() {
    // a
    // else
}

#[instrument]
async fn func2() {
    ()
    // else
}

#[instrument]
async fn func3() {
    let _ = ();
    // else
}

I expected to see this happen: No warning

Instead, this happened:

warning: this is an `else {..}` but the formatting might hide it
 --> src\lib.rs:4:17
  |
4 |   async fn func1() {
  |  _________________^
5 | |     // a;
6 | |     // else
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

Same warning for func1 to func3.

With version 0.1.28 of tracing.

Meta

Rust version (rustc -Vv):

rustc 1.55.0 (c8dfcfe04 2021-09-06)
binary: rustc
commit-hash: c8dfcfe046a7680554bf4eb612bad840e7631c4b
commit-date: 2021-09-06
host: x86_64-pc-windows-msvc
release: 1.55.0
LLVM version: 12.0.1
@hotate29 hotate29 added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Oct 4, 2021
@hotate29
Copy link
Contributor Author

hotate29 commented Oct 4, 2021

May be the same as this
#6249 (comment)

hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
## 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
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
## 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>
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
## 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>
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
## 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>
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
# 0.1.17 (October 1, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
# 0.1.17 (October 1, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
hawkw added a commit to tokio-rs/tracing that referenced this issue Oct 5, 2021
# 0.1.18 (October 5, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
@giraffate
Copy link
Contributor

It seemed to be addressed by tracing at tokio-rs/tracing#1617, and released in new version tokio-rs/tracing#1621.

@hotate29
Copy link
Contributor Author

hotate29 commented Oct 6, 2021

Thank you for telling me! Then I think I will close this issue as it is not needed.

@hotate29 hotate29 closed this as completed Oct 6, 2021
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
# 0.1.18 (October 5, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([tokio-rs#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([tokio-rs#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([tokio-rs#1614])

[tokio-rs#1617]: tokio-rs#1617
[tokio-rs#1616]: tokio-rs#1616
[tokio-rs#1614]: tokio-rs#1614
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

2 participants