don't lint [`mixed_attributes_style`] when mixing docs and other attrs by J-Zhen...
source link: https://github.com/rust-lang/rust-clippy/pull/12486
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
don't lint [mixed_attributes_style
] when mixing docs and other attrs
#12486
Conversation
added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label
Member
Author
I know |
Instead of changing the lint pass and having to handle new cases, why not handle doc comments separately from the other attributes? Should be simpler, no? |
Oh I see, |
Member
I wasn't sure if a post-expansion early pass wouldn't just run into the same issue. I could be wrong on this, but my understanding of the issue here is that outlined
I'm not sure. One "downside" I suppose is that post-expansion lints don't see the disabled |
I'm surprised that modules would get inlined in the AST pass. Would be worth checking it. |
I do think we should avoid linting on differing attributes, but I would also be fine with just landing the PR with allowing doc comments for now if that makes it easier (like it does rn), just so the lint level attribute bug is fixed and people who disagree with the lint can at least allow it properly
I also tested it locally and yes, a post-expansion early lint pass also seems to have the same issue (inner attributes of the other file being part of the |
Member
Author
Make sense, I'll open another issue addressing the other problem |
Member
Author
After experimenting on this a little bit, I ended up changes almost everything in the src file, might as well try to fix it... The only problem was it still has some false-negative afaik. One of them being it's unable to detect the problem inside a nested module inside of test module, idk if it's by design or not, since is very specific. (If it's unexpected, I can But hey, having FN is better than having FP right?! |
Contributor
Please prefer false negatives over false positives, especially for a novel lint! |
changes make sense to me, 👍 for also only linting on same attributes
just some minor comments and I think it's ready
Member
Inner proc macro attributes are unstable anyway so I would say that false negative is fine |
Looks good to me, just some doc nits I found
Can you also squash the commits?
Member
Author
@y21 oops, mess up the commit message, should've put the last line first... but anyway, it should be good now, thank you for all the patience~ |
Member
yep, looks good, thank you for fixing these issues! @bors r+ |
added the beta-nominated Nominated for backporting to the compiler in the beta channel. label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
No milestone
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK