Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jsdoc @template issue in syntactic classifier #8676

Closed
basarat opened this issue May 19, 2016 · 8 comments
Closed

Jsdoc @template issue in syntactic classifier #8676

basarat opened this issue May 19, 2016 · 8 comments
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@basarat
Copy link
Contributor

basarat commented May 19, 2016

Consider the following:

/**
 * @template T
 */
function ident<T>(x:T):T {
    return ident;
}

getEncodedSyntacticClassifications will fail with Cannot read property 'text' of undefined

Stack:

NodeObject.createChildren   @   ntypescript.js:50808
NodeObject.getChildren  @   ntypescript.js:50845
processJSDocTemplateTag @   ntypescript.js:56304
classifyJSDocComment    @   ntypescript.js:56270
classifyComment @   ntypescript.js:56243
classifyLeadingTriviaAndGetTokenStart   @   ntypescript.js:56213
tryClassifyNode @   ntypescript.js:56345
processElement  @   ntypescript.js:56482
processElement  @   ntypescript.js:56484
processElement  @   ntypescript.js:56484
getEncodedSyntacticClassifications  @   ntypescript.js:56185

Reason: sourceFile is undefined somewhere in the stack

Version : latest as of this morning

refs alm-tools/alm#82 🌹

@basarat
Copy link
Contributor Author

basarat commented May 19, 2016

Didn't want to pollute the issue with my potentially wrong analysis. This screenshot might help people figure out the fix

image

getChildren calls createChildren (both without the optional sourceFile) and since parent is not there it crashes .... I haven't taken a deeper look 🌹

@mhegazy mhegazy added Bug A bug in TypeScript API Relates to the public API for TypeScript labels May 19, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone May 19, 2016
@mhegazy
Copy link
Contributor

mhegazy commented May 19, 2016

@zhengbli was looking into a similar issue the other day.

@zhengbli
Copy link
Contributor

@basarat can you give #8702 a try? Thanks!

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

@basarat can you give #8103 a try instead?

@basarat
Copy link
Contributor Author

basarat commented Jun 1, 2016

I just made a new ntypescript build https://github.com/TypeStrong/ntypescript that has that PR https://github.com/Microsoft/TypeScript/tree/166f399d176486a1247c54dcd6ad4a18e33c18f3 but still no dice:

image

Thanks for pinging / attempting 🌹

@basarat
Copy link
Contributor Author

basarat commented Jun 1, 2016

I'm also getting a weird error in the latest build where a property is defined (e.g. styles below) but this.styles is showing an error.

image

But haven't been able to come up with a short repro ... so you can ignore that for now 🌹

@zhengbli
Copy link
Contributor

zhengbli commented Jun 2, 2016

It turns out when calling parseIsolatedJSDocComment, the fixing node's parent pointer part was skipped. This should be fixed by #8930, also added a test just for sure.

@zhengbli zhengbli added the Fixed A PR has been merged for this issue label Jun 2, 2016
@basarat
Copy link
Contributor Author

basarat commented Jun 2, 2016

Verified. Thanks 💯 🌹

basarat added a commit to alm-tools/alm that referenced this issue Jun 2, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants