Skip to content

Commit 841f851

Browse files
committed
fix(double_parens): don't lint in proc-macros
1 parent 0c592df commit 841f851

File tree

6 files changed

+266
-13
lines changed

6 files changed

+266
-13
lines changed

clippy_lints/src/double_parens.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::is_from_proc_macro;
23
use clippy_utils::source::{HasSession, snippet_with_applicability, snippet_with_context};
34
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
45
use rustc_errors::Applicability;
@@ -48,7 +49,7 @@ impl EarlyLintPass for DoubleParens {
4849
// ^^^^ inner
4950
ExprKind::Paren(inner) if matches!(inner.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => {
5051
// suggest removing the outer parens
51-
if expr.span.eq_ctxt(inner.span) {
52+
if expr.span.eq_ctxt(inner.span) && !is_from_proc_macro(cx, &**inner) {
5253
let mut applicability = Applicability::MachineApplicable;
5354
// We don't need to use `snippet_with_context` here, because:
5455
// - if `inner`'s `ctxt` is from macro, we don't lint in the first place (see the check above)
@@ -75,7 +76,7 @@ impl EarlyLintPass for DoubleParens {
7576
&& let ExprKind::Paren(inner) = &arg.kind =>
7677
{
7778
// suggest removing the inner parens
78-
if expr.span.eq_ctxt(arg.span) {
79+
if expr.span.eq_ctxt(arg.span) && !is_from_proc_macro(cx, &**arg) {
7980
let mut applicability = Applicability::MachineApplicable;
8081
let sugg = snippet_with_context(cx.sess(), inner.span, arg.span.ctxt(), "_", &mut applicability).0;
8182
span_lint_and_sugg(

clippy_utils/src/check_proc_macro.rs

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ use rustc_abi::ExternAbi;
1616
use rustc_ast as ast;
1717
use rustc_ast::AttrStyle;
1818
use rustc_ast::ast::{
19-
AttrKind, Attribute, GenericArgs, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy,
19+
AttrKind, Attribute, GenericArgs, IntTy, LitIntType, LitKind, RangeLimits, StrStyle, StructExpr, TraitObjectSyntax,
20+
UintTy,
2021
};
2122
use rustc_ast::token::CommentKind;
2223
use rustc_hir::intravisit::FnKind;
@@ -419,6 +420,22 @@ fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) {
419420
}
420421
}
421422

423+
fn ast_path_search_pat(path: &ast::Path) -> (Pat, Pat) {
424+
let (head, tail) = match &*path.segments {
425+
[] => return (Pat::Str(""), Pat::Str("")),
426+
[p] => (Pat::Sym(p.ident.name), p),
427+
[p, .., tail] => (Pat::Sym(p.ident.name), tail),
428+
};
429+
(
430+
head,
431+
if tail.args.is_some() {
432+
Pat::Str(">")
433+
} else {
434+
Pat::Sym(tail.ident.name)
435+
},
436+
)
437+
}
438+
422439
fn ast_ty_search_pat(ty: &ast::Ty) -> (Pat, Pat) {
423440
use ast::{Extern, FnRetTy, MutTy, Safety, TraitObjectSyntax, TyKind};
424441

@@ -537,6 +554,163 @@ fn ast_ty_search_pat(ty: &ast::Ty) -> (Pat, Pat) {
537554
}
538555
}
539556

557+
/// Get the search patterns to use for the given literal
558+
fn token_lit_search_pat(lit: &ast::token::Lit) -> (Pat, Pat) {
559+
use ast::token::LitKind;
560+
561+
match lit.kind {
562+
LitKind::Bool => (Pat::MultiStr(&["true", "false"]), Pat::MultiStr(&["true", "false"])),
563+
LitKind::Byte => (Pat::Str("b'"), Pat::Str("'")),
564+
LitKind::ByteStr => (Pat::Str("b\""), Pat::Str("\"")),
565+
LitKind::ByteStrRaw(0) => (Pat::Str("br\""), Pat::Str("\"")),
566+
LitKind::ByteStrRaw(_) => (Pat::Str("br#\""), Pat::Str("#")),
567+
LitKind::CStr => (Pat::Str("c\""), Pat::Str("\"")),
568+
LitKind::CStrRaw(0) => (Pat::Str("cr\""), Pat::Str("\"")),
569+
LitKind::CStrRaw(_) => (Pat::Str("cr#\""), Pat::Str("#\"")),
570+
LitKind::Char => (Pat::Str("'"), Pat::Str("'")),
571+
LitKind::Float | LitKind::Integer => (Pat::Sym(lit.symbol), Pat::Sym(lit.suffix.unwrap_or(lit.symbol))),
572+
LitKind::Str => (Pat::Str("\""), Pat::Str("\"")),
573+
LitKind::StrRaw(0) => (Pat::Str("r"), Pat::Str("\"")),
574+
LitKind::StrRaw(_) => (Pat::Str("r#"), Pat::Str("#")),
575+
LitKind::Err(_) => (Pat::Str(""), Pat::Str("")),
576+
}
577+
}
578+
579+
/// Get the search patterns to use for the given expression
580+
fn ast_expr_search_pat(e: &ast::Expr) -> (Pat, Pat) {
581+
fn inner(e: &ast::Expr, outer_span: Span) -> (Pat, Pat) {
582+
use ast::{
583+
Block, BlockCheckMode, CaptureBy, Closure, ExprKind, GenBlockKind, MatchKind, MethodCall, UnsafeSource,
584+
YieldKind,
585+
};
586+
587+
// The expression can have subexpressions in different contexts, in which case
588+
// building up a search pattern from the macro expansion would lead to false positives;
589+
// e.g. `return format!(..)` would be considered to be from a proc macro
590+
// if we build up a pattern for the macro expansion and compare it to the invocation `format!()`.
591+
// So instead we return an empty pattern such that `span_matches_pat` always returns true.
592+
if !e.span.eq_ctxt(outer_span) {
593+
return (Pat::Str(""), Pat::Str(""));
594+
}
595+
596+
match &e.kind {
597+
ExprKind::Underscore => (Pat::Str("_"), Pat::Str("_")),
598+
ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("")),
599+
ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), inner(e, outer_span).1),
600+
ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), inner(e, outer_span).1),
601+
ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), inner(e, outer_span).1),
602+
ExprKind::Lit(lit) => token_lit_search_pat(lit),
603+
ExprKind::Paren(e) => inner(e, outer_span),
604+
ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")),
605+
ExprKind::Range(None, None, lims) => range_limits_search_pat(lims),
606+
ExprKind::Range(None, Some(end), lims) => (range_limits_search_pat(lims).0, inner(end, outer_span).1),
607+
ExprKind::Range(Some(start), None, lims) => (inner(start, outer_span).0, range_limits_search_pat(lims).1),
608+
ExprKind::Call(e, args)
609+
| ExprKind::MethodCall(box MethodCall {
610+
seg: _,
611+
receiver: e,
612+
args,
613+
span: _,
614+
}) => (
615+
inner(e, outer_span).0,
616+
// Parenthesis are trimmed from the text before the search patterns are matched.
617+
// See: `span_matches_pat`
618+
match &**args {
619+
[] => Pat::Str("("),
620+
[.., last] => inner(last, outer_span).1,
621+
},
622+
),
623+
ExprKind::Binary(_, first, last)
624+
| ExprKind::Assign(first, last, _)
625+
| ExprKind::AssignOp(_, first, last)
626+
| ExprKind::Range(Some(first), Some(last), _) => {
627+
(inner(first, outer_span).0, inner(last, outer_span).1)
628+
},
629+
ExprKind::Tup(lst) => {
630+
match &**lst {
631+
// Parentheses are trimmed from the text before the search patterns are matched.
632+
// See: `span_matches_pat`
633+
[] => (Pat::Str(")"), Pat::Str("(")),
634+
[e] => inner(e, outer_span),
635+
[first, .., last] => (inner(first, outer_span).0, inner(last, outer_span).1),
636+
}
637+
},
638+
ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (inner(e, outer_span).0, Pat::Str("")),
639+
ExprKind::Let(_, init, _, _) => (Pat::Str("let"), inner(init, outer_span).1),
640+
ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")),
641+
ExprKind::Loop(_, Some(_), _)
642+
| ExprKind::While(_, _, Some(_))
643+
| ExprKind::ForLoop { label: Some(_), .. }
644+
| ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")),
645+
ExprKind::Loop(_, None, _) => (Pat::Str("loop"), Pat::Str("}")),
646+
ExprKind::While(_, _, None) => (Pat::Str("while"), Pat::Str("}")),
647+
ExprKind::ForLoop { label: None, .. } => (Pat::Str("for"), Pat::Str("}")),
648+
ExprKind::Match(_, _, MatchKind::Prefix) => (Pat::Str("match"), Pat::Str("}")),
649+
ExprKind::Match(e, _, MatchKind::Postfix) => (inner(e, outer_span).0, Pat::Str("}")),
650+
ExprKind::Try(e) => (inner(e, outer_span).0, Pat::Str("?")),
651+
ExprKind::TryBlock(_) => (Pat::Str("try"), Pat::Str("}")),
652+
ExprKind::Await(e, _) => (inner(e, outer_span).0, Pat::Str("await")),
653+
ExprKind::Closure(box Closure {
654+
capture_clause, body, ..
655+
}) => {
656+
let start = match capture_clause {
657+
CaptureBy::Value { .. } => "move",
658+
CaptureBy::Use { .. } => "use",
659+
CaptureBy::Ref => "|",
660+
};
661+
(Pat::Str(start), inner(body, outer_span).1)
662+
},
663+
ExprKind::Gen(_, _,GenBlockKind::Async | GenBlockKind::AsyncGen, _) => (Pat::Str("async"), Pat::Str("")),
664+
ExprKind::Gen(_, _,GenBlockKind::Gen, _) => (Pat::Str("gen"), Pat::Str("")),
665+
ExprKind::Block(
666+
box Block {
667+
rules: BlockCheckMode::Unsafe(UnsafeSource::UserProvided),
668+
..
669+
},
670+
None,
671+
) => (Pat::Str("unsafe"), Pat::Str("}")),
672+
ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")),
673+
ExprKind::Field(e, name) => (inner(e, outer_span).0, Pat::Sym(name.name)),
674+
ExprKind::Index(e, _, _) => (inner(e, outer_span).0, Pat::Str("]")),
675+
ExprKind::Path(None, path) => ast_path_search_pat(path),
676+
ExprKind::Path(Some(_), path) => (Pat::Str("<"), ast_path_search_pat(path).1),
677+
ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), inner(e, outer_span).1),
678+
ExprKind::Break(None, None) => (Pat::Str("break"), Pat::Str("break")),
679+
ExprKind::Break(Some(name), None) => (Pat::Str("break"), Pat::Sym(name.ident.name)),
680+
ExprKind::Break(_, Some(e)) => (Pat::Str("break"), inner(e, outer_span).1),
681+
ExprKind::Continue(None) => (Pat::Str("continue"), Pat::Str("continue")),
682+
ExprKind::Continue(Some(name)) => (Pat::Str("continue"), Pat::Sym(name.ident.name)),
683+
ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")),
684+
ExprKind::Ret(Some(e)) => (Pat::Str("return"), inner(e, outer_span).1),
685+
ExprKind::Become(e) => (Pat::Str("become"), inner(e, outer_span).1),
686+
ExprKind::Struct(box StructExpr { path, .. }) => (ast_path_search_pat(path).0, Pat::Str("")),
687+
ExprKind::Use(e, _) => (inner(e, outer_span).0, Pat::Str("use")),
688+
ExprKind::Yield(YieldKind::Prefix(_)) => (Pat::Str("yield"), Pat::Str("")),
689+
ExprKind::Yield(YieldKind::Postfix(_)) => (inner(e, outer_span).0, Pat::Str("")),
690+
ExprKind::OffsetOf(..)
691+
// Syntax unstable
692+
| ExprKind::Yeet(_) | ExprKind::UnsafeBinderCast(..)
693+
// Don't have a good `Pat` for `ByteSymbol`s
694+
| ExprKind::IncludedBytes(_)
695+
// We don't know how qualified the path to the macro was
696+
| ExprKind::FormatArgs(_) | ExprKind::InlineAsm(..)
697+
// Should've been expanded by now (?)
698+
| ExprKind::MacCall(..)
699+
// Dummy/placeholder
700+
| ExprKind::Err(_) | ExprKind::Dummy => (Pat::Str(""), Pat::Str("")),
701+
}
702+
}
703+
704+
inner(e, e.span)
705+
}
706+
707+
fn range_limits_search_pat(lims: &RangeLimits) -> (Pat, Pat) {
708+
match lims {
709+
RangeLimits::HalfOpen => (Pat::Str(".."), Pat::Str("..")),
710+
RangeLimits::Closed => (Pat::Str("..="), Pat::Str("..=")),
711+
}
712+
}
713+
540714
fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
541715
(Pat::Sym(ident.name), Pat::Sym(ident.name))
542716
}
@@ -572,6 +746,7 @@ impl_with_search_pat!((_cx: LateContext<'tcx>, self: Path<'_>) => path_search_pa
572746

573747
impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: Attribute) => attr_search_pat(self));
574748
impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: ast::Ty) => ast_ty_search_pat(self));
749+
impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: ast::Expr) => ast_expr_search_pat(self));
575750

576751
impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
577752
type Context = LateContext<'cx>;

tests/ui/auxiliary/macro_rules.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,10 @@ macro_rules! bad_transmute {
5757
std::mem::transmute($e)
5858
};
5959
}
60+
61+
#[macro_export]
62+
macro_rules! double_parens {
63+
($a:expr, $b:expr, $c:expr, $d:expr) => {
64+
InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32)
65+
};
66+
}

tests/ui/double_parens.fixed

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
//@aux-build:proc_macros.rs
2+
//@aux-build:macro_rules.rs
13
#![warn(clippy::double_parens)]
24
#![expect(clippy::eq_op, clippy::no_effect)]
35
#![feature(custom_inner_attributes)]
46
#![rustfmt::skip]
57

8+
use proc_macros::{external, with_span};
9+
610
fn dummy_fn<T>(_: T) {}
711

812
struct DummyStruct;
@@ -96,4 +100,35 @@ fn issue9000(x: DummyStruct) {
96100
//~^ double_parens
97101
}
98102

103+
fn issue15892() {
104+
use macro_rules::double_parens;
105+
106+
#[repr(transparent)]
107+
#[derive(Clone, Copy, PartialEq, Eq)]
108+
pub struct InterruptMask(u32);
109+
110+
impl InterruptMask {
111+
pub const OE: InterruptMask = InterruptMask(1 << 10);
112+
pub const BE: InterruptMask = InterruptMask(1 << 9);
113+
pub const PE: InterruptMask = InterruptMask(1 << 8);
114+
pub const FE: InterruptMask = InterruptMask(1 << 7);
115+
pub const E: InterruptMask = double_parens!(Self::OE, Self::BE, Self::PE, Self::FE);
116+
#[allow(clippy::unnecessary_cast)]
117+
pub const F: InterruptMask = external!(
118+
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
119+
);
120+
#[allow(clippy::unnecessary_cast)]
121+
pub const G: InterruptMask = with_span!(span
122+
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
123+
);
124+
pub const fn into_bits(self) -> u32 {
125+
self.0
126+
}
127+
#[must_use]
128+
pub const fn union(self, rhs: Self) -> Self {
129+
InterruptMask(self.0 | rhs.0)
130+
}
131+
}
132+
}
133+
99134
fn main() {}

tests/ui/double_parens.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
//@aux-build:proc_macros.rs
2+
//@aux-build:macro_rules.rs
13
#![warn(clippy::double_parens)]
24
#![expect(clippy::eq_op, clippy::no_effect)]
35
#![feature(custom_inner_attributes)]
46
#![rustfmt::skip]
57

8+
use proc_macros::{external, with_span};
9+
610
fn dummy_fn<T>(_: T) {}
711

812
struct DummyStruct;
@@ -96,4 +100,35 @@ fn issue9000(x: DummyStruct) {
96100
//~^ double_parens
97101
}
98102

103+
fn issue15892() {
104+
use macro_rules::double_parens;
105+
106+
#[repr(transparent)]
107+
#[derive(Clone, Copy, PartialEq, Eq)]
108+
pub struct InterruptMask(u32);
109+
110+
impl InterruptMask {
111+
pub const OE: InterruptMask = InterruptMask(1 << 10);
112+
pub const BE: InterruptMask = InterruptMask(1 << 9);
113+
pub const PE: InterruptMask = InterruptMask(1 << 8);
114+
pub const FE: InterruptMask = InterruptMask(1 << 7);
115+
pub const E: InterruptMask = double_parens!(Self::OE, Self::BE, Self::PE, Self::FE);
116+
#[allow(clippy::unnecessary_cast)]
117+
pub const F: InterruptMask = external!(
118+
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
119+
);
120+
#[allow(clippy::unnecessary_cast)]
121+
pub const G: InterruptMask = with_span!(span
122+
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
123+
);
124+
pub const fn into_bits(self) -> u32 {
125+
self.0
126+
}
127+
#[must_use]
128+
pub const fn union(self, rhs: Self) -> Self {
129+
InterruptMask(self.0 | rhs.0)
130+
}
131+
}
132+
}
133+
99134
fn main() {}

0 commit comments

Comments
 (0)