babel / babel-eslint

:tokyo_tower: A wrapper for Babel's parser used for ESLint (renamed to @babel/eslint-parser)
https://github.com/babel/babel/tree/main/eslint/babel-eslint-parser
MIT License
2.96k stars 208 forks source link

ESLint 6.2.0 + babel-eslint + no-unused-vars false positive with for-in loop #791

Closed feross closed 5 years ago

feross commented 5 years ago

There's a new issue in ESLint 6.2.0 just caught by the standard test suite. This may be an issue with babel-eslint, though I am not sure since the issue did not exist in ESLint 6.1.0. Here's a pointer to the issue I filed with ESLint: https://github.com/eslint/eslint/issues/12117

yyfearth commented 5 years ago

Seems the eslint-scope is outdated for the latest eslint.

jharris4 commented 5 years ago

FYI I tried upgrading eslint-scope to latest on my fork of this package, and the false positives are still happening.

DullReferenceException commented 5 years ago

Maybe related, having the same problem with for..of loops:

import scenarios from './scenarios';

for (const scenario of scenarios) {
  doSomethingWithScenario(scenario);
}

Get an eslint error:

'scenario' is defined but never used. eslint(no-unused-vars)

With eslint@6.2.0 and babel-eslint@10.0.2.

eslint@6.1.0 works.

yyfearth commented 5 years ago

According to the original eslint issue eslint/eslint#12117

Most likely it is that commit, with babel-eslint in the keyword example there is a TDZ scope between function and for, probably because it's using the old version of eslint-scope

feross commented 5 years ago

Just wanted to note that ESLint 6.2.1 contains an important security fix (see: https://eslint.org/blog/2019/08/eslint-v6.2.1-released) but it's not possible to upgrade to 6.2.0 or later until this issue is fixed.

JLHwung commented 5 years ago

For those who are curious about why ESLint 6.2 + babel-eslint does not work and how we may solve it on babel-eslint side, I have written a bit on https://github.com/babel/babel-eslint/pull/793#issuecomment-524435971.

hzoo commented 5 years ago

Ok merged https://github.com/babel/babel-eslint/pull/794 thanks to everyone and @JLHwung for the PR, released as 10.0.3. Not an easy package to maintain for sure given it's patching the packages themselves. Ideally we'll have better hooks for these things in the future.

Can also do one for v11 beta.

I verifed with one of the test cases on 10.0.2 and it failing and passing with 10.0.3. Let us know if this issue still persists! Thanks!

otakustay commented 5 years ago

@hzoo Sorry to be rude but is there any planed time to migrate this fix to the 11.x beta versions so we can eliminate our lint issues?

SebastienGllmt commented 5 years ago

I'm also stuck where I need 11.0.0-beta.0 to fix a Flow issue (10.0.3 can't handle default types for generic functions)

However upgrading to 11.0.0-beta.0 also gives me all these no-unused-vars warnings :(

I tried looking into porting the fix myself but it seems this involves too many internals for a first-time contributor.

przemyslawzalewski commented 5 years ago

During the time I have tried to fix the issue, I have also submitted a fix for the master branch. Due to issues with peer dependencies, file system based resolution was chosen instead. I do not know the current status of the master (11x) branch - if the problem was tackled there or not. To test the proposed fix, I have published published a fork on the npm to test the changes internally:

npm install @przemyslawzalewski/babel-eslint@11

You can check if the fix works for you by installing the fork and changing the parser within eslint config file from babel-eslint to @przemyslawzalewski/babel-eslint.

xsf0105 commented 4 years ago

It is a bug in eslint v6.2.0. Just update dependency eslint to v6.7.1.

mkastner commented 4 years ago

False positive on no-unused-vars Still happens in my environment with eslint v6.7.2 and babel-eslint v10.0.3.

pho3nixf1re commented 4 years ago

I am using eslint v6.8.0 with babel-eslint v10.0.3 and it is still happening in my environment. I've tried a variety of things to start with a clean setup and it is consistently reproducible.

jacekkarczmarczyk commented 4 years ago

Then I suggest to create a new issue and provide runnable reproduction @pho3nixf1re

kmarple1 commented 4 years ago

Also still experiencing this issue with eslint 6.8.0 and babel-eslint 10.0.3.

tronghiant commented 4 years ago

The issue was resolved with eslint 6.8.0 and babel-eslint 10.1.0

xsf0105 commented 4 years ago

The issue was resolved with eslint 6.8.0 and babel-eslint 10.1.0

Really?It's awsome!

joshuapinter commented 4 years ago

The issue was resolved with eslint 6.8.0 and babel-eslint 10.1.0

Really?It's awsome!

Confirmed.