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

"Convert match to let-else" assist should handle non-trivial patterns #13540

Closed
jonas-schievink opened this issue Nov 3, 2022 · 3 comments · Fixed by #14057
Closed

"Convert match to let-else" assist should handle non-trivial patterns #13540

jonas-schievink opened this issue Nov 3, 2022 · 3 comments · Fixed by #14057
Labels
A-assists A-pattern pattern handling related things C-feature Category: feature request S-actionable Someone could pick this issue up and work on it right now

Comments

@jonas-schievink
Copy link
Contributor

Currently, for code like

let (x, y) = match Some((0, 1)) {
    Some(it) => it,
    None => todo!(),
};

The "Convert match to let-else" assist doesn't trigger, because it only works with identifier patterns, but here we have (x, y) as the pattern. This is a pretty arbitrary limitation, and the assist should be able to turn it into:

let Some((x,y)) = Some((0, 1)) else { todo!() };
@jonas-schievink jonas-schievink added S-actionable Someone could pick this issue up and work on it right now A-assists C-feature Category: feature request labels Nov 3, 2022
@jonas-schievink
Copy link
Contributor Author

It should also handle more complex expressions in the diverging branch (preceding non-diverging expressions for example)

@liamhession
Copy link

Hi @jonas-schievink - i'm hoping to try to contribute to this. After familiarizing myself with the contribution guide for rust-analyzer.... Would this issue involve solely making changes within https://github.com/rust-lang/rust-analyzer/blob/master/crates/ide-assists/src/handlers/convert_match_to_let_else.rs ?

Going out on a limb - the work involved would be to expand the scope of what the assist looks for in the AST to trigger it, so that a let statement like you identified would cause it to trigger. And then adding some tests? Cheers

@jonas-schievink
Copy link
Contributor Author

Yeah, that's right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists A-pattern pattern handling related things C-feature Category: feature request S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants