Skip to content

Commit e1ac7da

Browse files
committed
fix: useless_let_if_seq wrongly unmangled macros
1 parent 8a1dde1 commit e1ac7da

File tree

3 files changed

+42
-12
lines changed

3 files changed

+42
-12
lines changed

clippy_lints/src/let_if_seq.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::res::MaybeResPath;
3-
use clippy_utils::source::snippet;
3+
use clippy_utils::source::snippet_with_context;
44
use clippy_utils::visitors::is_local_used;
55
use rustc_errors::Applicability;
66
use rustc_hir as hir;
@@ -117,14 +117,16 @@ fn check_block_inner<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>,
117117
// FIXME: this should not suggest `mut` if we can detect that the variable is not
118118
// use mutably after the `if`
119119

120+
let mut applicability = Applicability::HasPlaceholders;
121+
let (cond_snip, _) = snippet_with_context(cx, cond.span, if_.span.ctxt(), "_", &mut applicability);
122+
let (value_snip, _) = snippet_with_context(cx, value.span, if_.span.ctxt(), "<value>", &mut applicability);
123+
let (default_snip, _) =
124+
snippet_with_context(cx, default.span, if_.span.ctxt(), "<default>", &mut applicability);
120125
let sug = format!(
121-
"let {mutability}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
126+
"let {mutability}{name} = if {cond_snip} {{{then} {value_snip} }} else {{{else} {default_snip} }};",
122127
name=ident.name,
123-
cond=snippet(cx, cond.span, "_"),
124128
then=if then.stmts.len() > 1 { " ..;" } else { "" },
125129
else=if default_multi_stmts { " ..;" } else { "" },
126-
value=snippet(cx, value.span, "<value>"),
127-
default=snippet(cx, default.span, "<default>"),
128130
);
129131
span_lint_hir_and_then(
130132
cx,
@@ -133,12 +135,7 @@ fn check_block_inner<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>,
133135
span,
134136
"`if _ { .. } else { .. }` is an expression",
135137
|diag| {
136-
diag.span_suggestion(
137-
span,
138-
"it is more idiomatic to write",
139-
sug,
140-
Applicability::HasPlaceholders,
141-
);
138+
diag.span_suggestion(span, "it is more idiomatic to write", sug, applicability);
142139
if !mutability.is_empty() {
143140
diag.note("you might not need `mut` at all");
144141
}

tests/ui/let_if_seq.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,24 @@ fn issue16062(bar: fn() -> bool) {
149149
foo = 0;
150150
}
151151
}
152+
153+
fn issue16064(bar: fn() -> bool) {
154+
macro_rules! mac {
155+
($e:expr) => {
156+
$e()
157+
};
158+
($base:expr, $lit:expr) => {
159+
$lit * $base + 2
160+
};
161+
}
162+
163+
let foo;
164+
//~^ useless_let_if_seq
165+
if mac!(bar) {
166+
foo = mac!(10, 4);
167+
} else {
168+
foo = 0;
169+
}
170+
171+
let bar = 1;
172+
}

tests/ui/let_if_seq.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,17 @@ LL | | foo = 0;
6464
LL | | }
6565
| |_____^ help: it is more idiomatic to write: `let foo = if bar() { 42 } else { 0 };`
6666

67-
error: aborting due to 5 previous errors
67+
error: `if _ { .. } else { .. }` is an expression
68+
--> tests/ui/let_if_seq.rs:163:5
69+
|
70+
LL | / let foo;
71+
LL | |
72+
LL | | if mac!(bar) {
73+
LL | | foo = mac!(10, 4);
74+
LL | | } else {
75+
LL | | foo = 0;
76+
LL | | }
77+
| |_____^ help: it is more idiomatic to write: `let foo = if mac!(bar) { mac!(10, 4) } else { 0 };`
78+
79+
error: aborting due to 6 previous errors
6880

0 commit comments

Comments
 (0)