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

Possible problems involving clippy::shadow_reuse and auto-generated code #9757

Closed
c410-f3r opened this issue Oct 31, 2022 · 7 comments · Fixed by #10697
Closed

Possible problems involving clippy::shadow_reuse and auto-generated code #9757

c410-f3r opened this issue Oct 31, 2022 · 7 comments · Fixed by #10697
Labels
good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented Oct 31, 2022

Description

According to dtolnay/quote#232 (comment), clippy::shadow_reuse shouldn't be triggered but I am personally not aware of this intended limitation. Any thoughts?

Version

Latest 1.65 beta

Additional Labels

No response

@c410-f3r c410-f3r changed the title Possible problems with clippy::shadow_reuse and auto-generated code Possible problems involving clippy::shadow_reuse and auto-generated code Oct 31, 2022
@Alexendoo Alexendoo added I-false-positive Issue: The lint was triggered on code it shouldn't have good-first-issue These issues are a good way to get started with Clippy labels Oct 31, 2022
@Alexendoo
Copy link
Member

Alexendoo commented Oct 31, 2022

A fix for this case would be to check pat.span.from_expansion() in addition to the current ident.span.from_expansion() check

It wouldn't be intended for this to lint, macro span interactions are just non obvious

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Dec 2, 2022

Closing due to the lack of activity.

@c410-f3r c410-f3r closed this as completed Dec 2, 2022
@Alexendoo
Copy link
Member

The issue isn't fixed

@Alexendoo Alexendoo reopened this Dec 2, 2022
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Dec 2, 2022

I am house-keeping my dashboard closing inactive issues that will probably take some time to resolve.

For this use-case specifically, I guess I can try to solve the problem.

@lochetti
Copy link
Contributor

I am trying to take a look at this issue. Although, this would be my first contribution here.

Not linting the code if pat.span.from_expansion() does solve the specific issue mentioned for the quote crate, but I have one doubt: Which is the best way to increment the tests in order to add this use case? I am really not sure how to do that.

Thanks for the patience in helping a new contributor wannabe!

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Apr 22, 2023

To solve the issue, I think shadow_reuse should also check clippy_utils::is_from_proc_macro along side the already included ident.span.from_expansion(). See #10203 (comment).

In regards to test addition or how to test this issue specifically, I really have no clue because https://github.com/rust-lang/rust-clippy/pull/10203/files#diff-98b6b061d5b311aae09fd482c612aab5544d55b9c0da67438d79ef0fe181d526R129-R159 didn't help.

Perhaps one of the Clippy members can help you? @Alexendoo @Jarcho @xFrednet

EDIT: Oops, if pat.span.from_expansion() indeed solved the problem then forget about my clippy_utils::is_from_proc_macro suggestion.

@Alexendoo
Copy link
Member

You can add a test along the lines of

macro_rules! reuse {
    ($v:ident) => {
        let $v = $v + 1;
    }
}

let x = 1;
reuse!(x);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy 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.

3 participants