“To expect the unexpected shows a thoroughly modern intellect.” - Oscar Wilde
Previous post: Improved Syntax Error for Enum Member Colons
This post is a bit more dry than my others and I don’t think it’s pedagogically sound. I just wanted to get this info out of my brain and into the world. It’s probably most useful if you’re trying to understand this area of the TypeScript code base and want to see a reference for how the system was set up. Sorry not sorry! 😄
You can refer to the TypeScript pull request as a technical reference.
Backing Context
For several years, TypeScript has included a few built-in comment directives: comments that tell the compiler to behave differently for the next line or current file.
One popular comment directive is // @ts-ignore
, which tells the TypeScript compiler to ignore any error from the following line.
It’s meant to be a last-ditch flag only used in the rare circumstances where TypeScript’s errors are incorrect.
// @ts-ignore
const wrong: number = "nope";
TypeScript’s issue #29394 requested either @ts-ignore
or an new @ts-expect-error
equivalent cause an error if the next line doesn’t have an error.
This would be similar to ESLint’s --report-unused-disable-directives
, which lets you know about any disable directives that are no longer necessary.
I’m a fan of the general idea of not keeping around code (even comments) that are no longer necessary.
The TypeScript team indicated they would want the new @ts-expect-error
directive, to maintain compatibility with @ts-ignore
.
Super.
Time to dive into the compiler!
🚀
Initial Investigation
Before adding any new comment directive I needed to understand how the existing ones work. I figured I’d want to understand:
- Where does TypeScript read
@ts-ignore
or other comment directives? - How does TypeScript change its reporting logic for those comment directives?
Reading Comment Directives
I first ran a search in TypeScript’s src/
for // @ts-ignore
.
The most relevant looking result was on the comments describing a shouldReportDiagnostic
function.
/**
* Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines
*/
function shouldReportDiagnostic(diagnostic: Diagnostic) {
That function took in a diagnostic (read: potential error message) and roughly:
- Computed the line and character position of the diagnostic using the appropriately named
computeLineAndCharacterOfPosition
- Captured the preceding line’s text
- Checked if a complicated looking
ignoreDiagnosticCommentRegEx
regular expression matched the text - Repeated steps 2-3 for preceding lines if the line was blank
Its regular expression notably had a @ts-ignore
after a bunch of convoluted whitespace and backslash checking:
const ignoreDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/;
A good start!
Now we understand how TypeScript was accounting for @ts-ignore
comment directives: by checking, for any would-be error message, whether a preceding line matched that regular expression.
Making a Plan
TypeScript’s previous shouldReportDiagnostic
logic went from diagnostic to comment using line numbers.
TypeScript didn’t have to remember where comments were originally parsed to adjust its output diagnostics for comment directives; it just had to check preceding lines before each comment directive.
With my changes, we’d have to make new logic for the other way around: from comment to diagnostic.
TypeScript would have to be made to remember the comment directives of a file so that it could determine which @ts-expect-error
s in a file didn’t match to a diagnostic.
Per the TypeScript Compiler Internals, there would be roughly two places that comments would be need to be interacted with:
- Parsing & Scanning (its creation): Reading in comment directives and remembering basic metadata about them, such as which directive they are
- Checking (its usage): Silencing diagnostics preceded by comment directives and creating new diagnostics for unused directives
Creation
I figured there were two things the code would need to do to remember the comment directives from scanning:
- Retrieve comment directives during existing comment handling logic
- Store those comment directives somewhere for the checker to use later
Retrieving Comment Directives
TypeScript’s parser.ts
was previously known to me as the place that parses raw text into AST nodes.
I ran searches for scanComment
, scanSingleLine
, scan(.*)omment
, and similar comment-related things but didn’t find anything useful.
The closest comment
-related search result I could find was processCommentPragmas
, but that didn’t look like it was related to any prior @ts-ignore
logic.
Looks like nothing was explicitly handling the single line comments.
I next looked to the scanner itself in scanner.ts
to see how comment literals were handled.
That was more fruitful.
The general scan
function had a case
in its big switch(ch)
dealing with single line comment trivia.
case CharacterCodes.slash:
// Single-line comment
if (text.charCodeAt(pos + 1) === CharacterCodes.slash) {
pos += 2;
Excellent.
Looking closer at the logic in scan
:
tokenPos
was the starting position from scanning the new tokenpos
was the ending position, or where the scanner would scan next
I added a basic test to check whether a comment matched one of the two comment directives:
const commentText = text.slice(tokenPos, pos);
if (commentText.includes("@ts-expect-error")) {
console.log("Yay!", commentText);
}
Running the local tsc.js
on some sample code showed positive results for finding comments with the directives.
Looks like I was on the right track!
Parsing Directives From Comments
A more proper solution to filtering directive-containing comments would be a regular expression that checks for, in order:
^
: The start of the comment string\s*
: Any amount of whitespace\/\/\/?
: 2 or 3 slashes\s*
: Any amount of whitespace@(ts-expect-error|ts-ignore)
: Either@ts-expect-error
or@ts-ignore
That last step is a matching group, meaning the contents inside the parenthesis would be stored in the resultant array from commentDirectiveRegEx.exec(text)
.
const commentDirectiveRegEx = /^\s*\/\/\/?\s*@(ts-expect-error|ts-ignore)/;
I preemptively created a function to extract the directive type from comment’s text if it matched:
function getDirectiveFromComment(text: string) {
const match = commentDirectiveRegEx.exec(text);
if (!match) {
return undefined;
}
switch (match[1]) {
case "ts-expect-error":
return CommentDirectiveType.ExpectError;
case "ts-ignore":
return CommentDirectiveType.Ignore;
}
return undefined;
}
…where CommentDirectiveType
was a small new enum
for just those two values.
enum CommentDirectiveType {
ExpectError,
Ignore,
}
Storing Comment Directives
Now that the code could figure out what type of directive stored in a comment, it was time to store those comment directives somewhere. Question: where was the right place to add something to a source file?
I figured scanners probably had some initial state that I could add to, so I looked around to find where scanners are created.
A search for : Scanner =
found a scanner
variable at the top of scanner’s createScanner
function, preceded by a bunch of stateful variables referring to position, node type, and so on.
Seemed legit!
// ...
// Start position of whitespace before current token
let startPos: number;
// Start position of text of current token
let tokenPos: number;
// ...
I added a commentDirectives: CommentDirective[] | undefined
variable to those variables to store discovered directives.
let commentDirectives: CommentDirective[] | undefined;
The directives themselves didn’t need to store much information — just the original text range and directive type:
export interface CommentDirective {
range: TextRange;
type: CommentDirectiveType;
}
With all that set up, the scanner’s single line comment parsing was ready to append its discovered comment directives. Straight from the pull request’s source code:
const type = getDirectiveFromComment(text.slice(tokenPos, pos));
if (type !== undefined) {
commentDirectives = append(commentDirectives, {
range: { pos: tokenPos, end: pos },
type,
});
}
In case you’re wondering,
append
is a commonly used function in TypeScript’s source code for dealing with potentially undefined arrays. It creates a new array if the existing one doesn’t already exist.
State Clearing
I also noticed the parser had a clearState
method used after scanning to reset its state.
I added a new clearCommentDirectives
method onto Scanner
and called it by the parser as well.
Retrieval API
Lastly, these comment directives would only be useful if the rest of TypeScript had the ability to access them.
I added a getCommentDirectives
method to the Scanner
interface:
/* @internal */
getCommentDirectives(): CommentDirective[] | undefined;
…and called it in the parseSourceFileWorker
method used by the parser to fill out the new generated ts.SourceFile
:
sourceFile.commentDirectives = scanner.getCommentDirectives();
Usage
At this point, comment directives were being parsed by the scanner+parser into an array accessible by a new getCommentDirectives()
method.
Next up was using those comment directives to:
- Selectively ignore existing error diagnostics
- Create new error diagnostics for unused
ts-expect-error
directives
It was mentioned in the TypeScript issue that proactively finding comments and linking them to diagnostics is more work than what TypeScript used to do. The data structure used to keep track of the comments would need to be efficient during both phases of its usage:
-
Creation:
- Insertion time for adding a new comment
-
Usage:
- Marking a directive as “used” if a diagnostic was disabled by it
- Reporting the “unused” directives for a file after diagnostics were all checked
I fiddled around with naming of the thing that would keep the comment directive information throughout the pull request’s implementation. Eventually I settled on this interface, which is what I’ll refer to for the rest of this post:
export interface CommentDirectivesMap {
getUnusedExpectations(): CommentDirective[];
markUsed(matchedLine: number): boolean;
}
The expected usage of a CommentDirectivesMap
would be:
- Call
markUsed
whenever a diagnostic needed to check whether it was disabled (theboolean
return) - After all diagnostics were checked, call
getUnusedExpectations
to grab the directives of typeCommentDirectiveType.ExpectError
not marked as used
Creating the Comment Directives Map
I added a createCommentDirectivesMap
function to turn a comment directives array into a map keying from line number to comment directive.
Keying by lines seemed like the right approach here because diagnostics would later need to check whether their immediately preceding line had a directive.
Interlude: Big O Notation
If you’ve taken any kind of intensive developer interview training or gone through a typical college Computer Science curriculum, the following might be burned into your mind:
- Hash tables have O(1) insertions and lookups
- Binary search trees have O(log(N)) insertions and lookups
O(), or “Big O” Notation refers to how quickly quickly an operation takes place based on how many items are in your collection.
- O(1) means the operation always takes the same amount of time (most of the time that’s very fast).
- O(log(N)) is the number of times you need to square a number -normally 2- to get to your N… meaning, it gets worse less quickly as you add more elements.
The built-in Object
s, Map
s, and Set
s in JavaScript are both generally hash table data structures with O(1) insertions and lookups. [map and set specification references]
Map Logic
Anyway, the first part of the map to add was the marking of lines that were used: a relatively simple storage of which lines were already seen.
Normally I’d use a Set<number>
, but searching for new Set
, createSet
, and similar terms in the TypeScript source didn’t show anything I could use.
On the other hand, searching for new Map
showed a createMap
at the very beginning of src/compiler/core.ts
for creating a blank Map<T>
.
TypeScript’s Map<T>
interface only allows string
s as keys.
It looked like the comment directives map would have to use stringified line numbers as keys.
Fine.
const usedLines = createMap<boolean>();
Mapping Directives
I also needed the function to map from line numbers to the contained directives.
Next in src/compiler/core.ts
was createMapFromEntries
: a utility to create a new Map<T>
from array of pairs.
const directivesByLine = createMapFromEntries(
commentDirectives.map((commentDirective) => [
`${
getLineAndCharacterOfPosition(sourceFile, commentDirective.range.pos).line
}`,
commentDirective,
]),
);
API Implementations
The getUnusedExpectations
implementation was fairly straightforward.
I used some array helpers to:
- Convert the
directivesByLine
entries into an array of pairs containing a line number and directive each - Filter those entries where the directive type was
CommentDirectiveType.ExpectError
and the line wasn’t inusedLines
The markUsed
implementation was also fairly straightforward:
- If a line didn’t match up to an entry in
directivesByLine
, returnfalse
- Otherwise, add the line to
usedLines
and returntrue
Yay!
Applying Comment Directives
Now that the comment directives map was created and usable, I needed to place it… somewhere.
TypeScript’s soon-to-be-replaced logic for ignoring diagnostics using the existing ignoreDiagnosticCommentRegEx
was in a shouldReportDiagnostic
function called by getProgramDiagnostics
function.
Those names sounded like exactly what I needed, and their implementations reinforced what it sounded like they did!
getProgramDiagnostics
: retrieve diagnostics from source files and filter out the ones that should be ignored by comment directivesshouldReportDiagnostic
: check whether a diagnostic should be ignored by a comment directive- This was one was interesting: in checking whether a diagnostic’s preceding line had a comment directive, TypeScript was explicitly skipping over lines that were empty or had non-directive comments.
I created a getDirectiveFilteredProgramDiagnostics
function to replace the filtering logic in getProgramDiagnostics
.
function getDirectiveFilteredProgramDiagnostics(
sourceFile: SourceFile,
commentDirectives: CommentDirective[],
diagnostics: Diagnostic[],
);
As a utility, I replaced shouldReportDiagnostic
with a markPrecedingCommentDirectiveLine
function to mark a diagnostic as used, and return the index of the matching comment directive line (or -1
if not found).
Roughly:
if (!sourceFile.commentDirectives?.length) {
return diagnostics;
}
// Diagnostics are only reported if there is no comment directive preceding them
// This will modify the directives map by marking "used" ones with a corresponding diagnostic
const directives = createCommentDirectivesMap(sourceFile, commentDirectives);
const diagnostics = diagnostics.filter(
(diagnostic) =>
markPrecedingCommentDirectiveLine(diagnostic, directives) === -1,
);
return diagnostics;
This looked reasonable, except… Oh no! Compiler errors!
shouldReportDiagnostic
was used in a second location: a getBindAndCheckDiagnosticsForFileNoCache
function.
Oh dear.
Refactoring
Two locations in TypeScript were using the now-deleted shouldReportDiagnostic
function to filter out diagnostics disabled by comment directives.
They actually looked pretty similar to each other:
getProgramDiagnostics
was looping over two two arrays of diagnostics to create an output array of filtered diagnostics.getBindAndCheckDiagnosticsForFileNoCache
was looping over several potentially undefined arrays of diagnostics to… also create an output array of filtered diagnostics!
…so that’s great!
The same getDirectiveFilteredProgramDiagnostics
was suitable for both locations.
I changed its signature to allow a ...
rest parameter of potentially undefined diagnostics arrays:
function getDirectiveFilteredProgramDiagnostics(
sourceFile: SourceFile,
commentDirectives: CommentDirective[],
...allDiagnostics: (readonly Diagnostic[] | undefined)[]
) {
const flatDiagnostics = flatten(allDiagnostics);
// ...
One of the arrays passed to this function (
bindDiagnostics
) was already marked asreadonly
, soallDiagnostics
needed to be as well.
Reporting Unused Directives
At last, the final piece: using the comment directive map’s getUnusedExpectations
to create new diagnostic complaints.
That needed to involve a couple of steps:
- Figuring out where to call the method to create new errors
- Creating new diagnostics corresponding to the unused expectations
Retrieving Unused Directives
Followups and Resultant Issues
Most non-trivial changes to a language cause some amount of bugs and/or tooling improvements. This was no exception.
VS Code Suggestions
VS Code needed to be told to suggest // @ts-expect-error
in addition to // @ts-ignore
for its TypeScript quick fixes.
I found the place in VS Code that listed those directives and sent a quick pull request to add the new logic in.
You can get those changes in VS Code >=1.44. 🚀
Resultant Issues
I’ll be honest: although it was a lot of fun writing this feature and felt great seeing some of my TypeScript idols give it positive feedback, there were a few bugs added in the work that I wish I’d caught. Personally, I have a tendency to rush through work - it’s led to some bad production bugs in my day jobs in the past. It tends to show up when I’m excited about what I’m doing, which is generally at maximum levels with TypeScript.
I’m also not particularly familiar with the TypeScript ecosystem or all the common -let alone uncommon- use cases for the compiler.
The TypeScript team uses nightly releases and pre-release “Release Candidate” (RC) versions to let the community QA newer compiler versions before they’re ready.
Some kind of nightly/RC system is invaluable for catching bugs in evolving software, and was predictably useful in sniffing out negative aspects of the // @ts-expect-error
change.
Handling Incremental Compiles
🐛 Bug report: @ts-ignore stops in editing (regression)
A user reported in the TypeScript issues that a // @ts-ignore
comment wasn’t being applied after it was re-added back to a file from an undo of a deletion.
One of TypeScript’s core team members, @sheetalkamat, fixed it pretty quickly in a pull request to handle comment directives in incremental parsing.
Turns out TypeScript’s parser needed special logic for incremental parsing. I had no idea. Now I do.
JSX Comment Directives
🐛 Bug report: TSX @ts-ignore comments no longer suppressing errors in TypeScript 3.9
Did you know this was the standard way to write // @ts-ignore
comments prior to TypeScript 3.9?
{/*
// @ts-ignore */}
<MissingRequiredProp />
Bizarre, right?
It’s actually a fortuitous quirk of the way TypeScript used to parse // @ts-ignore
comment directives (issue discussion):
- If a line starts with
//
(ignoring whitespace), it’s a directive. - JSX does not allow for
//
comments (issue discussion) - JSX does allow for multiline
{/* ... */}
comments, which can include a line that starts with//
The changes to more more realistically parse single line comments actually “broke” these JSX comment directives by no longer allowing for comment directives within /* ... */
multiline comments! 😂
My followup pull request to allow comment directives to be multiline added the new comment directive parsing logic to multiline comments as well, so the above .tsx
code snippet can now be rewritten as:
{/* @ts-ignore */}
<MissingRequiredProp />
I had some trouble understanding how the incremental parser unit tests were set up, then some code churn around which forms of comments should or shouldn’t be allowed. The pull request was merged just in time to make it into the final 3.9 release. Phew!
In Closing
When I first set up these TypeScript diary posts, I had an inner drive to make some kind of core, intensive contribution to the “cool” parts of TypeScript. I think this contribution finally scratched that itch.
Thanks for making it this far through the post - I hope it was useful to you in some way!