Skip to content

Commit 99d85f8

Browse files
y21y21
committed
deal with differing syntax contexts for subexpressions in check_proc_macro
1 parent c8725f7 commit 99d85f8

File tree

3 files changed

+84
-57
lines changed

3 files changed

+84
-57
lines changed

‎clippy_utils/src/check_proc_macro.rs

+82-55
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_lint::{LateContext, LintContext};
2525
use rustc_middle::ty::TyCtxt;
2626
use rustc_session::Session;
2727
use rustc_span::symbol::{Ident, kw};
28-
use rustc_span::{Span, Symbol};
28+
use rustc_span::{Span, Symbol, SyntaxContext};
2929
use rustc_target::spec::abi::Abi;
3030

3131
/// The search pattern to look for. Used by `span_matches_pat`
@@ -141,62 +141,89 @@ fn path_search_pat(path: &Path<'_>) -> (Pat, Pat) {
141141

142142
/// Get the search patterns to use for the given expression
143143
fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) {
144-
match e.kind {
145-
ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")),
146-
// Parenthesis are trimmed from the text before the search patterns are matched.
147-
// See: `span_matches_pat`
148-
ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
149-
ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat(tcx, e).1),
150-
ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat(tcx, e).1),
151-
ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), expr_search_pat(tcx, e).1),
152-
ExprKind::Lit(lit) => lit_search_pat(&lit.node),
153-
ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")),
154-
ExprKind::Call(e, []) | ExprKind::MethodCall(_, e, [], _) => (expr_search_pat(tcx, e).0, Pat::Str("(")),
155-
ExprKind::Call(first, [.., last])
156-
| ExprKind::MethodCall(_, first, [.., last], _)
157-
| ExprKind::Binary(_, first, last)
158-
| ExprKind::Tup([first, .., last])
159-
| ExprKind::Assign(first, last, _)
160-
| ExprKind::AssignOp(_, first, last) => (expr_search_pat(tcx, first).0, expr_search_pat(tcx, last).1),
161-
ExprKind::Tup([e]) | ExprKind::DropTemps(e) => expr_search_pat(tcx, e),
162-
ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (expr_search_pat(tcx, e).0, Pat::Str("")),
163-
ExprKind::Let(let_expr) => (Pat::Str("let"), expr_search_pat(tcx, let_expr.init).1),
164-
ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")),
165-
ExprKind::Loop(_, Some(_), _, _) | ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")),
166-
ExprKind::Loop(_, None, LoopSource::Loop, _) => (Pat::Str("loop"), Pat::Str("}")),
167-
ExprKind::Loop(_, None, LoopSource::While, _) => (Pat::Str("while"), Pat::Str("}")),
168-
ExprKind::Loop(_, None, LoopSource::ForLoop, _) | ExprKind::Match(_, _, MatchSource::ForLoopDesugar) => {
169-
(Pat::Str("for"), Pat::Str("}"))
170-
},
171-
ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")),
172-
ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => (expr_search_pat(tcx, e).0, Pat::Str("?")),
173-
ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => {
174-
(expr_search_pat(tcx, e).0, Pat::Str("await"))
175-
},
176-
ExprKind::Closure(&Closure { body, .. }) => (Pat::Str(""), expr_search_pat(tcx, tcx.hir().body(body).value).1),
177-
ExprKind::Block(
178-
Block {
179-
rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
180-
..
144+
fn expr_search_pat_inner(tcx: TyCtxt<'_>, e: &Expr<'_>, outer_ctxt: SyntaxContext) -> (Pat, Pat) {
145+
// The expression can have subexpressions in different contexts, in which case
146+
// building up a search pattern from the macro expansion would lead to false positives;
147+
// e.g. `return format!(..)` would be considered to be from a proc macro
148+
// if we build up a pattern for the macro expansion and compare it to the invocation `format!()`.
149+
// So instead we return an empty pattern such that `span_matches_pat` always returns true.
150+
if e.span.ctxt() != outer_ctxt {
151+
return (Pat::Str(""), Pat::Str(""));
152+
}
153+
154+
match e.kind {
155+
ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")),
156+
// Parenthesis are trimmed from the text before the search patterns are matched.
157+
// See: `span_matches_pat`
158+
ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
159+
ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
160+
ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
161+
ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
162+
ExprKind::Lit(lit) => lit_search_pat(&lit.node),
163+
ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")),
164+
ExprKind::Call(e, []) | ExprKind::MethodCall(_, e, [], _) => {
165+
(expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Str("("))
181166
},
182-
None,
183-
) => (Pat::Str("unsafe"), Pat::Str("}")),
184-
ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")),
185-
ExprKind::Field(e, name) => (expr_search_pat(tcx, e).0, Pat::Sym(name.name)),
186-
ExprKind::Index(e, _, _) => (expr_search_pat(tcx, e).0, Pat::Str("]")),
187-
ExprKind::Path(ref path) => qpath_search_pat(path),
188-
ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat(tcx, e).1),
189-
ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")),
190-
ExprKind::Break(Destination { label: Some(name), .. }, None) => (Pat::Str("break"), Pat::Sym(name.ident.name)),
191-
ExprKind::Break(_, Some(e)) => (Pat::Str("break"), expr_search_pat(tcx, e).1),
192-
ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")),
193-
ExprKind::Continue(Destination { label: Some(name), .. }) => (Pat::Str("continue"), Pat::Sym(name.ident.name)),
194-
ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")),
195-
ExprKind::Ret(Some(e)) => (Pat::Str("return"), expr_search_pat(tcx, e).1),
196-
ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")),
197-
ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat(tcx, e).1),
198-
_ => (Pat::Str(""), Pat::Str("")),
167+
ExprKind::Call(first, [.., last])
168+
| ExprKind::MethodCall(_, first, [.., last], _)
169+
| ExprKind::Binary(_, first, last)
170+
| ExprKind::Tup([first, .., last])
171+
| ExprKind::Assign(first, last, _)
172+
| ExprKind::AssignOp(_, first, last) => (
173+
expr_search_pat_inner(tcx, first, outer_ctxt).0,
174+
expr_search_pat_inner(tcx, last, outer_ctxt).1,
175+
),
176+
ExprKind::Tup([e]) | ExprKind::DropTemps(e) => expr_search_pat_inner(tcx, e, outer_ctxt),
177+
ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Str("")),
178+
ExprKind::Let(let_expr) => (Pat::Str("let"), expr_search_pat_inner(tcx, let_expr.init, outer_ctxt).1),
179+
ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")),
180+
ExprKind::Loop(_, Some(_), _, _) | ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")),
181+
ExprKind::Loop(_, None, LoopSource::Loop, _) => (Pat::Str("loop"), Pat::Str("}")),
182+
ExprKind::Loop(_, None, LoopSource::While, _) => (Pat::Str("while"), Pat::Str("}")),
183+
ExprKind::Loop(_, None, LoopSource::ForLoop, _) | ExprKind::Match(_, _, MatchSource::ForLoopDesugar) => {
184+
(Pat::Str("for"), Pat::Str("}"))
185+
},
186+
ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")),
187+
ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => {
188+
(expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Str("?"))
189+
},
190+
ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => {
191+
(expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Str("await"))
192+
},
193+
ExprKind::Closure(&Closure { body, .. }) => (
194+
Pat::Str(""),
195+
expr_search_pat_inner(tcx, tcx.hir().body(body).value, outer_ctxt).1,
196+
),
197+
ExprKind::Block(
198+
Block {
199+
rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
200+
..
201+
},
202+
None,
203+
) => (Pat::Str("unsafe"), Pat::Str("}")),
204+
ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")),
205+
ExprKind::Field(e, name) => (expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Sym(name.name)),
206+
ExprKind::Index(e, _, _) => (expr_search_pat_inner(tcx, e, outer_ctxt).0, Pat::Str("]")),
207+
ExprKind::Path(ref path) => qpath_search_pat(path),
208+
ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
209+
ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")),
210+
ExprKind::Break(Destination { label: Some(name), .. }, None) => {
211+
(Pat::Str("break"), Pat::Sym(name.ident.name))
212+
},
213+
ExprKind::Break(_, Some(e)) => (Pat::Str("break"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
214+
ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")),
215+
ExprKind::Continue(Destination { label: Some(name), .. }) => {
216+
(Pat::Str("continue"), Pat::Sym(name.ident.name))
217+
},
218+
ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")),
219+
ExprKind::Ret(Some(e)) => (Pat::Str("return"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
220+
ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")),
221+
ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat_inner(tcx, e, outer_ctxt).1),
222+
_ => (Pat::Str(""), Pat::Str("")),
223+
}
199224
}
225+
226+
expr_search_pat_inner(tcx, e, e.span.ctxt())
200227
}
201228

202229
fn fn_header_search_pat(header: FnHeader) -> Pat {

‎tests/ui/dbg_macro/dbg_macro.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::dbg_macro)]
2-
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
2+
#![allow(clippy::unnecessary_operation, clippy::no_effect, clippy::unit_arg)]
33

44
fn foo(n: u32) -> u32 {
55
if let Some(n) = n.checked_sub(4) { n } else { n }

‎tests/ui/dbg_macro/dbg_macro.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::dbg_macro)]
2-
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
2+
#![allow(clippy::unnecessary_operation, clippy::no_effect, clippy::unit_arg)]
33

44
fn foo(n: u32) -> u32 {
55
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }

0 commit comments

Comments
 (0)