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.

Sign up for GitHub

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

Jump to bottom

attributes: suppress clippy::suspicious_else without nop let #1614

Merged
merged 1 commit into from
Oct 4, 2021
Merged

attributes: suppress clippy::suspicious_else without nop let #1614

merged 1 commit into from
Oct 4, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 4, 2021

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.

@hawkw
attributes: suppress clippy::suspicious_else without nop let
aecf7f4 
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.
@hawkw hawkw requested review from davidbarsky and a team as code owners October 4, 2021 16:33
@hawkw hawkw requested a review from oli-obk October 4, 2021 16:33
@hawkw hawkw self-assigned this Oct 4, 2021
@hawkw hawkw merged commit fb45ba9 into master Oct 4, 2021
@hawkw hawkw deleted the eliza/suspicious-else-2 branch October 4, 2021 18:35
@hawkw hawkw mentioned this pull request Oct 4, 2021
Closed
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
attributes: suppress clippy::suspicious_else without nop let (#1614)
d309e7e 
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.
@hawkw hawkw mentioned this pull request Oct 5, 2021
Merged
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
attributes: suppress clippy::suspicious_else without nop let (#1614)
ac74ba0 
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.
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
attributes: prepare to release 0.1.18
9c402e4 
# 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 hawkw mentioned this pull request Oct 5, 2021
Merged
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
attributes: prepare to release 0.1.18
480bede 
# 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 that referenced this pull request Oct 5, 2021
@hawkw
attributes: prepare to release 0.1.18 (#1621)
645f282 
# 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
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
tracing: prepare to release v0.1.29
aaeb074 
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
@hawkw hawkw mentioned this pull request Oct 5, 2021
Merged
hawkw added a commit that referenced this pull request Oct 5, 2021
@hawkw
tracing: prepare to release v0.1.29
854d6c9 
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Oct 6, 2021
@hawkw
tracing: prepare to release v0.1.29 (#1623)
d0ef14d 
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1605]: #1605
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
@str4d str4d mentioned this pull request Jan 9, 2023
Open
str4d added a commit to str4d/tracing that referenced this pull request Jan 9, 2023
@str4d
attributes: Apply fake return edge only to async fns
67f25c5 
The fake return edge was added in tokio-rs#2270 to improve type
errors in instrumented async functions. However, it meant that the user
block was being modified outside of `gen_block`. This created a negative
interaction with tokio-rs#1614, which suppressed a clippy lint
internally while explicitly enabling it on the user block. The installed
fake return edge generated this same lint, but the user had no way to
suppress it: lint directives above the instrumentation were ignored
because of the internal suppression, and lints inside the user block
could not influence the fake return edge's scope.

We now avoid modifying the user block outside of `gen_block`, and
restrict the fake return edge to async functions. We also apply the same
clippy lint suppression technique to the installed fake return edge.

Closes tokio-rs#2410.
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
attributes: prepare to release 0.1.18 (tokio-rs#1621)
e677d14 
# 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
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
tracing: prepare to release v0.1.29 (tokio-rs#1623)
dfe416a 
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([tokio-rs#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([tokio-rs#1600], [tokio-rs#1605],
  [tokio-rs#1614], [tokio-rs#1616], [tokio-rs#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([tokio-rs#1602])
- Documentation fixes and improvements ([tokio-rs#1595], [tokio-rs#1601], [tokio-rs#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[tokio-rs#1585]: tokio-rs#1585
[tokio-rs#1595]: tokio-rs#1596
[tokio-rs#1597]: tokio-rs#1597
[tokio-rs#1600]: tokio-rs#1600
[tokio-rs#1601]: tokio-rs#1601
[tokio-rs#1602]: tokio-rs#1602
[tokio-rs#1605]: tokio-rs#1605
[tokio-rs#1614]: tokio-rs#1614
[tokio-rs#1616]: tokio-rs#1616
[tokio-rs#1617]: tokio-rs#1617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

@oli-obk oli-obk Awaiting requested review from oli-obk

Assignees

@hawkw hawkw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hawkw @davidbarsky