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

no-use-before-define false negative #10227

Closed
BridgeAR opened this issue Apr 16, 2018 · 4 comments · Fixed by #10396
Closed

no-use-before-define false negative #10227

BridgeAR opened this issue Apr 16, 2018 · 4 comments · Fixed by #10396
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@BridgeAR
Copy link
Contributor

BridgeAR commented Apr 16, 2018

Tell us about your environment

  • ESLint Version: 4.18.0
  • Node Version: 10.0.0-pre
  • npm Version: 5.6.0

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

no-use-before-define: error

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

function urgs() {
  for (const value of array) {
    console.log(value);
  }

  const array = [];
}

What did you expect to happen?

An error should be reported for array in the first line of the function.

What actually happened? Please include the actual, raw output from ESLint.

Nothing is reported.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Apr 16, 2018
@platinumazure
Copy link
Member

I can reproduce in the demo with both ForInStatement and ForOfStatement. Seems like a bug to me. Thanks @BridgeAR for reporting this!

@platinumazure platinumazure added bug ESLint is working incorrectly rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Apr 16, 2018
@mysticatea
Copy link
Member

Wow, I can't believe that such bug has remained. 😳

@mysticatea
Copy link
Member

I will work on this if anybody has not started.

@mysticatea
Copy link
Member

Hmm, this bug is tougher than the first glance. Looks to be by some factors which contain breaking change to fix.

  1. no-use-before-define rule doesn't check for scope entirely.
  2. Linter#getScope() returns a certain wrong scope if the current node is any of ForStatement, ForInStatement, and ForOfStatement. It should return the scopes which their type is for, but it returned the parent scope of the for scope. We should fix the Linter#getScope() method, but it has big impact, maybe. Fortunately, we are making new major version currently.
  3. eslint-scope package doesn't seem to handle ForInStatement/ForOfStatement correctly. Reference objects of ForInStatement#right/ForOfStatement#right are missing. It has been stolen by enigmatic TDZ scope.

So it needs the following steps to fix this bug:

  1. Fix eslint-scope to remove TDZ scope such as I mentioned in Fix: wrong resolution about default parameters eslint-scope#33 (comment). This is a breaking change.
  2. Fix Linter#getScope() to return for scope for ForStatement, ForInStatement, and ForOfStatement. This may be a breaking change.
  3. Fix no-use-before-define rule to check for scopes.

@mysticatea mysticatea added the blocked This change can't be completed until another issue is resolved label Apr 21, 2018
@mysticatea mysticatea removed the blocked This change can't be completed until another issue is resolved label May 24, 2018
kaicataldo pushed a commit that referenced this issue Jun 9, 2018
…0396)

* Update: fix false negative in no-use-before-define (fixes #10227)

* reorder tests
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Dec 8, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants