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

no_effect_underscore_binding: FP on field inside derive macros #12045

Closed
xFrednet opened this issue Dec 29, 2023 · 3 comments · Fixed by #12125
Closed

no_effect_underscore_binding: FP on field inside derive macros #12045

xFrednet opened this issue Dec 29, 2023 · 3 comments · Fixed by #12125
Assignees
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

@xFrednet
Copy link
Member

xFrednet commented Dec 29, 2023

Summary

clippy::no_effect_underscore_binding can trigger on fields starting with an underscore inside proc-macros. I looked at the code and it seems like the lint only checks

Lint Name

no_effect_underscore_binding

Reproducer

I tried this code:

#![warn(clippy::no_effect_underscore_binding)]

use core::marker::PhantomData;

#[derive(typed_builder::TypedBuilder)]
pub(crate) struct CommonTyData<'ast> {
    #[builder(default)]
    _lifetime: PhantomData<&'ast ()>,
}

This example code, sadly, uses a dependency:

typed-builder      = "0.18"

I saw this happen:

warning: binding to `_` prefixed variable with no side-effect
  --> marker_api/src/ast/stmt.rs:62:43
   |
62 | #[derive(typed_builder::TypedBuilder)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding

I expected to see this happen:

Nothing

Version

rustc 1.77.0-nightly (89e2160c4 2023-12-27)
binary: rustc
commit-hash: 89e2160c4ca5808657ed55392620ed1dbbce78d1
commit-date: 2023-12-27
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6

Additional Labels

See:

@xFrednet xFrednet 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 Dec 29, 2023
@cocodery
Copy link
Contributor

cocodery commented Jan 2, 2024

I try same code in same environment but nothing happen.
Does this issue fixed by other commit?

@xFrednet
Copy link
Member Author

xFrednet commented Jan 3, 2024

The code example was not quite correct, as it included a cfg_attr and didn't enable the lint. I've updated the example and the lint still triggers:

warning: binding to `_` prefixed variable with no side-effect
 --> src/main.rs:5:10
  |
5 | #[derive(typed_builder::TypedBuilder)]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::no_effect_underscore_binding)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: this warning originates in the derive macro `typed_builder::TypedBuilder` (in Nightly builds, run with -Z macro-backtrace for more info)

@cocodery
Copy link
Contributor

@rustbot claim

bors added a commit that referenced this issue Jan 11, 2024
Fix error warning span for issue12045

fixes [Issue#12045](#12045)

In issue#12045, unexpected warning span occurs on attribute `#[derive(typed_builder::TypedBuilder)]`, actually the warning should underline `_lifetime`.

In the source code we can find that the original intend is to warning on `ident.span`, but in this case, `stmt.span` is unequal with `ident.span`. So, fix the nit here is fine.

Besides, `ident.span` have an accurate range than `stmt.span`.

changelog: [`no_effect_underscore_binding`]: correct warning span
bors added a commit that referenced this issue Jan 19, 2024
Fix error warning span for issue12045

fixes [Issue#12045](#12045)

In issue#12045, unexpected warning span occurs on attribute `#[derive(typed_builder::TypedBuilder)]`, actually the warning should underline `_lifetime`.

In the source code we can find that the original intend is to warning on `ident.span`, but in this case, `stmt.span` is unequal with `ident.span`. So, fix the nit here is fine.

Besides, `ident.span` have an accurate range than `stmt.span`.

changelog: [`no_effect_underscore_binding`]: correct warning span
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

Successfully merging a pull request may close this issue.

2 participants