Destructure assist
- [x] Tuple destructuring (implemented in #9855)
- [ ] RecordStruct destructuring
- [ ] TupleStruct destructuring
It would be nice to have an assist for destructuring a binding into its "subbindings" as in, when you have a name to a tuple value have it destructure into names for each componenet of the tuple, same for structure fields etc. See IntelliJ's implementation here https://github.com/intellij-rust/intellij-rust/blob/master/src/main/kotlin/org/rust/ide/intentions/DestructureIntention.kt
Some examples for how it should work:
let foo$0 = (1, 2, 3);
let bar = foo.0;
let _ = foo.into();
becomes
let (_0, _1, _2) = (1, 2, 3);
let bar = _0;
let _ = (_0, _1, _2).into();
Similarly this should work for TupleStructs and RecordStructs:
struct Foo { bar: i32 }
let foo$0 = Foo { bar: 1 };
let bar = foo.bar;
let _ = foo.into();
becomes
struct Foo { bar: i32 }
let Foo { bar } = Foo { bar: 1 };
let bar = bar;
let _ = (Foo { bar }).into();
This should also respect private fields as well as #[non_exhaustive], putting .. in that case for the fields that aren't exposed.
Hm, why do we turn
let foo$0 = (1, 2, 3);
let bar = foo.0;
let _ = foo.into();
into
let (_0, _1, _2) = (1, 2, 3);
let bar = _0;
let _ = /*foo*/.into();
and not
let (_0, _1, _2) = (1, 2, 3);
let bar = _0;
let _ = (_0, _1, _2).into();
?
Edit: Actually that seems to be what IntelliJ does from a look at their code, but the PR #9855 only seems to argue against leaving foo there :thinking:
That wouldn't work if into takes &self though would it? Since we can't create a ref to the tuple without moving everything into it. So for the following:
let foo$0 = (NonCopy, NonCopy, NonCopy);
let bar = foo.0;
let _ = foo.take_by_ref();
let _ = foo.take_by_ref();
we would have to generate this(or even without emitting a ref expression as it doesnt change the problem)
let (_0, _1, _2) = (1, 2, 3);
let bar = _0;
let _ = (&(_0, _1, _2)).take_by_ref();
let _ = (&(_0, _1, _2)).take_by_ref();
which won't compile.
We can definitely be smart about a few cases for this though like in the specific example of yours.
I think that'd still a better non-compiling result than /*foo*/. No need to be smart about it, IMO.
(Also, we can put (_0, _1, _2) there, without the &; (_0, _1, _2).take_by_ref() will always resolve to the same method as foo.take_by_ref(). It's different if it was foo = &(1, 2, 3) of course.)
Hm ye actually reconstructing instead of commenting out does seem more appealing and still prevents us from introducing logic errors by mistake that we would get if we didn't replace the usage at all.
@Veykril between https://github.com/rust-lang/rust-analyzer/pull/9855 and https://github.com/rust-lang/rust-analyzer/pull/16638, is there anything left to do here?
Having a quick test with the example snippets we can still improve this I feel like. Right now we seem to have two assists, one for tuples one for structs, both of which behave slightly differently. The tuple one comments out code where it would need to reassemble the destructured binding, where as the struct one places a todo!(). We should fix that, for one I would assume these assists to share code / be just a single one, not two separate ones. Additionally we should really be re-assembling the struct/tuple where needed instead of commenting out the code or placing a todo!()