From fb9b13a69ff7a65db793c442f8ac0381fe755da9 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Tue, 20 Jul 2021 19:02:48 -0400 Subject: [PATCH 1/5] Fix FP for nonstandard_macro_braces lint --- clippy_lints/src/nonstandard_macro_braces.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 043e7fa30d6..490ba5dce3a 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -99,7 +99,7 @@ fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a Mac if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); // we must check only invocation sites // https://github.com/rust-lang/rust-clippy/issues/7422 - if snip.starts_with(name); + if snip.starts_with(&format!("{}!", name)); // make formatting consistent let c = snip.replace(" ", ""); if !c.starts_with(&format!("{}!{}", name, braces.0)); From 5bc5bfce04527acdb7c6ce4d67ca369302c89ec0 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Wed, 21 Jul 2021 07:28:52 -0400 Subject: [PATCH 2/5] Add tests for FP in nonstandard_macro_braces --- .../nonstandard_macro_braces/clippy.toml | 1 + .../conf_nonstandard_macro_braces.rs | 10 +++++ .../conf_nonstandard_macro_braces.stderr | 38 ++++++++++++------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/tests/ui-toml/nonstandard_macro_braces/clippy.toml b/tests/ui-toml/nonstandard_macro_braces/clippy.toml index bced8948a02..50ee363c3ad 100644 --- a/tests/ui-toml/nonstandard_macro_braces/clippy.toml +++ b/tests/ui-toml/nonstandard_macro_braces/clippy.toml @@ -3,4 +3,5 @@ standard-macro-braces = [ { name = "quote::quote", brace = "{" }, { name = "eprint", brace = "[" }, { name = "type_pos", brace = "[" }, + { name = "printlnfoo", brace = "[" }, ] diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index e9f042ddefc..3d884abb4c6 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -32,6 +32,12 @@ macro_rules! type_pos { }; } +macro_rules! printlnfoo { + ($thing:expr) => { + println!("hey {}", $thing) + }; +} + #[rustfmt::skip] fn main() { let _ = vec! {1, 2, 3}; @@ -49,4 +55,8 @@ fn main() { let _: type_pos!(usize) = vec![]; eprint!("test if user config overrides defaults"); + + println!("test if println triggers for printlnfoo"); + + printlnfoo!("you"); } diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 86063a08280..4c3194e258e 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -1,48 +1,48 @@ error: use of irregular braces for `vec!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:37:13 + --> $DIR/conf_nonstandard_macro_braces.rs:43:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ | = note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings` help: consider writing `vec![1, 2, 3]` - --> $DIR/conf_nonstandard_macro_braces.rs:37:13 + --> $DIR/conf_nonstandard_macro_braces.rs:43:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ error: use of irregular braces for `format!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:38:13 + --> $DIR/conf_nonstandard_macro_braces.rs:44:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `format!("ugh () stop being such a good compiler", "hello")` - --> $DIR/conf_nonstandard_macro_braces.rs:38:13 + --> $DIR/conf_nonstandard_macro_braces.rs:44:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:39:13 + --> $DIR/conf_nonstandard_macro_braces.rs:45:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ | help: consider writing `quote! {let x = 1;}` - --> $DIR/conf_nonstandard_macro_braces.rs:39:13 + --> $DIR/conf_nonstandard_macro_braces.rs:45:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote::quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:40:13 + --> $DIR/conf_nonstandard_macro_braces.rs:46:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `quote::quote! {match match match}` - --> $DIR/conf_nonstandard_macro_braces.rs:40:13 + --> $DIR/conf_nonstandard_macro_braces.rs:46:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -67,28 +67,40 @@ LL | let _ = test!(); = 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:49:12 + --> $DIR/conf_nonstandard_macro_braces.rs:55:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ | help: consider writing `type_pos![usize]` - --> $DIR/conf_nonstandard_macro_braces.rs:49:12 + --> $DIR/conf_nonstandard_macro_braces.rs:55:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ error: use of irregular braces for `eprint!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:51:5 + --> $DIR/conf_nonstandard_macro_braces.rs:57: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:51:5 + --> $DIR/conf_nonstandard_macro_braces.rs:57:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: use of irregular braces for `printlnfoo!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:61:5 + | +LL | printlnfoo!("you"); + | ^^^^^^^^^^^^^^^^^^^ + | +help: consider writing `printlnfoo!["you"];` + --> $DIR/conf_nonstandard_macro_braces.rs:61:5 + | +LL | printlnfoo!("you"); + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors From f5c3ed4463acfcfd014a40f743b0a4863863e0f8 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Thu, 22 Jul 2021 08:55:45 -0400 Subject: [PATCH 3/5] Only trigger for one level of macros --- clippy_lints/src/nonstandard_macro_braces.rs | 3 +- .../nonstandard_macro_braces/clippy.toml | 1 - .../conf_nonstandard_macro_braces.rs | 8 ++--- .../conf_nonstandard_macro_braces.stderr | 33 +------------------ 4 files changed, 6 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 490ba5dce3a..5d6d1018a68 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -94,7 +94,8 @@ impl EarlyLintPass for MacroBraces { fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option> { if_chain! { - if in_macro(span); + // Make sure we are only one level deep otherwise there are to many FP's + if in_macro(span) && !in_macro(span.ctxt().outer_expn_data().call_site); if let Some((name, braces)) = find_matching_macro(span, &mac_braces.macro_braces); if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); // we must check only invocation sites diff --git a/tests/ui-toml/nonstandard_macro_braces/clippy.toml b/tests/ui-toml/nonstandard_macro_braces/clippy.toml index 50ee363c3ad..bced8948a02 100644 --- a/tests/ui-toml/nonstandard_macro_braces/clippy.toml +++ b/tests/ui-toml/nonstandard_macro_braces/clippy.toml @@ -3,5 +3,4 @@ standard-macro-braces = [ { name = "quote::quote", brace = "{" }, { name = "eprint", brace = "[" }, { name = "type_pos", brace = "[" }, - { name = "printlnfoo", brace = "[" }, ] diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index 3d884abb4c6..f169fd5ba9c 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -34,7 +34,7 @@ macro_rules! type_pos { macro_rules! printlnfoo { ($thing:expr) => { - println!("hey {}", $thing) + println!("{}", $thing) }; } @@ -44,7 +44,7 @@ fn main() { let _ = format!["ugh {} stop being such a good compiler", "hello"]; let _ = quote!(let x = 1;); let _ = quote::quote!(match match match); - let _ = test!(); + let _ = test!(); // don't trigger for macro calls inside macros let _ = vec![1,2,3]; let _ = quote::quote! {true || false}; @@ -56,7 +56,5 @@ fn main() { eprint!("test if user config overrides defaults"); - println!("test if println triggers for printlnfoo"); - - printlnfoo!("you"); + printlnfoo!["test if printlnfoo is triggered by println"]; } diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 4c3194e258e..846379def0c 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -47,25 +47,6 @@ help: consider writing `quote::quote! {match match match}` LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: use of irregular braces for `vec!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:18:9 - | -LL | vec!{0, 0, 0} - | ^^^^^^^^^^^^^ -... -LL | let _ = test!(); - | ------- in this macro invocation - | -help: consider writing `vec![0, 0, 0]` - --> $DIR/conf_nonstandard_macro_braces.rs:18:9 - | -LL | vec!{0, 0, 0} - | ^^^^^^^^^^^^^ -... -LL | let _ = test!(); - | ------- in this macro invocation - = 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 | @@ -90,17 +71,5 @@ help: consider writing `eprint!["test if user config overrides defaults"];` LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: use of irregular braces for `printlnfoo!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:61:5 - | -LL | printlnfoo!("you"); - | ^^^^^^^^^^^^^^^^^^^ - | -help: consider writing `printlnfoo!["you"];` - --> $DIR/conf_nonstandard_macro_braces.rs:61:5 - | -LL | printlnfoo!("you"); - | ^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors From 44d37a44bc9d6382424c903a624080bd8d9fa966 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Sat, 24 Jul 2021 07:03:52 -0400 Subject: [PATCH 4/5] Lint inside macro when owned by current crate --- clippy_lints/src/nonstandard_macro_braces.rs | 9 +++++++- .../conf_nonstandard_macro_braces.rs | 2 +- .../conf_nonstandard_macro_braces.stderr | 21 ++++++++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 5d6d1018a68..2a318946656 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -93,14 +93,21 @@ impl EarlyLintPass for MacroBraces { } fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option> { + let unnested_or_local = || { + let nested = in_macro(span.ctxt().outer_expn_data().call_site); + let in_local_macro = nested + && matches!(span.macro_backtrace().last().and_then(|e| e.macro_def_id), Some(defid) if defid.is_local()); + !nested || in_local_macro + }; if_chain! { // Make sure we are only one level deep otherwise there are to many FP's - if in_macro(span) && !in_macro(span.ctxt().outer_expn_data().call_site); + if in_macro(span); if let Some((name, braces)) = find_matching_macro(span, &mac_braces.macro_braces); if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); // we must check only invocation sites // https://github.com/rust-lang/rust-clippy/issues/7422 if snip.starts_with(&format!("{}!", name)); + if unnested_or_local(); // make formatting consistent let c = snip.replace(" ", ""); if !c.starts_with(&format!("{}!{}", name, braces.0)); diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index f169fd5ba9c..5b4adc868df 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -44,7 +44,7 @@ fn main() { let _ = format!["ugh {} stop being such a good compiler", "hello"]; let _ = quote!(let x = 1;); let _ = quote::quote!(match match match); - let _ = test!(); // don't trigger for macro calls inside macros + let _ = test!(); // trigger when macro def is inside our own crate let _ = vec![1,2,3]; let _ = quote::quote! {true || false}; diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 846379def0c..87e962b9228 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -47,6 +47,25 @@ help: consider writing `quote::quote! {match match match}` LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: use of irregular braces for `vec!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:18:9 + | +LL | vec!{0, 0, 0} + | ^^^^^^^^^^^^^ +... +LL | let _ = test!(); // trigger when macro def is inside our own crate + | ------- in this macro invocation + | +help: consider writing `vec![0, 0, 0]` + --> $DIR/conf_nonstandard_macro_braces.rs:18:9 + | +LL | vec!{0, 0, 0} + | ^^^^^^^^^^^^^ +... +LL | let _ = test!(); // trigger when macro def is inside our own crate + | ------- in this macro invocation + = 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 | @@ -71,5 +90,5 @@ help: consider writing `eprint!["test if user config overrides defaults"];` LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors From bc7fac9526a157cff245c0bbd55e156b37292a0b Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Tue, 3 Aug 2021 08:19:13 -0400 Subject: [PATCH 5/5] fixup! Lint inside macro when owned by current crate --- clippy_lints/src/nonstandard_macro_braces.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 2a318946656..99423b8a840 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -7,6 +7,7 @@ use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, use if_chain::if_chain; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def_id::DefId; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -95,9 +96,11 @@ impl EarlyLintPass for MacroBraces { fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option> { let unnested_or_local = || { let nested = in_macro(span.ctxt().outer_expn_data().call_site); - let in_local_macro = nested - && matches!(span.macro_backtrace().last().and_then(|e| e.macro_def_id), Some(defid) if defid.is_local()); - !nested || in_local_macro + !nested + || span + .macro_backtrace() + .last() + .map_or(false, |e| e.macro_def_id.map_or(false, DefId::is_local)) }; if_chain! { // Make sure we are only one level deep otherwise there are to many FP's