Request for info on jsDoc Node addition in TypeScript 2.1.x

[JamesHenry]

TypeScript Version: 2.1.5

Code

/**
 * Foo bar qux
 */
var a = true;

Background: Supporting TypeScript 2.1.x in typescript-eslint-parser

I am a member of the ESLint Team working on the the typescript-eslint-parser, you may have seen issues from me before 😄 As always I am excited about enabling ESLint users to work with TypeScript!

As part of the project we are naturally exposed to breaking changes in the TypeScript AST between versions.

The only remaining issue with supporting TS 2.1.x in the parser is accommodating the changes that were made around JSDoc comments between TS 2.0.x and TS 2.1.x.

For reference: eslint/typescript-eslint-parser#128 (comment)

I cannot find any information on this change at all, despite numerous attempts. Please can you point me towards any relevant documentation if there is any?

At the AST level, I can observe that the code sample above would previously produce just a leadingComment node in TS 2.0.x, but it now contains a new jsDoc property and corresponding new node in TS 2.1.x.

The actual issue for the typescript-eslint-parser, however, is the fact that the behaviour of TypeScript node utility functions seems to have been affected by this change.

ast.getFirstToken() for this code in TS 2.0.x would return a TokenObject, but in TS 2.1.x it returns undefined.

Is this the intended behaviour?

For the ESTree AST, we need to produce an array of all the tokens in the program, and so we iterate through them, starting with the first token, so this is currently a blocker.

As a final note, if you remove the second asterisk to turn the JSDoc comment into a standard multi-line comment, the behaviour is identical to TS 2.0.x. Perhaps this is a related issue #9975?

Many thanks in advance for your help!

[JamesHenry]

@DanielRosenwasser do you have any thoughts on this?

[mhegazy]

ast.getFirstToken() for this code in TS 2.0.x would return a TokenObject, but in TS 2.1.x it returns undefined.

Is this the intended behaviour?

no. this looks like a bug.

[JamesHenry]

Thanks, @mhegazy!

Are there any docs/issues on why this change was actually made? Are you guys planning some more advanced jsDoc features?

[sandersn]

Yep, JSDoc comments now have a different SyntaxKind than [SingleLine|MultiLine]CommentTrivia, so getFirstToken tries to recur into them, which doesn't terminate correctly.

For now I'll add special-case code in get[First|Last]Token, but the long-term right fix is to treat jsdoc comments the same as other comments unless explicitly asked by the caller.

[sandersn]

@JamesHenry Before 2.1, Typescript had two jsdoc parsers, both very simple. The one in the checker looked only for types and the one in the language service looked only for text. In 2.1 I unified them with #10671, and we expect to continue improving jsdoc support since Javascript will rely on it pretty heavily to give good errors.

[mhegazy]

@JamesHenry can you give the nightly a try some time after tomorrow?

[JamesHenry]

Thanks @mhegazy and @sandersn. That PR does indeed change the behaviour of Node.getFirstToken, and it's no longer returning undefined.

It is not a full fix for our particular case, however.

Here is our logic from typescript-eslint-parser for converting all of the tokens in the TS AST to ESTreeTokens:

/**
 * Converts all tokens for the given AST
 * @param  {Object} ast the AST object
 * @returns {ESTreeToken[]}     the converted ESTreeTokens
 */
function convertTokens(ast) {
    var token = ast.getFirstToken(),
        converted,
        result = [];

    while (token) {
        converted = convertToken(token, ast);
        if (converted) {
            result.push(converted);
        }
        token = ts.findNextToken(token, ast);
    }

    return result;
}

Failing test case source code:

/**
 * this is anonymous class.
 */
export default class {
    /**
     * this is method1.
     */
    method1(){
    }
}

Thanks to @sandersn PR, var token = ast.getFirstToken() is no longer returning undefined, but the first iteration of the loop's token = ts.findNextToken(token, ast); is returning undefined instead of the next token. This was not the case in TS 2.0.x

Many thanks for your support on this!

[sandersn]

Yeah, I don't think the PR had the right fix. The better fix is tojust skip jsdoc since 2.0 didn't even return them. I'll put another PR up shortly.

[sandersn]

By the way @JamesHenry, it would be a lot more efficient to recursively walk the tree converting tokens instead of relying on findNextToken. findNextToken does one part of the recursive walk that walk does below, but over and over, and with a lot of extra checks. It's also not part of the typescript public API.

function isToken(n: ts.Node) {
        return n.kind >= ts.SyntaxKind.FirstToken && n.kind <= ts.SyntaxKind.LastToken;
}
function convertTokens(root: ts.Node): void {
    let result = [];
    function walk(node: ts.Node) {
        if (isToken(node)) {
            let converted = convertToken(node, root);
            if (converted) {
                result.push(converted);
            }
        }
        else {
            node.getChildren().forEach(walk);
        }
    }
    walk(root);
}

[JamesHenry]

Great, thanks! Just let me know when the new logic is available in typescript@next and I'll try it out

[sandersn]

I merged it just now, so it should be available tomorrow.

[JamesHenry]

It works! Thanks so much for your help, @sandersn!

Will this be released as a patch release to 2.1.x?

[sandersn]

I'm not sure about our 2.1.x release schedule. @mhegazy ?

[mhegazy]

We have no plans for additional 2.1 releases at the time being. the next release would be 2.2 RC in the next few weeks.

[JamesHenry]

Ok, thanks guys.

It don't think it would be right for our release to depend on typescript@next, so I will just have to leave it on a branch for people to install via git until 2.2 stabilises.

Fingers crossed the change TS 2.2 is nice and smooth 😁