Skip to content

Commit a408e76

Browse files
koka831koka831
committed
Suggest the correct variable name for manual_let_else 
1 parent 75d7680 commit a408e76

File tree

3 files changed

+52
-43
lines changed

3 files changed

+52
-43
lines changed

‎clippy_lints/src/manual_let_else.rs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -68,29 +68,21 @@ impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]);
6868

6969
impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
7070
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) {
71-
let if_let_or_match = if_chain! {
72-
if self.msrv.meets(msrvs::LET_ELSE);
73-
if !in_external_macro(cx.sess(), stmt.span);
74-
if let StmtKind::Local(local) = stmt.kind;
75-
if let Some(init) = local.init;
76-
if local.els.is_none();
77-
if local.ty.is_none();
78-
if init.span.ctxt() == stmt.span.ctxt();
79-
if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init);
80-
then {
81-
if_let_or_match
82-
} else {
83-
return;
84-
}
85-
};
86-
71+
if self.msrv.meets(msrvs::LET_ELSE) &&
72+
!in_external_macro(cx.sess(), stmt.span) &&
73+
let StmtKind::Local(local) = stmt.kind &&
74+
let Some(init) = local.init &&
75+
local.els.is_none() &&
76+
local.ty.is_none() &&
77+
init.span.ctxt() == stmt.span.ctxt() &&
78+
let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) {
8779
match if_let_or_match {
8880
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
8981
if expr_is_simple_identity(let_pat, if_then);
9082
if let Some(if_else) = if_else;
9183
if expr_diverges(cx, if_else);
9284
then {
93-
emit_manual_let_else(cx, stmt.span, if_let_expr, let_pat, if_else);
85+
emit_manual_let_else(cx, stmt.span, if_let_expr, local.pat, let_pat, if_else);
9486
}
9587
},
9688
IfLetOrMatch::Match(match_expr, arms, source) => {
@@ -120,15 +112,23 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
120112
return;
121113
}
122114

123-
emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body);
115+
emit_manual_let_else(cx, stmt.span, match_expr, local.pat, pat_arm.pat, diverging_arm.body);
124116
},
125117
}
118+
};
126119
}
127120

128121
extract_msrv_attr!(LateContext);
129122
}
130123

131-
fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: &Pat<'_>, else_body: &Expr<'_>) {
124+
fn emit_manual_let_else(
125+
cx: &LateContext<'_>,
126+
span: Span,
127+
expr: &Expr<'_>,
128+
local: &Pat<'_>,
129+
pat: &Pat<'_>,
130+
else_body: &Expr<'_>,
131+
) {
132132
span_lint_and_then(
133133
cx,
134134
MANUAL_LET_ELSE,
@@ -137,12 +137,11 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat:
137137
|diag| {
138138
// This is far from perfect, for example there needs to be:
139139
// * mut additions for the bindings
140-
// * renamings of the bindings
140+
// * renamings of the bindings for `PatKind::Or`
141141
// * unused binding collision detection with existing ones
142142
// * putting patterns with at the top level | inside ()
143143
// for this to be machine applicable.
144144
let mut app = Applicability::HasPlaceholders;
145-
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
146145
let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app);
147146
let (sn_else, _) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app);
148147

@@ -151,10 +150,20 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat:
151150
} else {
152151
format!("{{ {sn_else} }}")
153152
};
154-
let sn_bl = if matches!(pat.kind, PatKind::Or(..)) {
155-
format!("({sn_pat})")
156-
} else {
157-
sn_pat.into_owned()
153+
let sn_bl = match pat.kind {
154+
PatKind::Or(..) => {
155+
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
156+
format!("({sn_pat})")
157+
},
158+
PatKind::TupleStruct(ref w, ..) => {
159+
let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();
160+
let (sn_inner, _) = snippet_with_context(cx, local.span, span.ctxt(), "", &mut app);
161+
format!("{sn_wrapper}({sn_inner})")
162+
},
163+
_ => {
164+
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
165+
sn_pat.into_owned()
166+
},
158167
};
159168
let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};");
160169
diag.span_suggestion(span, "consider writing", sugg, app);

‎tests/ui/manual_let_else.stderr

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: this could be rewritten as `let...else`
22
--> $DIR/manual_let_else.rs:18:5
33
|
44
LL | let v = if let Some(v_some) = g() { v_some } else { return };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { return };`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };`
66
|
77
= note: `-D clippy::manual-let-else` implied by `-D warnings`
88

@@ -18,7 +18,7 @@ LL | | };
1818
|
1919
help: consider writing
2020
|
21-
LL ~ let Some(v_some) = g() else {
21+
LL ~ let Some(v) = g() else {
2222
LL + return;
2323
LL + };
2424
|
@@ -48,19 +48,19 @@ error: this could be rewritten as `let...else`
4848
--> $DIR/manual_let_else.rs:38:9
4949
|
5050
LL | let v = if let Some(v_some) = g() { v_some } else { continue };
51-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { continue };`
51+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { continue };`
5252

5353
error: this could be rewritten as `let...else`
5454
--> $DIR/manual_let_else.rs:39:9
5555
|
5656
LL | let v = if let Some(v_some) = g() { v_some } else { break };
57-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { break };`
57+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { break };`
5858

5959
error: this could be rewritten as `let...else`
6060
--> $DIR/manual_let_else.rs:43:5
6161
|
6262
LL | let v = if let Some(v_some) = g() { v_some } else { panic!() };
63-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { panic!() };`
63+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { panic!() };`
6464

6565
error: this could be rewritten as `let...else`
6666
--> $DIR/manual_let_else.rs:46:5
@@ -74,7 +74,7 @@ LL | | };
7474
|
7575
help: consider writing
7676
|
77-
LL ~ let Some(v_some) = g() else {
77+
LL ~ let Some(v) = g() else {
7878
LL + std::process::abort()
7979
LL + };
8080
|
@@ -91,7 +91,7 @@ LL | | };
9191
|
9292
help: consider writing
9393
|
94-
LL ~ let Some(v_some) = g() else {
94+
LL ~ let Some(v) = g() else {
9595
LL + if true { return } else { panic!() }
9696
LL + };
9797
|
@@ -109,7 +109,7 @@ LL | | };
109109
|
110110
help: consider writing
111111
|
112-
LL ~ let Some(v_some) = g() else {
112+
LL ~ let Some(v) = g() else {
113113
LL + if true {}
114114
LL + panic!();
115115
LL + };
@@ -129,7 +129,7 @@ LL | | };
129129
|
130130
help: consider writing
131131
|
132-
LL ~ let Some(v_some) = g() else {
132+
LL ~ let Some(v) = g() else {
133133
LL + match () {
134134
LL + _ if panic!() => {},
135135
LL + _ => panic!(),
@@ -141,7 +141,7 @@ error: this could be rewritten as `let...else`
141141
--> $DIR/manual_let_else.rs:80:5
142142
|
143143
LL | let v = if let Some(v_some) = g() { v_some } else { if panic!() {} };
144-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { if panic!() {} };`
144+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { if panic!() {} };`
145145

146146
error: this could be rewritten as `let...else`
147147
--> $DIR/manual_let_else.rs:83:5
@@ -157,7 +157,7 @@ LL | | };
157157
|
158158
help: consider writing
159159
|
160-
LL ~ let Some(v_some) = g() else {
160+
LL ~ let Some(v) = g() else {
161161
LL + match panic!() {
162162
LL + _ => {},
163163
LL + }
@@ -178,7 +178,7 @@ LL | | };
178178
|
179179
help: consider writing
180180
|
181-
LL ~ let Some(v_some) = g() else { if true {
181+
LL ~ let Some(v) = g() else { if true {
182182
LL + return;
183183
LL + } else {
184184
LL + panic!("diverge");
@@ -199,7 +199,7 @@ LL | | };
199199
|
200200
help: consider writing
201201
|
202-
LL ~ let Some(v_some) = g() else {
202+
LL ~ let Some(v) = g() else {
203203
LL + match (g(), g()) {
204204
LL + (Some(_), None) => return,
205205
LL + (None, Some(_)) => {
@@ -226,7 +226,7 @@ LL | | };
226226
|
227227
help: consider writing
228228
|
229-
LL ~ let Some(v_some) = g().map(|v| (v, 42)) else {
229+
LL ~ let Some((v, w)) = g().map(|v| (v, 42)) else {
230230
LL + return;
231231
LL + };
232232
|
@@ -252,7 +252,7 @@ error: this could be rewritten as `let...else`
252252
--> $DIR/manual_let_else.rs:134:13
253253
|
254254
LL | let $n = if let Some(v) = $e { v } else { return };
255-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };`
255+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some($n) = g() else { return };`
256256
...
257257
LL | create_binding_if_some!(w, g());
258258
| ------------------------------- in this macro invocation
@@ -266,7 +266,7 @@ LL | / let _ = match ff {
266266
LL | | Some(value) => value,
267267
LL | | _ => macro_call!(),
268268
LL | | };
269-
| |______^ help: consider writing: `let Some(value) = ff else { macro_call!() };`
269+
| |______^ help: consider writing: `let Some(_) = ff else { macro_call!() };`
270270

271271
error: aborting due to 18 previous errors
272272

‎tests/ui/manual_let_else_match.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | / let v = match g() {
55
LL | | Some(v_some) => v_some,
66
LL | | None => return,
77
LL | | };
8-
| |______^ help: consider writing: `let Some(v_some) = g() else { return };`
8+
| |______^ help: consider writing: `let Some(v) = g() else { return };`
99
|
1010
= note: `-D clippy::manual-let-else` implied by `-D warnings`
1111

@@ -16,7 +16,7 @@ LL | / let v = match g() {
1616
LL | | Some(v_some) => v_some,
1717
LL | | _ => return,
1818
LL | | };
19-
| |______^ help: consider writing: `let Some(v_some) = g() else { return };`
19+
| |______^ help: consider writing: `let Some(v) = g() else { return };`
2020

2121
error: this could be rewritten as `let...else`
2222
--> $DIR/manual_let_else_match.rs:44:9

0 commit comments

Comments
 (0)