Skip to content

Add matches! checking to nonstandard_macro_braces #9471

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

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Add matches! checking to nonstandard_macro_braces #9471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clippy_lints/src/nonstandard_macro_braces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, Str
name: "vec",
braces: ("[", "]"),
),
macro_matcher!(
name: "matches",
braces: ("(", ")"),
),
Comment on lines +187 to +190
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

@kraktus kraktus Sep 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea where to even look so I'll let you take care of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've opened #9499 and rebase on your work so you won't have any conflit

]
.into_iter()
.collect::<FxHashMap<_, _>>();
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ macro_rules! printlnfoo {
fn main() {
let _ = vec! {1, 2, 3};
let _ = format!["ugh {} stop being such a good compiler", "hello"];
let _ = matches!{{}, ()};
let _ = quote!(let x = 1;);
let _ = quote::quote!(match match match);
let _ = test!(); // trigger when macro def is inside our own crate
Expand Down
30 changes: 21 additions & 9 deletions tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,38 @@ help: consider writing `format!("ugh () stop being such a good compiler", "hello
LL | let _ = format!["ugh {} stop being such a good compiler", "hello"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: use of irregular braces for `quote!` macro
error: use of irregular braces for `matches!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:45:13
|
LL | let _ = matches!{{}, ()};
| ^^^^^^^^^^^^^^^^
|
help: consider writing `matches!((), ())`
--> $DIR/conf_nonstandard_macro_braces.rs:45:13
|
LL | let _ = matches!{{}, ()};
| ^^^^^^^^^^^^^^^^

error: use of irregular braces for `quote!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:46:13
|
LL | let _ = quote!(let x = 1;);
| ^^^^^^^^^^^^^^^^^^
|
help: consider writing `quote! {let x = 1;}`
--> $DIR/conf_nonstandard_macro_braces.rs:45:13
--> $DIR/conf_nonstandard_macro_braces.rs:46:13
|
LL | let _ = quote!(let x = 1;);
| ^^^^^^^^^^^^^^^^^^

error: use of irregular braces for `quote::quote!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:46:13
--> $DIR/conf_nonstandard_macro_braces.rs:47:13
|
LL | let _ = quote::quote!(match match match);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider writing `quote::quote! {match match match}`
--> $DIR/conf_nonstandard_macro_braces.rs:46:13
--> $DIR/conf_nonstandard_macro_braces.rs:47:13
|
LL | let _ = quote::quote!(match match match);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -67,28 +79,28 @@ LL | let _ = test!(); // trigger when macro def is inside our own crate
= note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

error: use of irregular braces for `type_pos!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:55:12
--> $DIR/conf_nonstandard_macro_braces.rs:56:12
|
LL | let _: type_pos!(usize) = vec![];
| ^^^^^^^^^^^^^^^^
|
help: consider writing `type_pos![usize]`
--> $DIR/conf_nonstandard_macro_braces.rs:55:12
--> $DIR/conf_nonstandard_macro_braces.rs:56:12
|
LL | let _: type_pos!(usize) = vec![];
| ^^^^^^^^^^^^^^^^

error: use of irregular braces for `eprint!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:57:5
--> $DIR/conf_nonstandard_macro_braces.rs:58:5
|
LL | eprint!("test if user config overrides defaults");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider writing `eprint!["test if user config overrides defaults"]`
--> $DIR/conf_nonstandard_macro_braces.rs:57:5
--> $DIR/conf_nonstandard_macro_braces.rs:58:5
|
LL | eprint!("test if user config overrides defaults");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 7 previous errors
error: aborting due to 8 previous errors