Some improvements to the manual_let_else lint suggestions
* Replace variables inside | patterns in the if let: let v = if let V::A(v) | V::B(v) = v { v } else ...
* Support nested patterns: let v = if let Ok(Ok(Ok(v))) = v { v } else ...
* Support tuple structs with more than one arg: let v = V::W(v, _) = v { v } else ...
* Correctly handle .. in tuple struct patterns: let v = V::X(v, ..) = v { v } else ...
This commit is contained in:
est31
parent
83a4b0987f
commit
ef38662d04
@ -146,10 +146,9 @@ fn emit_manual_let_else(
|
||||
"this could be rewritten as `let...else`",
|
||||
|diag| {
|
||||
// This is far from perfect, for example there needs to be:
|
||||
// * mut additions for the bindings
|
||||
// * renamings of the bindings for `PatKind::Or`
|
||||
// * tracking for multi-binding cases: let (foo, bar) = if let (Some(foo), Ok(bar)) = ...
|
||||
// * renamings of the bindings for many `PatKind`s like structs, slices, etc.
|
||||
// * unused binding collision detection with existing ones
|
||||
// * putting patterns with at the top level | inside ()
|
||||
// for this to be machine applicable.
|
||||
let mut app = Applicability::HasPlaceholders;
|
||||
let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app);
|
||||
@ -160,28 +159,62 @@ fn emit_manual_let_else(
|
||||
} else {
|
||||
format!("{{ {sn_else} }}")
|
||||
};
|
||||
let sn_bl = match pat.kind {
|
||||
PatKind::Or(..) => {
|
||||
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
|
||||
format!("({sn_pat})")
|
||||
},
|
||||
// Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.
|
||||
PatKind::TupleStruct(ref w, args, ..) if args.len() == 1 => {
|
||||
let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();
|
||||
let (sn_inner, _) = snippet_with_context(cx, local.span, span.ctxt(), "", &mut app);
|
||||
format!("{sn_wrapper}({sn_inner})")
|
||||
},
|
||||
_ => {
|
||||
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
|
||||
sn_pat.into_owned()
|
||||
},
|
||||
};
|
||||
let sn_bl = replace_in_pattern(cx, span, local, pat, &mut app);
|
||||
let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};");
|
||||
diag.span_suggestion(span, "consider writing", sugg, app);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
// replaces the locals in the pattern
|
||||
fn replace_in_pattern(
|
||||
cx: &LateContext<'_>,
|
||||
span: Span,
|
||||
local: &Pat<'_>,
|
||||
pat: &Pat<'_>,
|
||||
app: &mut Applicability,
|
||||
) -> String {
|
||||
let mut bindings_count = 0;
|
||||
pat.each_binding_or_first(&mut |_, _, _, _| bindings_count += 1);
|
||||
// If the pattern creates multiple bindings, exit early,
|
||||
// as otherwise we might paste the pattern to the positions of multiple bindings.
|
||||
if bindings_count > 1 {
|
||||
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app);
|
||||
return sn_pat.into_owned();
|
||||
}
|
||||
|
||||
match pat.kind {
|
||||
PatKind::Binding(..) => {
|
||||
let (sn_bdg, _) = snippet_with_context(cx, local.span, span.ctxt(), "", app);
|
||||
return sn_bdg.to_string();
|
||||
},
|
||||
PatKind::Or(pats) => {
|
||||
let patterns = pats
|
||||
.iter()
|
||||
.map(|pat| replace_in_pattern(cx, span, local, pat, app))
|
||||
.collect::<Vec<_>>();
|
||||
let or_pat = patterns.join(" | ");
|
||||
return format!("({or_pat})");
|
||||
},
|
||||
// Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.
|
||||
PatKind::TupleStruct(ref w, args, dot_dot_pos) => {
|
||||
let mut args = args
|
||||
.iter()
|
||||
.map(|pat| replace_in_pattern(cx, span, local, pat, app))
|
||||
.collect::<Vec<_>>();
|
||||
if let Some(pos) = dot_dot_pos.as_opt_usize() {
|
||||
args.insert(pos, "..".to_owned());
|
||||
}
|
||||
let args = args.join(", ");
|
||||
let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();
|
||||
return format!("{sn_wrapper}({args})");
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app);
|
||||
sn_pat.into_owned()
|
||||
}
|
||||
|
||||
/// Check whether an expression is divergent. May give false negatives.
|
||||
fn expr_diverges(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
struct V<'cx, 'tcx> {
|
||||
|
||||
@ -146,10 +146,20 @@ fn e() -> Variant {
|
||||
Variant::A(0, 0)
|
||||
}
|
||||
|
||||
// Should not be renamed
|
||||
let v = if let Variant::A(a, 0) = e() { a } else { return };
|
||||
// Should be renamed
|
||||
let v = if let Variant::B(b) = e() { b } else { return };
|
||||
|
||||
// `mut v` is inserted into the pattern
|
||||
let mut v = if let Variant::B(b) = e() { b } else { return };
|
||||
|
||||
// Nesting works
|
||||
let nested = Ok(Some(e()));
|
||||
let v = if let Ok(Some(Variant::B(b))) | Err(Some(Variant::A(b, _))) = nested {
|
||||
b
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
// dot dot works
|
||||
let v = if let Variant::A(.., a) = e() { a } else { return };
|
||||
}
|
||||
|
||||
fn not_fire() {
|
||||
|
||||
@ -260,19 +260,42 @@ LL | create_binding_if_some!(w, g());
|
||||
= note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info)
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else.rs:150:5
|
||||
--> $DIR/manual_let_else.rs:149:5
|
||||
|
|
||||
LL | let v = if let Variant::A(a, 0) = e() { a } else { return };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::A(a, 0) = e() else { return };`
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::A(v, 0) = e() else { return };`
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else.rs:152:5
|
||||
|
|
||||
LL | let v = if let Variant::B(b) = e() { b } else { return };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::B(v) = e() else { return };`
|
||||
LL | let mut v = if let Variant::B(b) = e() { b } else { return };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::B(mut v) = e() else { return };`
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else.rs:262:5
|
||||
--> $DIR/manual_let_else.rs:156:5
|
||||
|
|
||||
LL | / let v = if let Ok(Some(Variant::B(b))) | Err(Some(Variant::A(b, _))) = nested {
|
||||
LL | | b
|
||||
LL | | } else {
|
||||
LL | | return;
|
||||
LL | | };
|
||||
| |______^
|
||||
|
|
||||
help: consider writing
|
||||
|
|
||||
LL ~ let (Ok(Some(Variant::B(v))) | Err(Some(Variant::A(v, _)))) = nested else {
|
||||
LL + return;
|
||||
LL + };
|
||||
|
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else.rs:162:5
|
||||
|
|
||||
LL | let v = if let Variant::A(.., a) = e() { a } else { return };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::A(.., v) = e() else { return };`
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else.rs:272:5
|
||||
|
|
||||
LL | / let _ = match ff {
|
||||
LL | | Some(value) => value,
|
||||
@ -280,5 +303,5 @@ LL | | _ => macro_call!(),
|
||||
LL | | };
|
||||
| |______^ help: consider writing: `let Some(_) = ff else { macro_call!() };`
|
||||
|
||||
error: aborting due to 20 previous errors
|
||||
error: aborting due to 22 previous errors
|
||||
|
||||
|
||||
@ -68,7 +68,7 @@ fn fire() {
|
||||
let f = Variant::Bar(1);
|
||||
|
||||
let _value = match f {
|
||||
Variant::Bar(_) | Variant::Baz(_) => (),
|
||||
Variant::Bar(v) | Variant::Baz(v) => v,
|
||||
_ => return,
|
||||
};
|
||||
|
||||
|
||||
@ -58,10 +58,10 @@ error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else_match.rs:70:5
|
||||
|
|
||||
LL | / let _value = match f {
|
||||
LL | | Variant::Bar(_) | Variant::Baz(_) => (),
|
||||
LL | | Variant::Bar(v) | Variant::Baz(v) => v,
|
||||
LL | | _ => return,
|
||||
LL | | };
|
||||
| |______^ help: consider writing: `let (Variant::Bar(_) | Variant::Baz(_)) = f else { return };`
|
||||
| |______^ help: consider writing: `let (Variant::Bar(_value) | Variant::Baz(_value)) = f else { return };`
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else_match.rs:76:5
|
||||
|
||||
Loading…
Reference in New Issue
Block a user