Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
9855: feature: Destructure Tuple Assist r=Veykril a=Booksbaum Part of #8673. This PR only handles tuples, not TupleStruct and RecordStruct. Code Assist to destructure a tuple into its items: ![Destructure_Tuple_Assist](https://user-images.githubusercontent.com/15612932/129020107-775d7c94-dca7-4d1f-a0a2-cd63cabf4132.gif) * Should work in nearly all pattern positions, like let assignment, function parameters, match arms, for loops, and nested variables (`if let Some($0t) = Some((1,2))`) -> everywhere `IdentPat` is allowed * Exception: If there's a sub-pattern (``@`):` ```rust if let t @ (1..=3, 1..=3) = ... {} // ^ ``` -> `t` must be a `Name`; `TuplePat` (`(_0, _1)`) isn't allowed * inside subpattern is ok: ```rust let t @ (a, _) = ((1,2), 3); // ^ ``` -> ```rust let t @ ((_0, _1), _) = ((1,2), 3); ``` * Assist triggers only at tuple declaration, not tuple usage. (might be useful especially when it creates a sub-pattern (after ``@`)` and only changes the usage under cursor -- but not part of this PR). ### References References can be destructured: ```rust let t = &(1,2); // ^ let v = t.0; ``` -> ```rust let (_0, _1) = &(1,2); let v = _0; ``` BUT: `t.0` and `_0` have different types (`i32` vs. `&i32`) -> `v` has now a different type. I think that's acceptable: I think the destructure assist is mostly used in simple, immediate scopes and not huge existing code. Additional Notes: * `ref` has same behaviour (-> `ref` is kept for items) ```rust let ref t = (1,2); // ^ ``` -> ```rust let (ref _0, ref _1) = (1,2); ``` * Rust IntelliJ Plugin: doesn't trigger with `&` or `ref` at all ### mutable ```rust let mut t = (1,2); // ^ ``` -> ```rust let (mut _0, mut _1) = (1,2); ``` and ```rust let t = &mut (1,2); // ^ ``` -> ```rust let (_0, _1) = &mut (1,2); ``` Again: with reference (`&mut`), `t.0` and `_0` have different types (`i32` vs `&mut i32`). And there's an additional issue with `&mut` and assignment: ```rust let t = &mut (1,2); // ^ t.0 = 9; ``` -> ```rust let (_0, _1) = &mut (1,2); _0 = 9; // ^ // mismatched types // expected `&mut {integer}`, found integer // consider dereferencing here to assign to the mutable borrowed piece of memory ``` But I think that's quite a niche use case, so I don't catch that (`*_0 = 9;`) Additional Notes: * Rust IntelliJ Plugin: removes the `mut` (`let mut t = ...` -> `let (_0, _1) = ...`), doesn't trigger with `&mut` ### Binding after ``@`` Destructure tuple in sub-pattern is implemented: ```rust let t = (1,2); // ^ let v = t.0; let f = t.into(); ``` -> ```rust let t @ (_0, _1) = (1,2); let v = _0; let f = t.into(); ``` BUT: Bindings after ``@`` aren't currently in stable and require `#![feature(bindings_after_at)]` (though should be generally [available quite soon](rust-lang/rust#85305 (comment)) (with `1.56.0`)). But I don't know how to check for an enabled feature -> Destructure tuple in sub-pattern [isn't enabled](https://github.com/Booksbaum/rust-analyzer/blob/a4ee6c7954f910da3ca74fc0e25edda9077ad184/crates/ide_assists/src/handlers/destructure_tuple_binding.rs#L32) yet. * When Destructure in sub-pattern is enabled there are two assists: * `Destructure tuple in place`: ```rust let t = (1,2); // ^ ``` -> ```rust let (_0, _1) = (1,2); let v = _0; let f = /*t*/.into(); ``` * `Destructure tuple in sub-pattern`: ```rust let t = (1,2); // ^ let v = t.0; let f = t.into(); ``` -> ```rust let t @ (_0, _1) = (1,2); let v = _0; let f = t.into(); ``` * When Destructure in sub-pattern is disabled, only the first one is available and just named `Destructure tuple` <br/> <br/> ### Caveats * Unlike in #8673 or IntelliJ rust plugin, I'm not leaving the previous tuple name at function calls. **Reasoning**: It's not too unlikely the tuple variable shadows another variable. Destructuring the tuple while leaving the function call untouched, results in still a valid function call -- but now with another variable: ```rust let t = (8,9); let t = (1,2); // ^ t.into() ``` => Destructure Tuple ```rust let t = (8,9); let (_0, _1) = (1,2); t.into() ``` `t.into()` is still valid -- using the first tuple. Instead I comment out the tuple usage, which results in invalid code -> must be handled by user: ```rust /*t*/.into() ``` * (though that might be a biased decision: For testing I just declared a lot of `t`s and quite ofen in lines next to each other...) * Issue: there are some cases that results in still valid code: * macro that accept the tuple as well as no arguments: ```rust macro_rules! m { () => { "foo" }; ($e:expr) => { $e; "foo" }; } let t = (1,2); m!(t); m!(/*t*/); ``` -> both calls are valid ([test](https://github.com/Booksbaum/rust-analyzer/blob/a4ee6c7954f910da3ca74fc0e25edda9077ad184/crates/ide_assists/src/handlers/destructure_tuple_binding.rs#L1474)) * Probably with tuple as return value. Changing the return value most likely results in an error -- but in another place; not where the tuple usage was. -> not sure that's the best way.... Additional the tuple name surrounded by comment is more difficult to edit than just the name. * Code Assists don't support snippet placeholder, and rust analyzer just the first `$0` -> unfortunately no editing of generated tuple item variables. Cursor (`$0`) is placed on first generated item. <br/> <br/> ### Issues * Tuple index usage in macro calls aren't converted: ```rust let t = (1,2); // ^ let v = t.0; println!("{}", t.0); ``` -> ```rust let (_0, _1) = (1,2); let v = _0; println!("{}", /*t*/.0); ``` ([tests](https://github.com/Booksbaum/rust-analyzer/blob/a4ee6c7954f910da3ca74fc0e25edda9077ad184/crates/ide_assists/src/handlers/destructure_tuple_binding.rs#L1294)) * Issue is: [name.syntax()](https://github.com/Booksbaum/rust-analyzer/blob/a4ee6c7954f910da3ca74fc0e25edda9077ad184/crates/ide_assists/src/handlers/destructure_tuple_binding.rs#L242-L244) in each [usage](https://github.com/Booksbaum/rust-analyzer/blob/a4ee6c7954f910da3ca74fc0e25edda9077ad184/crates/ide_assists/src/handlers/destructure_tuple_binding.rs#L108-L113) of a tuple is syntax & text_range in its file. EXCEPT when tuple usage is in a macro call (`m!(t.0)`), the macro is expanded and syntax (and range) is based on that expanded macro, not in actual file. That leads to several things: * I cannot differentiate between calling the macro with the tuple or with tuple item: ```rust macro_rules! m { ($t:expr, $i:expr) => { $t.0 + $i }; } let t = (1,2); m!(t, t.0); ``` -> both `t` usages are resolved as tuple index usage * Range of resolved tuple index usage is in expanded macro, not in actual file -> don't know where to replace index usage -> tuple items passed into a macro are ignored, and only the tuple name itself is handled (uncommented) * I'm not checking if the generated names conflict with already existing variables. ```rust let _0 = 42; // >-| let t = (1,2); // | let v = _0; // <-| // ^ 42 ``` => deconstruct tuple ```rust let _0 = 42; let (_0, _1) = (1,2); // >-| let v = _0; // <-| // ^ now 1 ``` * I tried to get the scope at tuple declaration and its usages. And then iterate all names with [`process_all_names`](https://github.com/rust-analyzer/rust-analyzer/blob/145b51f9daf5371f1754c09eb2e3a77e0a24a0dc/crates/hir/src/semantics.rs#L935). But that doesn't find all local names for declarations (`let t = (1,2)`) (for usages it does) * This isn't unique to this Code Assist, but happen in others too (like `extract into variable` or `extract into function`). But here a name conflict is more likely (when destructuring multiple tuples, for examples nested ones (`let t = ((1,2),3)` -> `let (_0, _1) = ...` -> `let ((_0, _1), _1) = ...` -> error)) * IntelliJ rust plugin does handle this (-> name is `_00`) Co-authored-by: BooksBaum <15612932+Booksbaum@users.noreply.github.com>
- Loading branch information