feat: In "Fill match arms", allow users to prefer Self to the enum name when possible
#19939
Conversation
…hen possible But default to not to. I chose to have a more generic config name because maybe other assists could also use the same approach.
|
Highly appreciated, thanks a lot! This is huge for me :) |
There was a problem hiding this comment.
(I should probably scour the issue tracker and open a proper issue for this if there's none for this already)
r-a still seems to perform syntactic equality checks instead of semantic ones when trying to determine which variants are already present. Originally reported in #6404 (comment) (from 2021).
For example, let's assume we're in a method of enum Ty { A, B }. Them, if we have match self { Ty::A => {} $0 } and rust-analyzer.assist.preferSelf, r-a doesn't realize that Type::A and Self::A are semantically equal and thus generates
match self {
Ty::A => {}
Self::A => {}
Self::B => {}
}This isn't specific to rust-analyzer.assist.preferSelf and an issue that has existed since at least 2021. For example if !preferSelf and type Alias = Ty;, this assist also doesn't consider Alias::A and Ty::A to be equal either leading to the same issue.
There was a problem hiding this comment.
That's completely unrelated to this PR which is concerned with the code r-a generates (and on a different assist even).
There was a problem hiding this comment.
I mean of course you know the impl contrary to me and I'm not saying that this PR regressed anything (and I should definitely open a separate issue for this),
I'm trying to refer to the fact that there seems to be some filtering going on to skip already covered variants (which is affected by preferSelf as added in this PR). E.g., with preferSelf, match … { Self::A => {} $0 } completes to match … { Self::A => {}, Self::B => {} } (correct) under add_missing_match_arms (the main assist modified in this PR and the only one I'm referring to, I hope?) but with !preferSelf it completes to match … { Self::A => {} Ty::A => {} Ty::B => {} } (incorrect).
There was a problem hiding this comment.
Ah, we seem to have a proper issue for this already: #4650.
But default to not to.
I chose to have a more generic config name because maybe other assists could also use the same approach.
Closes #19927.