-
-
Notifications
You must be signed in to change notification settings - Fork 74
Breaking: throw syntax error on incomplete variable declarations (fixes #531) #547
Conversation
| "coverageReporters": [ | ||
| "text-summary" | ||
| ], | ||
| "collectCoverageFrom": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this configuration of jest because jest threw a syntax error on new tests/fixtures/basics/solo-const.src.js file while collecting coverage.
platinumazure
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we can control in the TS parser (e.g., with a parser option)? It might be better/easier if we just tell TS not to tolerate that error type.
|
I guess |
|
Got it. I guess my other thought is, should this be handled in typescript-estree? If that project is supposed to create ESTree output and variable declarations with no declarators are invalid, maybe that should throw the error rather than typescript-eslint-parser? What do you think? |
|
I'm thinking about it, too. I'm not sure if this is a spec violation or not because: ESTree defines types (E.g., Anyway, I should open an issue on typescript-estree repo. |
|
Yep we can get that in as an option in the parser 👍 |
|
See my comment here after Andy from the TypeScript team responded: #531 (comment) It should hopefully be possible for us to leverage TypeScript here, as long as we can clarify the behaviour is correct |
|
Response from Andy:
Maybe a good next step would be if I add an option to the parser (which will be program.getSyntacticDiagnostics(file);
program.getSemanticDiagnostics(file);E.g. the option could be ...and we could see if that is something this ESLint parser would to enable? |
|
I'm sorry for the delay. I have overlooked this.
|
|
|
|
|
|
FYI I added the |
|
Once we merge the latest updates of typescript-estree (#596), we can explore surfacing an option for the diagnostics reporting |
|
Closing this for now, we'll add in full support for |
This PR makes the parser throwing on incomplete variable declarations.
It's a syntax error on both JavaScript and TypeScript. But TypeScript parser looks so tolerant and triggers that core rules are confusing.