Skip to content

Commit 71140d8

Browse files
committed
Support struct patterns
1 parent d8226f8 commit 71140d8

File tree

3 files changed

+111
-6
lines changed

3 files changed

+111
-6
lines changed

clippy_lints/src/manual_let_else.rs

+29-2
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ fn emit_manual_let_else(
147147
"this could be rewritten as `let...else`",
148148
|diag| {
149149
// This is far from perfect, for example there needs to be:
150-
// * tracking for multi-binding cases: let (foo, bar) = if let (Some(foo), Ok(bar)) = ...
151-
// * renamings of the bindings for many `PatKind`s like structs, slices, etc.
150+
// * renamings of the bindings for many `PatKind`s like slices, etc.
151+
// * limitations in the existing replacement algorithms
152152
// * unused binding collision detection with existing ones
153153
// for this to be machine applicable.
154154
let mut app = Applicability::HasPlaceholders;
@@ -227,6 +227,33 @@ fn replace_in_pattern(
227227
}
228228
return or_pat;
229229
},
230+
PatKind::Struct(path, fields, has_dot_dot) => {
231+
let fields = fields
232+
.iter()
233+
.map(|fld| {
234+
if let PatKind::Binding(_, _, name, None) = fld.pat.kind &&
235+
let Some(pat_to_put) = ident_map.get(&name.name)
236+
{
237+
let (sn_fld_name, _) = snippet_with_context(cx, fld.ident.span, span.ctxt(), "", app);
238+
let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app);
239+
// TODO: this is a bit of a hack, but it does its job. Ideally, we'd check if pat_to_put is
240+
// a PatKind::Binding but that is also hard to get right.
241+
if sn_fld_name == sn_ptp {
242+
// Field init shorthand
243+
return format!("{sn_fld_name}");
244+
}
245+
return format!("{sn_fld_name}: {sn_ptp}");
246+
}
247+
let (sn_fld, _) = snippet_with_context(cx, fld.span, span.ctxt(), "", app);
248+
sn_fld.into_owned()
249+
})
250+
.collect::<Vec<_>>();
251+
let fields_string = fields.join(", ");
252+
253+
let dot_dot_str = if has_dot_dot { " .." } else { "" };
254+
let (sn_pth, _) = snippet_with_context(cx, path.span(), span.ctxt(), "", app);
255+
return format!("{sn_pth} {{ {fields_string}{dot_dot_str} }}");
256+
},
230257
// Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.
231258
PatKind::TupleStruct(ref w, args, dot_dot_pos) => {
232259
let mut args = args

tests/ui/manual_let_else.rs

+29-2
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,27 @@ fn fire() {
163163

164164
// () is preserved: a bit of an edge case but make sure it stays around
165165
let w = if let (Some(v), ()) = (g(), ()) { v } else { return };
166+
167+
// Tuple structs work
168+
let w = if let Some(S { v: x }) = Some(S { v: 0 }) {
169+
x
170+
} else {
171+
return;
172+
};
173+
174+
// Field init shorthand is suggested
175+
let v = if let Some(S { v: x }) = Some(S { v: 0 }) {
176+
x
177+
} else {
178+
return;
179+
};
180+
181+
// Multi-field structs also work
182+
let (x, S { v }, w) = if let Some(U { v, w, x }) = None::<U<S<()>>> {
183+
(x, v, w)
184+
} else {
185+
return;
186+
};
166187
}
167188

168189
fn not_fire() {
@@ -298,6 +319,12 @@ fn not_fire() {
298319
};
299320
}
300321

301-
struct S {
302-
v: u32,
322+
struct S<T> {
323+
v: T,
324+
}
325+
326+
struct U<T> {
327+
v: T,
328+
w: T,
329+
x: T,
303330
}

tests/ui/manual_let_else.stderr

+53-2
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,64 @@ LL | let w = if let (Some(v), ()) = (g(), ()) { v } else { return };
301301
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let (Some(w), ()) = (g(), ()) else { return };`
302302

303303
error: this could be rewritten as `let...else`
304-
--> $DIR/manual_let_else.rs:275:5
304+
--> $DIR/manual_let_else.rs:168:5
305+
|
306+
LL | / let w = if let Some(S { v: x }) = Some(S { v: 0 }) {
307+
LL | | x
308+
LL | | } else {
309+
LL | | return;
310+
LL | | };
311+
| |______^
312+
|
313+
help: consider writing
314+
|
315+
LL ~ let Some(S { v: w }) = Some(S { v: 0 }) else {
316+
LL + return;
317+
LL + };
318+
|
319+
320+
error: this could be rewritten as `let...else`
321+
--> $DIR/manual_let_else.rs:175:5
322+
|
323+
LL | / let v = if let Some(S { v: x }) = Some(S { v: 0 }) {
324+
LL | | x
325+
LL | | } else {
326+
LL | | return;
327+
LL | | };
328+
| |______^
329+
|
330+
help: consider writing
331+
|
332+
LL ~ let Some(S { v }) = Some(S { v: 0 }) else {
333+
LL + return;
334+
LL + };
335+
|
336+
337+
error: this could be rewritten as `let...else`
338+
--> $DIR/manual_let_else.rs:182:5
339+
|
340+
LL | / let (x, S { v }, w) = if let Some(U { v, w, x }) = None::<U<S<()>>> {
341+
LL | | (x, v, w)
342+
LL | | } else {
343+
LL | | return;
344+
LL | | };
345+
| |______^
346+
|
347+
help: consider writing
348+
|
349+
LL ~ let Some(U { v: S { v }, w, x }) = None::<U<S<()>>> else {
350+
LL + return;
351+
LL + };
352+
|
353+
354+
error: this could be rewritten as `let...else`
355+
--> $DIR/manual_let_else.rs:296:5
305356
|
306357
LL | / let _ = match ff {
307358
LL | | Some(value) => value,
308359
LL | | _ => macro_call!(),
309360
LL | | };
310361
| |______^ help: consider writing: `let Some(_) = ff else { macro_call!() };`
311362

312-
error: aborting due to 23 previous errors
363+
error: aborting due to 26 previous errors
313364

0 commit comments

Comments
 (0)