Don't include already-covered cases in switch completions#51790
Don't include already-covered cases in switch completions#51790
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
| if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { | ||
| const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); | ||
| literals = literals.filter(literal => !tracker.hasValue(literal)); | ||
| symbols.forEach((symbol, i) => { |
There was a problem hiding this comment.
I can't filter the symbols array because the index of a symbol in this array is tied to the information in symbolToOriginInfoMap. I didn't want to create an abstraction on top of these two arrays and refactor everything, so I added an "ignore" flag to the origin info, but I'm open to suggestions on other ways to achieve this.
There was a problem hiding this comment.
We are overdue to make a nice abstraction over a similar data structure to what we have so it’s easier to work with without sacrificing (and probably improving on) the reasonably efficient allocations we have going. This seems fine for now.
| const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); | ||
| literals = literals.filter(literal => !tracker.hasValue(literal)); | ||
| symbols.forEach((symbol, i) => { | ||
| if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { |
There was a problem hiding this comment.
Note that the filtering won't work if someone is using e.g. an object literal as an enum, because checker.constantValue only works for enum members. But I'm hoping that filtering out literals and enum members already covers a lot of the cases.
There was a problem hiding this comment.
Seems like something that can be expanded to readonly members with literal types if there’s demand for it 👍
|
Oh yes! Thank you ❤️ |
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
Your comment on the reason for marking symbols as ignored might be worth putting in the code as well.
| if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { | ||
| const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); | ||
| literals = literals.filter(literal => !tracker.hasValue(literal)); | ||
| symbols.forEach((symbol, i) => { |
There was a problem hiding this comment.
We are overdue to make a nice abstraction over a similar data structure to what we have so it’s easier to work with without sacrificing (and probably improving on) the reasonably efficient allocations we have going. This seems fine for now.
| // filter literals & enum symbols whose values are already present in existing case clauses. | ||
| const caseClause = findAncestor(contextToken, isCaseClause); | ||
| if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { | ||
| const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); |
There was a problem hiding this comment.
Very cool that this abstraction came in handy in an additional place 🎉
| const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); | ||
| literals = literals.filter(literal => !tracker.hasValue(literal)); | ||
| symbols.forEach((symbol, i) => { | ||
| if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { |
There was a problem hiding this comment.
Seems like something that can be expanded to readonly members with literal types if there’s demand for it 👍
5 participants
Fixes #13711.