Goldblog

GitHubSiteTwitter

TypeScript Contribution Diary: // @ts-expect-error

July 21, 2020

“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:

  1. Where does TypeScript read @ts-ignore or other comment directives?
  2. 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 desribing 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:

  1. Computed the line and character position of the diagnostic using the appropriately named computeLineAndCharacterOfPosition
  2. Captured the preceding line’s text
  3. Checked if a complicated looking ignoreDiagnosticCommentRegEx regular expression matched the text
  4. 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-errors 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:

  1. Parsing & Scanning (its creation): Reading in comment directives and remembering basic metadata about them, such as which directive they are
  2. 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:

  1. Retrieve comment directives during existing comment handling logic
  2. 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 token
  • pos 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(tokenos, 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:

  1. ^: The start of the comment string
  2. \s*: Any amount of whitespace
  3. \/\/\/?: 2 or 3 slashes
  4. \s*: Any amount of whitespace
  5. @(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:

  1. Selectively ignore existing error diagnostics
  2. 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:

  1. Call markUsed whenever a diagnostic needed to check whether it was disabled (the boolean return)
  2. After all diagnostics were checked, call getUnusedExpectations to grab the directives of type CommentDirectiveType.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 Objects, Maps, and Sets 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 strings 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:

  1. Convert the directivesByLine entries into an array of pairs containing a line number and directive each
  2. Filter those entries where the directive type was CommentDirectiveType.ExpectError and the line wasn’t in usedLines

The markUsed implementation was also fairly straightforward:

  1. If a line didn’t match up to an entry in directivesByLine, return false
  2. Otherwise, add the line to usedLines and return true

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 directives
  • shouldReportDiagnostic: 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:

…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 as readonly, so allDiagnostics 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:

  1. Figuring out where to call the method to create new errors
  2. 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!

Josh GoldbergHi! I'm a frontend developer from New York. This is my blog about JavaScript, TypeScript, and scaling web application development.
This site's open source on GitHub. Found a problem? File an issue!