Skip to content

Exhaustive case completion for switch statements #50996

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.

Sign up for GitHub

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

Merged
gabritto merged 27 commits into from
Dec 2, 2022
Merged

Exhaustive case completion for switch statements #50996

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f6c3566
fix services' type's isLiteral
gabritto Sep 23, 2022
4467330
update literal completions tests
gabritto Sep 23, 2022
a206fe1
initial prototype
gabritto Sep 7, 2022
73c1eea
use symbol to expression. TODO: filter existing, replace import nodes
gabritto Sep 16, 2022
5648cba
WIP
gabritto Sep 23, 2022
d46c0d2
WIP
gabritto Sep 28, 2022
297f892
remove booleans from literals
gabritto Sep 28, 2022
fd1d6ed
Merge branch 'gabritto/servicesIsLiteral' into gabritto/switchsnippet
gabritto Sep 28, 2022
4c528b3
trigger at case keyword positions
gabritto Sep 29, 2022
1a5cd05
clean up tests
gabritto Nov 8, 2022
ee42732
fix element access expression case
gabritto Nov 9, 2022
1819d0b
refactor dealing with existing values into a tracker
gabritto Nov 9, 2022
b19543e
Merge branch 'main' into gabritto/switchsnippet
gabritto Nov 10, 2022
bd5b817
fix merge errors
gabritto Nov 10, 2022
f02122b
cleanup and more tests
gabritto Nov 10, 2022
a35bc4a
fix lint errors
gabritto Nov 10, 2022
83b88f7
more merge conflict fixes and cleanup
gabritto Nov 11, 2022
599fb30
use appropriate quotes
gabritto Nov 11, 2022
97dcf69
small indentation fix
gabritto Nov 11, 2022
3b92638
refactor case clause tracker
gabritto Nov 14, 2022
89f6f6b
Merge branch 'main' into gabritto/switchsnippet
gabritto Nov 14, 2022
1894d2e
experiment: support tabstops after each case clause
gabritto Nov 22, 2022
90767fc
address small CR comments
gabritto Nov 23, 2022
d1c8968
fix completion entry details; add test case
gabritto Nov 30, 2022
fb15ba1
Merge branch 'main' into gabritto/switchsnippet
gabritto Nov 30, 2022
3980b93
fix lint errors
gabritto Dec 1, 2022
8823108
remove space before tab stops; refactor
gabritto Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor dealing with existing values into a tracker
  • Loading branch information
@gabritto
gabritto committed Nov 9, 2022
commit 1819d0badd377a7cf7adcd3161cce7cf177d1829
143 changes: 79 additions & 64 deletions src/services/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,47 +604,11 @@ namespace ts.Completions {
const switchType = checker.getTypeAtLocation(caseBlock.parent.expression);
// >> TODO: handle unit type case?
if (switchType && switchType.isUnion() && every(switchType.types, type => type.isLiteral())) {
const existingStrings = new Set<string>();
const existingNumbers = new Set<number>();
const existingBigInts = new Set<string>();

// Collect constant values in existing clauses.
const tracker = newCaseClauseTracker(checker);
for (const clause of clauses) {
if (isDefaultClause(clause)) {
continue;
}
if (isLiteralExpression(clause.expression)) {
const expression = clause.expression;
switch (expression.kind) {
case SyntaxKind.NoSubstitutionTemplateLiteral:
case SyntaxKind.StringLiteral:
existingStrings.add(expression.text);
break;
case SyntaxKind.NumericLiteral:
existingNumbers.add(parseInt(expression.text)); // >> do we need to parse it??
break;
case SyntaxKind.BigIntLiteral:
const parsedBigInt = parseBigInt(endsWith(expression.text, "n") ? expression.text.slice(0, -1) : expression.text);
if (parsedBigInt) {
existingBigInts.add(pseudoBigIntToString(parsedBigInt)); // >> does it work? answer: no
}
break;
}
}
else {
const symbol = checker.getSymbolAtLocation(clause.expression);
if (symbol && symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) {
const enumValue = checker.getConstantValue(symbol.valueDeclaration);
if (enumValue !== undefined) {
switch (typeof enumValue) {
case "string":
existingStrings.add(enumValue);
break;
case "number":
existingNumbers.add(enumValue);
}
}
}
if (!isDefaultClause(clause)) {
tracker.addClause(clause);
}
}

Expand All @@ -659,20 +623,10 @@ namespace ts.Completions {
// Filter existing enums by their values
const enumValue = type.symbol.valueDeclaration && checker.getConstantValue(type.symbol.valueDeclaration as EnumMember);
if (enumValue !== undefined) {
switch (typeof enumValue) {
case "string":
if (existingStrings.has(enumValue)) {
continue;
}
existingStrings.add(enumValue);
break;
case "number":
if (existingNumbers.has(enumValue)) {
continue;
}
existingNumbers.add(enumValue);
break;
if (tracker.hasValue(enumValue)) {
continue;
}
tracker.addValue(enumValue);
}
const typeNode = codefix.typeToAutoImportableTypeNode(checker, importAdder, type, caseBlock, target);
if (!typeNode) {
Expand All @@ -685,24 +639,16 @@ namespace ts.Completions {
elements.push(expr);
}
// Literals
else {
else if (!tracker.hasValue(type.value)) {
switch (typeof type.value) {
case "object":
// BigInt literal
if (!existingBigInts.has(pseudoBigIntToString(type.value))) {
elements.push(factory.createBigIntLiteral(type.value));
}
elements.push(factory.createBigIntLiteral(type.value));
break;
case "number":
// number literal
if (!existingNumbers.has(type.value)) {
elements.push(factory.createNumericLiteral(type.value));
}
elements.push(factory.createNumericLiteral(type.value));
break;
case "string":
if (!existingStrings.has(type.value)) {
elements.push(factory.createStringLiteral(type.value));
}
elements.push(factory.createStringLiteral(type.value));
break;
}
}
Expand Down Expand Up @@ -748,6 +694,75 @@ namespace ts.Completions {
return undefined;
}

interface CaseClauseTracker {
addClause(clause: CaseClause): void;
addValue(value: string | number): void;
hasValue(value: string | number | PseudoBigInt): boolean;
}

function newCaseClauseTracker(checker: TypeChecker): CaseClauseTracker {
const existingStrings = new Set<string>();
const existingNumbers = new Set<number>();
const existingBigInts = new Set<string>();

return {
addValue,
addClause,
hasValue,
};

function addClause(clause: CaseClause) {
if (isLiteralExpression(clause.expression)) {
const expression = clause.expression;
switch (expression.kind) {
case SyntaxKind.NoSubstitutionTemplateLiteral:
case SyntaxKind.StringLiteral:
existingStrings.add(expression.text);
break;
case SyntaxKind.NumericLiteral:
existingNumbers.add(parseInt(expression.text)); // >> TODO: do we need to parse it??
break;
case SyntaxKind.BigIntLiteral:
const parsedBigInt = parseBigInt(endsWith(expression.text, "n") ? expression.text.slice(0, -1) : expression.text);
Copy link
Copy Markdown
Member

@amcasey amcasey Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me why we need to parse and then re-stringify bigints?

Copy link
Copy Markdown
Member Author

@gabritto gabritto Nov 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use a standard representation of big ints for our sets, so that big ints that are written differently but are the same end up with the same representation. Since we can't use actual big ints because they're not supported in all runtimes (that might change eventually?), we use "pseudo big ints" in our codebase to represent big ints, but then that's not something we can put into a set, therefore I need to stringify the pseudo big ints.

if (parsedBigInt) {
existingBigInts.add(pseudoBigIntToString(parsedBigInt));
Comment thread
gabritto marked this conversation as resolved.
}
break;
}
}
else {
const symbol = checker.getSymbolAtLocation(clause.expression);
if (symbol && symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) {
const enumValue = checker.getConstantValue(symbol.valueDeclaration);
if (enumValue !== undefined) {
addValue(enumValue);
}
}
}
}

function addValue(value: string | number) {
switch (typeof value) {
case "string":
existingStrings.add(value);
break;
case "number":
existingNumbers.add(value);
}
}

function hasValue(value: string | number | PseudoBigInt): boolean {
switch (typeof value) {
case "string":
return existingStrings.has(value);
case "number":
return existingNumbers.has(value);
case "object":
return existingBigInts.has(pseudoBigIntToString(value));
}
}
}

function typeNodeToExpression(typeNode: TypeNode, languageVersion: ScriptTarget): Expression | undefined {
switch (typeNode.kind) {
case SyntaxKind.TypeReference:
Expand Down