Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,9 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
// Due to the architecture of the compiler, currently `cfg_attr` attributes on crate
// level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass.
store.register_pre_expansion_lint_pass(Box::new(move || Box::new(attrs::EarlyAttributes::new(conf))));
store.register_pre_expansion_lint_pass(Box::new(move || {
Box::new(nonstandard_macro_braces::MacroBraces::new(conf))
}));

let format_args_storage = FormatArgsStorage::default();
let attr_storage = AttrStorage::default();
Expand Down
206 changes: 104 additions & 102 deletions clippy_lints/src/nonstandard_macro_braces.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
use clippy_config::Conf;
use clippy_config::types::MacroMatcher;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{SourceText, SpanExt};
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_ast::token::{Delimiter, Token, TokenKind};
use rustc_ast::tokenstream::{TokenStream, TokenTree};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::Span;
use rustc_span::hygiene::{ExpnKind, MacroKind};

use crate::rustc_lint::LintContext;
use clippy_utils::source::snippet_opt;

declare_clippy_lint! {
/// ### What it does
Expand All @@ -29,127 +31,111 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.55.0"]
pub NONSTANDARD_MACRO_BRACES,
nursery,
style,
"check consistent use of braces in macro"
}

impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]);

struct MacroInfo {
callsite_span: Span,
callsite_snippet: SourceText,
old_open_brace: char,
braces: (char, char),
}

pub struct MacroBraces {
macro_braces: FxHashMap<String, (char, char)>,
done: FxHashSet<Span>,
macro_braces: (FxHashMap<String, (char, char)>, usize),
/// Spans for statement macro calls, they have special behaviour with semicolons
mac_stmt_spans: Vec<Span>,
}

impl MacroBraces {
pub fn new(conf: &'static Conf) -> Self {
Self {
macro_braces: macro_braces(&conf.standard_macro_braces),
done: FxHashSet::default(),
mac_stmt_spans: Vec::new(),
}
}
}

impl EarlyLintPass for MacroBraces {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
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(MacroInfo {
callsite_span,
callsite_snippet,
braces,
old_open_brace,
}) = is_offending_macro(cx, stmt.span, self)
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &ast::MacCall) {
if let Some(last_segment) = mac.path.segments.last()
&& let name = last_segment.ident.as_str()
&& let Some(&braces) = self.macro_braces.0.get(name)
&& let Some(snip) = snippet_opt(cx.sess(), mac.span().with_lo(last_segment.span().lo()))
&& 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
{
// 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);
// Semicolons added for statements that previously ended in braces, see issue #9913
let add_semi = self.mac_stmt_spans.iter().any(|s| *s == mac.span());
emit_help(
cx,
&snippet_opt(cx.sess(), mac.span()).unwrap(),
braces,
mac.span(),
add_semi,
);
}
}

fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let Some(MacroInfo {
callsite_span,
callsite_snippet,
braces,
..
}) = is_offending_macro(cx, expr.span, self)
// See issue #9913
fn check_stmt(&mut self, _: &EarlyContext<'_>, stmt: &ast::Stmt) {
if let ast::StmtKind::MacCall(mac_callstmt) = &stmt.kind
&& let ast::MacCallStmt {
style: ast::MacStmtStyle::Braces,
..
} = **mac_callstmt
{
emit_help(cx, &callsite_snippet, braces, callsite_span, false);
self.done.insert(callsite_span);
self.mac_stmt_spans.push(mac_callstmt.mac.span());
}
}

fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
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);
fn check_mac_def(&mut self, cx: &EarlyContext<'_>, mac: &ast::MacroDef) {
fn check_ts(cx: &EarlyContext<'_>, ts: &TokenStream, macro_braces: &FxHashMap<String, (char, char)>) {
for (i, current_token) in ts.iter().enumerate() {
if let TokenTree::Delimited(_, _, _, token_stream) = current_token {
// Peel extra braces and parenthesis in macros!
check_ts(cx, token_stream, macro_braces);
} else
// |-TokenKind::Bang
// v
// println! { "Hi" }
// ^^^^^^^
// | ^^^^^^^^ Brackets always come 1 token after TokenKind::Bang
// ident_token
if let TokenTree::Token(
Token {
kind: TokenKind::Ident(ident_token, _),
span: ident_span,
},
_,
) = current_token
&& let Some(bang_token) = ts.get(i + 1)
&& let Some(macro_args_token) = ts.get(i + 2)
&& let TokenTree::Token(
Token {
kind: TokenKind::Bang, ..
},
_,
) = *bang_token
&& let TokenTree::Delimited(delim_span, _, delim, _) = macro_args_token
// Span from ident_token to brackets (so, the full macro call)
&& let snip_span = ident_span.with_hi(delim_span.close.hi())
&& let Some(snip) = snippet_opt(cx, snip_span)
&& let Some(&braces) = macro_braces.get(ident_token.as_str())
&& let Some(old_open_brace) = match delim {
Delimiter::Brace => Some('{'),
Delimiter::Parenthesis => Some('('),
Delimiter::Bracket => Some('['),
Delimiter::Invisible(_) => None,
}
&& old_open_brace != braces.0
{
emit_help(cx, &snip, braces, snip_span, false);
}
}
}
}
}

fn is_offending_macro(cx: &EarlyContext<'_>, span: Span, mac_braces: &MacroBraces) -> Option<MacroInfo> {
let unnested_or_local = |span: Span| {
!span.from_expansion()
|| span
.macro_backtrace()
.last()
.is_some_and(|e| e.macro_def_id.is_some_and(DefId::is_local))
};

let mut ctxt = span.ctxt();
while !ctxt.is_root() {
let expn_data = ctxt.outer_expn_data();
if let ExpnKind::Macro(MacroKind::Bang, mac_name) = expn_data.kind
&& let name = mac_name.as_str()
&& let Some(&braces) = mac_braces.macro_braces.get(name)
&& let Some(snip) = expn_data.call_site.get_text(cx)
// we must check only invocation sites
// https://github.com/rust-lang/rust-clippy/issues/7422
&& 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(expn_data.call_site)
&& !mac_braces.done.contains(&expn_data.call_site)
{
return Some(MacroInfo {
callsite_span: expn_data.call_site,
callsite_snippet: snip,
old_open_brace,
braces,
});
if mac.macro_rules {
check_ts(cx, &mac.body.tokens, &self.macro_braces.0);
}

ctxt = expn_data.call_site.ctxt();
}

None
}

fn emit_help(cx: &EarlyContext<'_>, snip: &str, (open, close): (char, char), span: Span, add_semi: bool) {
Expand All @@ -171,17 +157,21 @@ fn emit_help(cx: &EarlyContext<'_>, snip: &str, (open, close): (char, char), spa
}
}

fn macro_braces(conf: &[MacroMatcher]) -> FxHashMap<String, (char, char)> {
fn macro_braces(conf: &[MacroMatcher]) -> (FxHashMap<String, (char, char)>, usize) {
// TODO: Use `Symbol`s here, instead of strings.
let mut braces = FxHashMap::from_iter(
[
("print", ('(', ')')),
("println", ('(', ')')),
("assert_matches", ('(', ')')),
("cfg_select", ('{', '}')),
("debug_assert_matches", ('(', ')')),
("format", ('(', ')')),
("format_args", ('(', ')')),
("eprint", ('(', ')')),
("eprintln", ('(', ')')),
("print", ('(', ')')),
("println", ('(', ')')),
("write", ('(', ')')),
("writeln", ('(', ')')),
("format", ('(', ')')),
("format_args", ('(', ')')),
("vec", ('[', ']')),
Comment thread
blyxyas marked this conversation as resolved.
("matches", ('(', ')')),
]
Expand All @@ -191,5 +181,17 @@ fn macro_braces(conf: &[MacroMatcher]) -> FxHashMap<String, (char, char)> {
for it in conf {
braces.insert(it.name.clone(), it.braces);
}
braces

#[expect(
rustc::potential_query_instability,
reason = "iteration order does not matter for `.max()`"
)]
#[expect(clippy::redundant_closure_for_method_calls, reason = "Clarity")]
let max_len = braces
.keys()
.map(|macro_name| macro_name.len())
Comment thread
blyxyas marked this conversation as resolved.
.max()
.expect("`braces` is non-empty");

(braces, max_len)
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ proc_macro_derive::foo_bar!();

#[rustfmt::skip]
macro_rules! test {
() => {
() => {{
//~v nonstandard_macro_braces
vec![0, 0, 0]
//~^ nonstandard_macro_braces
};
}};
}

#[rustfmt::skip]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ proc_macro_derive::foo_bar!();

#[rustfmt::skip]
macro_rules! test {
() => {
() => {{
//~v nonstandard_macro_braces
vec!{0, 0, 0}
//~^ nonstandard_macro_braces
};
}};
}

#[rustfmt::skip]
Expand Down
Loading