Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add assist to turn let statements + match expression into let-else statements #11908

Closed
fmease opened this issue Apr 5, 2022 · 1 comment
Labels
A-assists C-feature Category: feature request

Comments

@fmease
Copy link
Member

fmease commented Apr 5, 2022

This is the counterpart to #11906 (assist for turning let-else statements to let statements + match expression).
Read about the unstable feature let_else. Rust Analyzer has basic support for it.

Motivation

  1. Once let-else statements get stabilized, users will want to transform some of their match expressions into let-else statements to reduce both visual noise and line count. This process is annoying since so many things have to be shifted around. Rust Analyzer should automate this.
  2. Refactoring assists should have an inverse. In this case of Feature request: Add assist to turn let-else statements into let statements + match expression #11906 (read its distinct motivation over there)

Transformation

Not all match expressions (…) can be expressed as let-else statements, they are a proper subset.
Just like with the assist Replace match with if let, this assist (Replace match with let else) should only trigger if the match has exactly two branches / arms (this is a conservative albeit pragmatic decision) and if only one introduces new bindings.
Ideally, it should only trigger if the branch that does not define any bindings (or the last branch if none do) diverges.
As far as I can tell the existing Replace match with if let assist chooses the branch that introduces bindings as the scrutinee of the if let (or the first branch if none do). Let's do the same for this assist.

let $all_binders_in_defining_pat_modulo_subst = match $expr {
    $defining_pat => $all_binders_in_defining_pat,
    $pat => $diverging_expr,
    // or
    // $pat => { $( $stmt );* $diverging_expr }
};

// ---->

let $unified_pat = $expr else {
    $( $stmt );*
    $diverging_expr
};

In most cases probably, $all_binders_in_defining_pat_modulo_subst is just an identifier and equal to $all_binders_in_defining_pat. We might want to restrict ourselves to this simple case for the MVP (but also support mut). See example (A).
In such an MVP, $unified_pat would just be $defining_pat unless $all_binders_in_defining_pat_modulo_subst is a mut identifier, then $unified_pat would be mut $unified_pat.

$all_binders_in_defining_pat has to be a tuple of identifiers, a single identifier or an empty block {} and has to contain all the binders in $defining_pat in order. This restriction prevents the assist from altering the program logic.

If we want to support more complex $defining_pats and $all_binders_in_defining_pat_modulo_substs, the two have to be unified applying substitutions. I am not sure if there is already any infrastructure for this in RA.

E.g. if $defining_pat is Some(name) and $all_binders_in_defining_pat_modulo_subst is (mut x, y), they somehow need to be unified to Some((mut x, y)) substituting name with the subpattern (mut x, y).

The assist should treat match statements as if they were match expressions inside of let () = , ; because the inverse assist (#11906) intentionally does not generate the redundant let () (for cosmetic reasons) but the user should still be able to keep switching back and forth between match and let-else. Obviously, this special-casing of () is a teeny-tiny artificial corner case. Feel free to drop that requirement.

Examples

(A) Super basic, probably the most common case where one might want to switch to let-else:

let name = match f() {
    Some(name) => name,
    None => return,
};

// ---->

let Some(name) = f() else { return };

(B) Basic, probably the second most common case (…) where the inner name does not match and where we have mutability:

let mut very_long_name = match f() {
    Some(name) => name,
    None => return,
};

// ---->

let Some(mut very_long_name) = f() else { return };    // irref subpattern `name` was subst. w/ `mut very_long_name`

(C) Let statement with a more complex pattern (is this out of scope?):

let (x, y) = match f() {
    Some(throwaway) => throwaway,
    None => return,
};

// ---->

let Some((x, y)) = f() else { return };    // irref subpattern `throwaway` was subst. w/ `(x, y)`

(C) (out of scope?):

let it = match f() {
    Some([_, (intermediate, _)]) => intermediate,
    _ => todo!(),
};

// ---->

let Some([_, (it, _)]) = f() else { todo!() };    // irref subpattern `intermediate` was subst. w/ `it`
@Veykril
Copy link
Member

Veykril commented Nov 7, 2022

Implemented in #13516

@Veykril Veykril closed this as completed Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists C-feature Category: feature request
Projects
None yet
Development

No branches or pull requests

2 participants