Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Parser does not handle jsdoc well #118

Closed
jtheoof opened this issue Nov 24, 2016 · 10 comments
Closed

Parser does not handle jsdoc well #118

jtheoof opened this issue Nov 24, 2016 · 10 comments
Labels

Comments

@jtheoof
Copy link
Contributor

jtheoof commented Nov 24, 2016

What version of TypeScript are you using?
2.1.1

What version of typescript-eslint-parser are you using?
1.0.0

What code were you trying to parse?

/**
 * This crashes eslint.
 */
export interface Test {
    name: string;
}

What did you expect to happen?
Syntax should parse correctly

What happened?
Stacktrace:

Cannot read property 'loc' of undefined
TypeError: Cannot read property 'loc' of undefined
    at getNodeIndent (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/rules/indent.js:236:54)
    at EventEmitter.Program (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/rules/indent.js:793:49)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
    at CommentEventGenerator.enterNode (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/util/comment-event-generator.js:97:23)
    at Controller.enter (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/eslint/lib/eslint.js:903:36)
    at Controller.__execute (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/Users/jattali/dev/mnubo/mnubo-front-end/app/node_modules/estraverse/estraverse.js:501:28)

Note: If you remove the jsdoc comments, eslint does not crash anymore.

@jtheoof
Copy link
Contributor Author

jtheoof commented Nov 24, 2016

In fact, if you replace:

/**
 * This crashes eslint.
 */

with:

/*
 * This crashes eslint.
 */

eslint is happy again.

@jtheoof
Copy link
Contributor Author

jtheoof commented Nov 24, 2016

Probably related to #5

@nzakas nzakas added bug and removed triage labels Dec 1, 2016
@weirdpattern
Copy link
Contributor

@nzakas if you don't mind, I think I may have a fix for this and a way to attributing all comment nodes (single line, multiline and jsdoc) as mentioned by @JamesHenry in #5. I think this will enable rules like valid-jsdoc and others...

Code is done, but I'm still working on the tests, so give me a couple of days and I will send a PR.

@JamesHenry
Copy link
Member

Thanks for the suggestion, @weirdpattern!

You must have missed the resolution in that issue #5 (comment) - comment attribution will be done in an upcoming version of ESLint, not in this project, nor any other other future parsers.

@weirdpattern
Copy link
Contributor

weirdpattern commented Dec 23, 2016

@JamesHenry But we still need to identify the comments and convert them to ESTree nodes, right?

Maybe I misunderstood the whole thing and did a terrible job at explaining what the code actually does. The code identifies and converts comments in typescript to ESTree nodes, which is responsibility of the parser if I'm not mistaken.

Let me know if this is still something we don't need and I will just move on to a different issue =)

@nzakas
Copy link
Member

nzakas commented Dec 31, 2016

Yes, comments will still need to be converted and included in the top level comments array, so I think we still need this fixed.

@weirdpattern
Copy link
Contributor

ok, then give me a couple of days to complete the unit tests, then I will submit a PR for you to review...

@JamesHenry
Copy link
Member

@weirdpattern The logic to scan for and convert comments was already added here

/**
* Create a TypeScript Scanner, with skipTrivia set to false so that
* we can parse the comments
*/
var triviaScanner = ts.createScanner(ast.languageVersion, false, 0, code);
var kind = triviaScanner.scan();
while (kind !== ts.SyntaxKind.EndOfFileToken) {
if (kind !== ts.SyntaxKind.SingleLineCommentTrivia && kind !== ts.SyntaxKind.MultiLineCommentTrivia) {
kind = triviaScanner.scan();
continue;
}
var isBlock = (kind === ts.SyntaxKind.MultiLineCommentTrivia);
var range = {
pos: triviaScanner.getTokenPos(),
end: triviaScanner.getTextPos(),
kind: triviaScanner.getToken()
};
var comment = code.substring(range.pos, range.end);
var text = comment.replace("//", "").replace("/*", "").replace("*/", "");
var loc = getLocFor(range.pos, range.end, ast);
var esprimaComment = convertTypeScriptCommentToEsprimaComment(isBlock, text, range.pos, range.end, loc.start, loc.end);
extra.comments.push(esprimaComment);
kind = triviaScanner.scan();
}
, but some active assertions will definitely be needed.

We currently have old espree test fixtures within attach-comments, but I think it might be best to wait until the comment attribution support lands in ESLint 4, before attempting to finalise how and what we test in this parser.

Many thanks for your offer to contribute, it is always welcome!

@weirdpattern
Copy link
Contributor

Makes sense, let's wait for eslint 4 to be released, then we can discuss.

thanks for taking the time to answer my questions =)

@JamesHenry
Copy link
Member

This is not actually a bug in the currently supported logic (TypeScript 2.0.x).

This is an issue that applies to supporting 2.1.x, which is now being worked on in #128

Closing this issue in favour of that one, particularly to avoid the confusion with comment attribution, which is a totally separate issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants