Add matches! checking to nonstandard_macro_braces #9471
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
| macro_matcher!( | ||
| name: "matches", | ||
| braces: ("(", ")"), | ||
| ), |
There was a problem hiding this comment.
I think it would be nice to add a test even though the change is trivial so that someone modifying the code latter won't break it
There was a problem hiding this comment.
This is just default configuration, if there are no tests at all for this feature I can kind of see your point, otherwise not. In any case I unfortunately don't have much time to figure out how to add completely new testing for this feature, but if there are already some tests and you insist of having them extended for this change, let me know where to find them.
There was a problem hiding this comment.
The tests are here. If you don't have time to add them I don't think it's a blocker, I can add them later
There was a problem hiding this comment.
Added a test and squashed everything. The output in the particular test case I've added is a little strange since it re-"format"s {} to () inside the macro invocation. Should I open an issue about that or is it something known from other lint suggestions that already has an issue?
There was a problem hiding this comment.
Nice catch, I've opened an issue: #9498 Looks like it's there from the initial implementation. Would you like to fix it at the same time? If not it can be done independently
There was a problem hiding this comment.
I have no idea where to even look so I'll let you take care of it.
There was a problem hiding this comment.
I've opened #9499 and rebase on your work so you won't have any conflit
|
If you don't want to merge this first, you can close this in favor of #9499, sure. |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
[`nonstandard_macro_braces`] Do not modify macro arguments fix #9498 based on top of #9471 Also simplify the lint by not caring about code format which should be `rustfmt` job, and turn the lint into machine Applicable changelog: Suggestion: [`nonstandard_macro_braces`]: The suggestion is now machine applicable and will no longer replace brackets inside the macro argument. [#9499](#9499)
changelog: Enhancement: [
nonstandard_macro_braces]: Now includesmatches!()in the default lint config#9471