Auto merge of #9499 - kraktus:nonstandard_macro_braces, r=xFrednet

bors · bors · commit 61fd2a8c6f83 · 2022-09-22T14:25:27.000Z
[`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)
diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs
@@ -3,16 +3,17 @@ use std::{
     hash::{Hash, Hasher},
 };
 
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_opt;
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
 use rustc_lint::{EarlyContext, EarlyLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::hygiene::{ExpnKind, MacroKind};
-use rustc_span::{Span, Symbol};
+use rustc_span::Span;
 use serde::{de, Deserialize};
 
 declare_clippy_lint! {
@@ -39,8 +40,8 @@ declare_clippy_lint! {
 
 const BRACES: &[(&str, &str)] = &[("(", ")"), ("{", "}"), ("[", "]")];
 
-/// The (name, (open brace, close brace), source snippet)
-type MacroInfo<'a> = (Symbol, &'a (String, String), String);
+/// The (callsite span, (open brace, close brace), source snippet)
+type MacroInfo<'a> = (Span, &'a (String, String), String);
 
 #[derive(Clone, Debug, Default)]
 pub struct MacroBraces {
@@ -62,33 +63,29 @@ impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]);
 
 impl EarlyLintPass for MacroBraces {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
-        if let Some((name, braces, snip)) = is_offending_macro(cx, item.span, self) {
-            let span = item.span.ctxt().outer_expn_data().call_site;
-            emit_help(cx, snip, braces, name, span);
+        if let Some((span, braces, snip)) = is_offending_macro(cx, item.span, self) {
+            emit_help(cx, &snip, braces, span);
             self.done.insert(span);
         }
     }
 
     fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
-        if let Some((name, braces, snip)) = is_offending_macro(cx, stmt.span, self) {
-            let span = stmt.span.ctxt().outer_expn_data().call_site;
-            emit_help(cx, snip, braces, name, span);
+        if let Some((span, braces, snip)) = is_offending_macro(cx, stmt.span, self) {
+            emit_help(cx, &snip, braces, span);
             self.done.insert(span);
         }
     }
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
-        if let Some((name, braces, snip)) = is_offending_macro(cx, expr.span, self) {
-            let span = expr.span.ctxt().outer_expn_data().call_site;
-            emit_help(cx, snip, braces, name, span);
+        if let Some((span, braces, snip)) = is_offending_macro(cx, expr.span, self) {
+            emit_help(cx, &snip, braces, span);
             self.done.insert(span);
         }
     }
 
     fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
-        if let Some((name, braces, snip)) = is_offending_macro(cx, ty.span, self) {
-            let span = ty.span.ctxt().outer_expn_data().call_site;
-            emit_help(cx, snip, braces, name, span);
+        if let Some((span, braces, snip)) = is_offending_macro(cx, ty.span, self) {
+            emit_help(cx, &snip, braces, span);
             self.done.insert(span);
         }
     }
@@ -102,48 +99,44 @@ fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a Mac
                 .last()
                 .map_or(false, |e| e.macro_def_id.map_or(false, DefId::is_local))
     };
+    let span_call_site = span.ctxt().outer_expn_data().call_site;
     if_chain! {
         if let ExpnKind::Macro(MacroKind::Bang, mac_name) = span.ctxt().outer_expn_data().kind;
         let name = mac_name.as_str();
         if let Some(braces) = mac_braces.macro_braces.get(name);
-        if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site);
+        if let Some(snip) = snippet_opt(cx, span_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));
-        if !mac_braces.done.contains(&span.ctxt().outer_expn_data().call_site);
+        if !mac_braces.done.contains(&span_call_site);
         then {
-            Some((mac_name, braces, snip))
+            Some((span_call_site, braces, snip))
         } else {
             None
         }
     }
 }
 
-fn emit_help(cx: &EarlyContext<'_>, snip: String, braces: &(String, String), name: Symbol, span: Span) {
-    let with_space = &format!("! {}", braces.0);
-    let without_space = &format!("!{}", braces.0);
-    let mut help = snip;
-    for b in BRACES.iter().filter(|b| b.0 != braces.0) {
-        help = help.replace(b.0, &braces.0).replace(b.1, &braces.1);
-        // Only `{` traditionally has space before the brace
-        if braces.0 != "{" && help.contains(with_space) {
-            help = help.replace(with_space, without_space);
-        } else if braces.0 == "{" && help.contains(without_space) {
-            help = help.replace(without_space, with_space);
-        }
+fn emit_help(cx: &EarlyContext<'_>, snip: &str, braces: &(String, String), span: Span) {
+    if let Some((macro_name, macro_args_str)) = snip.split_once('!') {
+        let mut macro_args = macro_args_str.trim().to_string();
+        // now remove the wrong braces
+        macro_args.remove(0);
+        macro_args.pop();
+        span_lint_and_sugg(
+            cx,
+            NONSTANDARD_MACRO_BRACES,
+            span,
+            &format!("use of irregular braces for `{}!` macro", macro_name),
+            "consider writing",
+            format!("{}!{}{}{}", macro_name, braces.0, macro_args, braces.1),
+            Applicability::MachineApplicable,
+        );
     }
-    span_lint_and_help(
-        cx,
-        NONSTANDARD_MACRO_BRACES,
-        span,
-        &format!("use of irregular braces for `{}!` macro", name),
-        Some(span),
-        &format!("consider writing `{}`", help),
-    );
 }
 
 fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, String)> {
diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed
@@ -0,0 +1,62 @@
+// aux-build:proc_macro_derive.rs
+// run-rustfix
+
+#![warn(clippy::nonstandard_macro_braces)]
+
+extern crate proc_macro_derive;
+extern crate quote;
+
+use quote::quote;
+
+#[derive(proc_macro_derive::DeriveSomething)]
+pub struct S;
+
+proc_macro_derive::foo_bar!();
+
+#[rustfmt::skip]
+macro_rules! test {
+    () => {
+        vec![0, 0, 0]
+    };
+}
+
+#[rustfmt::skip]
+macro_rules! test2 {
+    ($($arg:tt)*) => {
+        format_args!($($arg)*)
+    };
+}
+
+macro_rules! type_pos {
+    ($what:ty) => {
+        Vec<$what>
+    };
+}
+
+macro_rules! printlnfoo {
+    ($thing:expr) => {
+        println!("{}", $thing)
+    };
+}
+
+#[rustfmt::skip]
+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
+    let _ = vec![1,2,3];
+
+    let _ = quote::quote! {true || false};
+    let _ = vec! [0 ,0 ,0];
+    let _ = format!("fds{}fds", 10);
+    let _ = test2!["{}{}{}", 1, 2, 3];
+
+    let _: type_pos![usize] = vec![];
+
+    eprint!["test if user config overrides defaults"];
+
+    printlnfoo!["test if printlnfoo is triggered by println"];
+}
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
@@ -1,4 +1,5 @@
 // aux-build:proc_macro_derive.rs
+// run-rustfix
 
 #![warn(clippy::nonstandard_macro_braces)]
 
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
@@ -1,106 +1,57 @@
 error: use of irregular braces for `vec!` macro
-  --> $DIR/conf_nonstandard_macro_braces.rs:43:13
+  --> $DIR/conf_nonstandard_macro_braces.rs:44:13
    |
 LL |     let _ = vec! {1, 2, 3};
-   |             ^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^ help: consider writing: `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:43:13
-   |
-LL |     let _ = vec! {1, 2, 3};
-   |             ^^^^^^^^^^^^^^
 
 error: use of irregular braces for `format!` macro
-  --> $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:44:13
+  --> $DIR/conf_nonstandard_macro_braces.rs:45:13
    |
 LL |     let _ = format!["ugh {} stop being such a good compiler", "hello"];
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `format!("ugh {} stop being such a good compiler", "hello")`
 
 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
+  --> $DIR/conf_nonstandard_macro_braces.rs:46:13
    |
 LL |     let _ = matches!{{}, ()};
-   |             ^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^ help: consider writing: `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:46:13
+  --> $DIR/conf_nonstandard_macro_braces.rs:47:13
    |
 LL |     let _ = quote!(let x = 1;);
-   |             ^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^ help: consider writing: `quote!{let x = 1;}`
 
 error: use of irregular braces for `quote::quote!` macro
-  --> $DIR/conf_nonstandard_macro_braces.rs:47:13
+  --> $DIR/conf_nonstandard_macro_braces.rs:48:13
    |
 LL |     let _ = quote::quote!(match match match);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: consider writing `quote::quote! {match match match}`
-  --> $DIR/conf_nonstandard_macro_braces.rs:47:13
-   |
-LL |     let _ = quote::quote!(match match match);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `quote::quote!{match match match}`
 
 error: use of irregular braces for `vec!` macro
-  --> $DIR/conf_nonstandard_macro_braces.rs:18:9
+  --> $DIR/conf_nonstandard_macro_braces.rs:19:9
    |
 LL |         vec!{0, 0, 0}
-   |         ^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^ help: consider writing: `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:56:12
+  --> $DIR/conf_nonstandard_macro_braces.rs:57:12
    |
 LL |     let _: type_pos!(usize) = vec![];
-   |            ^^^^^^^^^^^^^^^^
-   |
-help: consider writing `type_pos![usize]`
-  --> $DIR/conf_nonstandard_macro_braces.rs:56:12
-   |
-LL |     let _: type_pos!(usize) = vec![];
-   |            ^^^^^^^^^^^^^^^^
+   |            ^^^^^^^^^^^^^^^^ help: consider writing: `type_pos![usize]`
 
 error: use of irregular braces for `eprint!` macro
-  --> $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:58:5
+  --> $DIR/conf_nonstandard_macro_braces.rs:59:5
    |
 LL |     eprint!("test if user config overrides defaults");
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `eprint!["test if user config overrides defaults"]`
 
 error: aborting due to 8 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`// aux-build:proc_macro_derive.rs`
	`2`	`+// run-rustfix`
`2`	`3`
`3`	`4`	`#![warn(clippy::nonstandard_macro_braces)]`
`4`	`5`