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

Fix: Remove jsdoc node property from ts nodes (fixes #164) #177

Merged
merged 1 commit into from
Feb 26, 2017

Conversation

soda0289
Copy link
Member

JSDoc property is not found on parsed ESTree nodes and should be
removed when parsing typescript nodes.

@eslintbot
Copy link

LGTM

@soda0289
Copy link
Member Author

I looked at parsed espree and acorn nodes, they do not include the jsdoc property so I dont think we would need them on ts nodes since we dont have them on estree nodes.

JSDoc property is not found on parsed ESTree nodes and should be
removed when parsing typescript nodes.
@eslintbot
Copy link

LGTM

@soda0289
Copy link
Member Author

I added a test

@JamesHenry
Copy link
Member

This is currently not doing any harm right?

We have started planning ESLint v4 which will contain the comment attribution work, so I think I would prefer to wait and address the comments situation holistically.

Please let me know if this is actually an issue

@soda0289
Copy link
Member Author

When we convert Class, Function, or MethodDeclaration nodes we strip out the jsDoc comment property.
We do not do this on typescript nodes like Interface. The example code I posted in issue #164 will not parse and causes the parser to throw an exception. The exception is caused by the JSDocTag failing on node.getStart(). This used to work in older version of typescript I think.

I do not see any harm in remove this property since it is not included on other parsers like acorn or espree. We can wait if you like but I think having this parser not fail or throw uncaught exceptions is really important.

@soda0289
Copy link
Member Author

I could look into the typescript code base or post an issue to figure out why node.getStart() is failing?

@weirdpattern
Copy link
Contributor

take a look at #118, this was closed a while, but refers to the exact same issue @soda0289 is talking about...

@soda0289
Copy link
Member Author

@weirdpattern Issue #118 was a bug in typescript where it would fail to produce tokens correctly for JSDoc comments. This issue is a little different as it deals with converting typescript JSDoc nodes into non existing ESTree nodes.

@weirdpattern
Copy link
Contributor

by the way, node.getStart() fails because the node does not declare a parent... I looked into this when #118 was still open =]

@soda0289
Copy link
Member Author

@weirdpattern I noticed that the parent property was null which means it probably is a bug in typescript, since it did work in typescript 2.0.

@JamesHenry
Copy link
Member

Ah sorry @soda0289 I did misunderstand 😄 ! Yes let's get this in

@JamesHenry JamesHenry merged commit 2640d81 into eslint:master Feb 26, 2017
@soda0289 soda0289 deleted the fix-jsdoc-comment branch May 23, 2017 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants