eslint / typescript-eslint-parser

An ESLint custom parser which leverages TypeScript ESTree to allow for ESLint to lint TypeScript source code.
Other
915 stars 92 forks source link

Crash on running ESLint #553

Closed ficristo closed 5 years ago

ficristo commented 5 years ago

What version of TypeScript are you using? 3.1.6

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

What code were you trying to parse? See https://github.com/quadre-code/quadre/pull/103 I cannot hunt down to specific code...

What did you expect to happen? Eslint should pass.

What happened?

TypeError: Cannot read property 'type' of undefined
    at isForInRef (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:406:24)
    at variable.references.some.ref (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:443:21)
    at Array.some (<anonymous>)
    at isUsedVariable (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:442:40)
    at collectUnusedVariables (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:565:26)
    at collectUnusedVariables (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:572:17)
    at collectUnusedVariables (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:572:17)
    at Program:exit (F:\github\quadre\node_modules\eslint\lib\rules\no-unused-vars.js:617:36)
    at listeners.(anonymous function).forEach.listener (F:\github\quadre\node_modules\eslint\lib\util\safe-emitter.js:47:58)
    at Array.forEach (<anonymous>)

Some more info At the moment I cannot update ESLint to version 5.* To see the error on my project I run grunt eslint --stack --debug. Is it possible the crash is because I do not use eslint-plugin-typescript and its typescript/no-unused-vars rule? Nonetheless I would expect an ESLint error and not a crash.

mysticatea commented 5 years ago

Thank you for this issue.

However, we cannot guarantee that ESLint core rules work properly on TypeScript syntax because the rules don't know TypeScript syntax. It's the reason that eslint-plugin-typescript has some copies of core rules.

I'd like to recommend to disable no-unused-vars rule because TypeScript compiler itself verifies it.

ficristo commented 5 years ago

I tryed to use eslint-plugin-typescript without success: https://github.com/bradzacher/eslint-plugin-typescript/issues/171 In the end I disabled the rule for ts and tsx files. If you think this is the expected behaviour feel free to close this issue.

@mysticatea thank you for your fast reply and help!

armano2 commented 5 years ago

@mysticatea i was able to reproduce this by this code (tested on master branch)

enum Foo {
    BAR = 'bar'
}
mysticatea commented 5 years ago

@armano2 In that case, I guess that no-unused-vars misunderstands EnumMember variables as regular variables because core rules don't know enum syntax. However, enum members make variables (see also https://github.com/eslint/typescript-eslint-parser/issues/552#issuecomment-439276282). I think that this is a case that we have to make the alternative rule in eslint-plugin-typescript.

armano2 commented 5 years ago

@mysticatea not really issue is with missing parent on identifier https://gist.github.com/armano2/3d27d7de6291ea01ef4893d8c3475132 - AST, scope, source

function isForInRef(ref) {
            let target = ref.identifier.parent;
mysticatea commented 5 years ago

Oh. Interesting. Reference#Identifier must not be null. And ESLint should set parent property of all nodes before gives nodes to rules.

armano2 commented 5 years ago

@mysticatea damn i found whats wrong xdddddd

in visitor-keys there is TSEnumMember: ["initializer"],

and should be TSEnumMember: ["id", "initializer"],

mysticatea commented 5 years ago

Ahhh!! Thank you so much for found!

armano2 commented 5 years ago

do you want me do to PR with this?

mysticatea commented 5 years ago

If you have time. Otherwise, I will do.

armano2 commented 5 years ago

ok pushed PR