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

[`nonstandard_macro_braces`] Do not modify macro arguments fix #9498 based on top of https://github.com/rust-lang/rust-clippy/pull/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](https://github.com/rust-lang/rust-clippy/pull/9499)
2022-09-22 14:25:27 +00:00 · 2022-09-22 14:25:27 +00:00 · 61fd2a8c6f
commit 61fd2a8c6f
parent c2c170adbe a390115c6f
4 changed files with 111 additions and 104 deletions
--- 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,11 +99,12 @@ 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));
@ -114,36 +112,31 @@ fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a Mac
        // 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)> {
--- 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"];
+}
--- 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)]

--- 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
+  --> $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")`
-  --> $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")`

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

 error: use of irregular braces for `quote!` macro
-  --> $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;}`
-  --> $DIR/conf_nonstandard_macro_braces.rs:46: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
+  --> $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"]`
-  --> $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"]`

 error: aborting due to 8 previous errors