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

Turn duplicated_attributes into a late lint #12646

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Turn duplicated_attributes into a late lint #12646

merged 1 commit into from
Apr 9, 2024

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Apr 8, 2024 •
edited
Loading

Fixes #12537.

changelog: Turn duplicated_attributes into a late lint

@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2024

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 8, 2024
@GuillaumeGomez GuillaumeGomez mentioned this pull request Apr 8, 2024
Closed
@GuillaumeGomez
Copy link
Member Author

Easiest fix: turn this lint into a late lint. No more problem with proc macros. I'm frustrated as I was unable to write a case to reproduce what test_case does though...

@GuillaumeGomez GuillaumeGomez force-pushed the regression-test-12537 branch 2 times, most recently from 9f3d980 to 95e77bc Compare April 9, 2024 13:59
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Apr 9, 2024 •
edited
Loading

And fixed dogfood tests. :3

blyxyas
blyxyas approved these changes Apr 9, 2024
View reviewed changes
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ❤️
I've tested this and compared it with master, this branch fixes the issue. Could you update the PR title, the commit message, the changelog message and squash?

@GuillaumeGomez GuillaumeGomez changed the title Add regression test for #12537 Turn duplicated_attributes into a late lint Apr 9, 2024
@GuillaumeGomez GuillaumeGomez force-pushed the regression-test-12537 branch from 95e77bc to f7d49a3 Compare April 9, 2024 14:42
@GuillaumeGomez
Copy link
Member Author

All done. :)

@blyxyas
Copy link
Member

blyxyas commented Apr 9, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2024

📌 Commit f7d49a3 has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 9, 2024

⌛ Testing commit f7d49a3 with merge fc7e73b...

bors added a commit that referenced this pull request Apr 9, 2024
@bors
Auto merge of #12646 - GuillaumeGomez:regression-test-12537, r=blyxyas
fc7e73b 
Turn `duplicated_attributes` into a late lint

Fixes #12537.

changelog: Turn `duplicated_attributes` into a late lint
tamird
tamird reviewed Apr 9, 2024
View reviewed changes
tests/ui/useless_attribute.fixed
@@ -1,6 +1,6 @@
//@aux-build:proc_macro_derive.rs

#![allow(unused)]
#![allow(unused, clippy::duplicated_attributes)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why allow? What's the duplicate here?

Copy link
Member Author

Choose a reason for hiding this comment

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

allow(dead_code).

Copy link
Contributor

Choose a reason for hiding this comment

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

the duplicated attribute was introduced in 81f5969 - it seems better to change the attribute to still test the same thing without piling on suppressions.

tests/ui/useless_attribute.rs
@@ -1,6 +1,6 @@
//@aux-build:proc_macro_derive.rs

#![allow(unused)]
#![allow(unused, clippy::duplicated_attributes)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

allow(dead_code).

@blyxyas
Copy link
Member

blyxyas commented Apr 9, 2024

@bors r-

@bors
Copy link
Contributor

bors commented Apr 9, 2024

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: fc7e73b (fc7e73be690bc6901dde0bb2cc87a070e7011f67)

@GuillaumeGomez
Copy link
Member Author

Why the r-?

@blyxyas
Copy link
Member

blyxyas commented Apr 9, 2024

I just wanted to check the review comment from @tamird and got a little bit scared of a possible merging. I've rechecked, everything's correct. False alarm

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2024

📌 Commit f7d49a3 has been approved by blyxyas

It is now in the queue for this repository.

@GuillaumeGomez
Copy link
Member Author

That was fast. :o

@bors
Copy link
Contributor

bors commented Apr 9, 2024

⌛ Testing commit f7d49a3 with merge 62fd1d5...

@bors
Copy link
Contributor

bors commented Apr 9, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 62fd1d5 to master...

@bors bors merged commit 62fd1d5 into rust-lang:master Apr 9, 2024
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the regression-test-12537 branch April 9, 2024 15:18
@bors bors mentioned this pull request Apr 9, 2024
Merged
@dtolnay dtolnay mentioned this pull request Dec 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tamird tamird tamird left review comments

@blyxyas blyxyas blyxyas approved these changes

Assignees

@blyxyas blyxyas

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

duplicated_attributes false positive: cfg expressions
5 participants
@GuillaumeGomez @rustbot @blyxyas @bors @tamird