fix nonstandard_macro_braces: suggest trailing semicolon when needed

Author: ada4a

clippy_lints/src/nonstandard_macro_braces.rs

@@ -16,8 +16,8 @@ declare_clippy_lint! {
     /// Checks that common macros are used with consistent bracing.
     ///
     /// ### Why is this bad?
-    /// This is mostly a consistency lint although using () or []
-    /// doesn't give you a semicolon in item position, which can be unexpected.
+    /// Having non-conventional braces on well-stablished macros can be confusing
+    /// when debugging, and they bring incosistencies with the rest of the ecosystem.
     ///
     /// ### Example
     /// ```no_run
@@ -33,8 +33,12 @@ declare_clippy_lint! {
     "check consistent use of braces in macro"
 }
 
-/// The (callsite span, (open brace, close brace), source snippet)
-type MacroInfo = (Span, (char, char), SourceText);
+struct MacroInfo {
+    callsite_span: Span,
+    callsite_snippet: SourceText,
+    old_open_brace: char,
+    braces: (char, char),
+}
 
 pub struct MacroBraces {
     macro_braces: FxHashMap<String, (char, char)>,
@@ -54,30 +58,58 @@ impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]);
 
 impl EarlyLintPass for MacroBraces {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
-        if let Some((span, braces, snip)) = is_offending_macro(cx, item.span, self) {
-            emit_help(cx, &snip, braces, span);
-            self.done.insert(span);
+        if let Some(MacroInfo {
+            callsite_span,
+            callsite_snippet,
+            braces,
+            ..
+        }) = is_offending_macro(cx, item.span, self)
+        {
+            emit_help(cx, &callsite_snippet, braces, callsite_span, false);
+            self.done.insert(callsite_span);
         }
     }
 
     fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
-        if let Some((span, braces, snip)) = is_offending_macro(cx, stmt.span, self) {
-            emit_help(cx, &snip, braces, span);
-            self.done.insert(span);
+        if let Some(MacroInfo {
+            callsite_span,
+            callsite_snippet,
+            braces,
+            old_open_brace,
+        }) = is_offending_macro(cx, stmt.span, self)
+        {
+            // if we turn `macro!{}` into `macro!()`/`macro![]`, we'll no longer get the implicit
+            // trailing semicolon, see #9913
+            // NOTE: `stmt.kind != StmtKind::MacCall` because `EarlyLintPass` happens after macro expansion
+            let add_semi = matches!(stmt.kind, ast::StmtKind::Expr(..)) && old_open_brace == '{';
+            emit_help(cx, &callsite_snippet, braces, callsite_span, add_semi);
+            self.done.insert(callsite_span);
         }
     }
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
-        if let Some((span, braces, snip)) = is_offending_macro(cx, expr.span, self) {
-            emit_help(cx, &snip, braces, span);
-            self.done.insert(span);
+        if let Some(MacroInfo {
+            callsite_span,
+            callsite_snippet,
+            braces,
+            ..
+        }) = is_offending_macro(cx, expr.span, self)
+        {
+            emit_help(cx, &callsite_snippet, braces, callsite_span, false);
+            self.done.insert(callsite_span);
         }
     }
 
     fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
-        if let Some((span, braces, snip)) = is_offending_macro(cx, ty.span, self) {
-            emit_help(cx, &snip, braces, span);
-            self.done.insert(span);
+        if let Some(MacroInfo {
+            callsite_span,
+            braces,
+            callsite_snippet,
+            ..
+        }) = is_offending_macro(cx, ty.span, self)
+        {
+            emit_help(cx, &callsite_snippet, braces, callsite_span, false);
+            self.done.insert(callsite_span);
         }
     }
 }
@@ -90,39 +122,44 @@ fn is_offending_macro(cx: &EarlyContext<'_>, span: Span, mac_braces: &MacroBrace
                 .last()
                 .is_some_and(|e| e.macro_def_id.is_some_and(DefId::is_local))
     };
-    let span_call_site = span.ctxt().outer_expn_data().call_site;
+    let callsite_span = span.ctxt().outer_expn_data().call_site;
     if let ExpnKind::Macro(MacroKind::Bang, mac_name) = span.ctxt().outer_expn_data().kind
         && let name = mac_name.as_str()
         && let Some(&braces) = mac_braces.macro_braces.get(name)
-        && let Some(snip) = span_call_site.get_source_text(cx)
+        && let Some(snip) = callsite_span.get_source_text(cx)
         // we must check only invocation sites
         // https://github.com/rust-lang/rust-clippy/issues/7422
-        && snip.starts_with(&format!("{name}!"))
+        && let Some(macro_args_str) = snip.strip_prefix(name).and_then(|snip| snip.strip_prefix('!'))
+        && let Some(old_open_brace @ ('{' | '(' | '[')) = macro_args_str.trim_start().chars().next()
+        && old_open_brace != braces.0
         && unnested_or_local()
-        // make formatting consistent
-        && let c = snip.replace(' ', "")
-        && !c.starts_with(&format!("{name}!{}", braces.0))
-        && !mac_braces.done.contains(&span_call_site)
+        && !mac_braces.done.contains(&callsite_span)
     {
-        Some((span_call_site, braces, snip))
+        Some(MacroInfo {
+            callsite_span,
+            callsite_snippet: snip,
+            old_open_brace,
+            braces,
+        })
     } else {
         None
     }
 }
 
-fn emit_help(cx: &EarlyContext<'_>, snip: &str, (open, close): (char, char), span: Span) {
+fn emit_help(cx: &EarlyContext<'_>, snip: &str, (open, close): (char, char), span: Span, add_semi: bool) {
+    let semi = if add_semi { ";" } else { "" };
     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();
+        macro_args.remove(0);
         span_lint_and_sugg(
             cx,
             NONSTANDARD_MACRO_BRACES,
             span,
             format!("use of irregular braces for `{macro_name}!` macro"),
             "consider writing",
-            format!("{macro_name}!{open}{macro_args}{close}"),
+            format!("{macro_name}!{open}{macro_args}{close}{semi}"),
             Applicability::MachineApplicable,
         );
     }

tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed

@@ -67,3 +67,11 @@ fn main() {
 
     printlnfoo!["test if printlnfoo is triggered by println"];
 }
+
+#[rustfmt::skip]
+#[expect(clippy::no_effect)]
+fn issue9913() {
+    println!("hello world");
+    [0]; // separate statement, not indexing into the result of println.
+    //~^^ nonstandard_macro_braces
+}

tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs

@@ -67,3 +67,11 @@ fn main() {
 
     printlnfoo!["test if printlnfoo is triggered by println"];
 }
+
+#[rustfmt::skip]
+#[expect(clippy::no_effect)]
+fn issue9913() {
+    println! {"hello world"}
+    [0]; // separate statement, not indexing into the result of println.
+    //~^^ nonstandard_macro_braces
+}

tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr

@@ -54,5 +54,11 @@ error: use of irregular braces for `eprint!` macro
 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
+error: use of irregular braces for `println!` macro
+  --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:74:5
+   |
+LL |     println! {"hello world"}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `println!("hello world");`
+
+error: aborting due to 9 previous errors