TypeScript Contribution Diary: Improved Syntax Error for Enum Member Colons

'Petit verdot', or 'Colon', French grapes. Get it? Ha...

original

Previous post: Identifiers after Numeric Literals

Problem Statement

Pop quiz (no peeking!): for enum members with values, do you use a colon : or equals sign = between the name and value?

…

It’s hard to remember, right? I personally have a 50/50 chance of getting it right each time.

The answer is use an equals sign =. This is invalid TypeScript syntax:

enum HasIssue {
    Nope: 'Wat',
    //  ~
    // ',' expected.
}

That error message isn’t great. The real issue is that the code should have a = instead, not that it needs a comma.

Issue #838 was filed in October of 2014 (!) to provide a better value. This past September I figured it was time to finally resolve it.

Prior Art

The first question I needed to ask was: how does TypeScript parse in a source file and convert that into an AST?

…hang on, I’ve already started that investigation! My previous post, Identifiers after Numeric Literals, already describes that in its Enter the Compiler section.

From that post, we already know that TypeScript’s parser.ts uses a function called parseSourceFileWorker, which creates sourceFile.statements with a parseList function. For this investigation, the starting question within the parser was: what causes TypeScript to produce the ',' expected complaint?

Per TypeScript’s contribution guide’s section on localization, we know that the error message in code would look something like _0_expected.

Searching for Errors

I ran a text search in parser.ts for _0_expected and found several instances. The first one, within a function named parseExpected, seemed the most relevant - the others were mostly in functions that either referred to JSDoc parsing or unrelated syntax areas.

parseExpected’s contents looked to be pretty minimal:

• If token() (the function that gets the next token to be parsed) returns the expected kind of token, all is well, hooray!
• If not, per its comment: “Report specific message if provided with one. Otherwise, report generic fallback message.”

I added a console.log('wat', diagnosticMessage); to parseExpected and ran TypeScript on the above HasIssue syntax error example code. The resultant logs were a little surprising:

wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
wat undefined
index.ts:2:9 - error TS1005: ',' expected.

2     Nope: 'Wat',
          ~


Found 1 error.

What the heck? Why were there 16 wats but only one reported error?

This stumped me for a little while. After fumbling around with more console logs to no success, I gave up for the night and went to bed.

Only the next day did I think to add sourceFile.fileName to the logs:

wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/node_modules/@types/node/child_process.d.ts
wat undefined C:/Code/tsdevtest/index.ts
index.ts:2:9 - error TS1005: ',' expected.

2     Nope: 'Wat',
          ~


Found 1 error.

💡 Aha! There are some weird syntax errors in the @types/node enum in child_process.d.ts! Amusing.

Those typings were only there from some old playing around I’d done in that tsdevtest directory before, so I removed them and re-ran TypeScript:

wat undefined
index.ts:2:9 - error TS1005: ',' expected.

2     Nope: 'Wat',
          ~


Found 1 error.

Great.

Next up was to figure out who was calling parseExpected, so that I could change the code to give a better error message.

More Info

Since I still haven’t figured out VS Code debugging, I added new Error().stack to the log:

wat undefined index.ts Error
    at parseExpected (C:\Code\typescript\built\local\tsc.js:18106:51)
    at parseDelimitedList (C:\Code\typescript\built\local\tsc.js:18890:21)
    at parseEnumDeclaration (C:\Code\typescript\built\local\tsc.js:22432:32)
    at parseDeclarationWorker (C:\Code\typescript\built\local\tsc.js:21937:28)
    at parseDeclaration (C:\Code\typescript\built\local\tsc.js:21911:24)
    at parseStatement (C:\Code\typescript\built\local\tsc.js:21879:32)
    at parseListElement (C:\Code\typescript\built\local\tsc.js:18584:20)
    at parseList (C:\Code\typescript\built\local\tsc.js:18568:35)
    at parseSourceFileWorker (C:\Code\typescript\built\local\tsc.js:17803:37)
    at Object.parseSourceFile (C:\Code\typescript\built\local\tsc.js:17672:26)

Since parseExpected can take in a better error message, I figured the right way to go was to have parseDelimitedList pass one. parseDelimitedList is widely used and can take in considerSemicolonAsDelimiter?: boolean, so I was a little hesitant to add another parameter to parseDelimitedList itself.

However, there was a kind: ParsingContext parameter already passed to parseDelimitedList. ParsingContext is an enum with members indicating what kind of structure is being parsed, such as ArrayLiteralMembers, ClassMembers, or -most relevant here- EnumMembers.

I added a function to create a better diagnostic message based on that context:

function getExpectedCommaDiagnostic(kind: ParsingContext) {
	return kind === ParsingContext.EnumMembers
		? Diagnostics.An_enum_member_name_must_be_followed_by_a_or
		: undefined;
}

…and a new error message to diagnosticMessage.json:

"An enum member name must be followed by a ',' or '='.": {
    "category": "Error",
    "code": 1357
},

…and a getExpectedCommaDiagnostic call to parseExpected’s parameters:

- parseExpected(SyntaxKind.CommaToken);
+ parseExpected(SyntaxKind.CommaToken, getExpectedCommaDiagnostic(kind));

Running this new TypeScript version without the debugger log produced the exact right output:

index.ts:2:9 - error TS1357: An enum member name must be followed by a ',' or '='.

2     Nope: 'Wat',
          ~


Found 1 error.

🙌 Hooray!

Code Review

I committed this into a pull request and sent it out for review: https://github.com/microsoft/TypeScript/pull/33336. The feedback was generally positive, with a suggestion to also mention the closing curly bracket } as an acceptable character.

I made the change, pushed it to the pull request, and was pleased to see the pull request approved and merged a couple hours later.

index.ts:2:9 - error TS1357: An enum member name must be followed by a ',', '=', or '}'.

2     Nope: 'Wat',
          ~


Found 1 error.

Thanks orta for the speedy reviews!

Closing Thoughts

Honestly, I was a little disappointed by this investigation. I was hoping for something meaty and thick to bite into, with a result of achieving some significantly deeper understanding of the TypeScript compiler. I guess it’s probably better for end users that the pull request was minimal and accepted without major complexities?

Takeaways

• Issue age does not necessarily reflect solution complexity - old issues might be low hanging fruit too!
• Think carefully about your error messages, especially when balancing specificity and usefulness.