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

Do not lint use_self in proc macro expansion #9454

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Conversation

kraktus
Copy link
Contributor

@kraktus kraktus commented Sep 10, 2022

fix #9440
fix #8910
fix #6902

changelog: [use_self]: Do not lint in proc macro expansion

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 10, 2022
@@ -103,6 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
if parameters.as_ref().map_or(true, |params| {
!params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
});
if !is_from_proc_macro(cx, item); // expensive, should be last check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would item_path.span.from_expansion() be enough here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried testing it, and from_expansion still produces the false positive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, what about for item.span?

Copy link
Contributor

@Jarcho Jarcho Sep 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is the proc macro using a real span instead of making a new one. Basically every lint can be triggered from a proc macro, but with the span of the item being linted coming from a token outside the proc macro.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path span happened to do that but the whole item doesn't, so checking its span should be fine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends what span the proc macro sets. If it doesn't set a span with expansion info, but just uses the span given from the item/tokens it looks to clippy as if it is unexpanded code.

@flip1995
Copy link
Member

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Sep 13, 2022

📌 Commit 59ee6a8 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 13, 2022

⌛ Testing commit 59ee6a8 with merge 2e55b42...

@bors
Copy link
Contributor

bors commented Sep 13, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 2e55b42 to master...

@bors bors merged commit 2e55b42 into rust-lang:master Sep 13, 2022
@mrnossiom
Copy link
Contributor

In which version will this patch be released ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
7 participants