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-redeclare not detecting redefinition of for...of variable #7900

Closed
oliverwoodings opened this issue Jan 11, 2017 · 5 comments
Closed

no-redeclare not detecting redefinition of for...of variable #7900

oliverwoodings opened this issue Jan 11, 2017 · 5 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@oliverwoodings
Copy link

oliverwoodings commented Jan 11, 2017

Tell us about your environment

  • ESLint Version: 3.8.1 and the online demo environment
  • Node Version: 6.2
  • npm Version: 3

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

Please show your full configuration:

{
  env: {
    es6: true
  },
  rules: {
    'no-redeclare': 2,
  }
}

What did you do? Please include the actual source code causing the issue.

  for (let foo of bar) {
    const foo = 'bar'
  }

What did you expect to happen?
I expected to receive an error about line to redeclaring foo

What actually happened? Please include the actual, raw output from ESLint.
I got no warnings or errors (screenshot is of the demo tool with only the no-redeclare rule enabled):

screen shot 2017-01-11 at 10 32 34

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 11, 2017
@not-an-aardvark
Copy link
Member

I'm not sure about this, but I think the foo declared in the loop body is actually a different variable than the one declared in the loop header. So this would fall under no-shadow instead.

If foo was referring to the same variable in both cases, I would expect your code to throw a syntax error for the same reason as this:

let foo = 1;
const foo = 1; // SyntaxError: Identifier 'foo' has already been declared

But in reality, I think the code is more similar to this:

let foo = 1;
{
  const foo = 1; // creates a new `foo` variable that shadows the other one in this block
}

@oliverwoodings
Copy link
Author

I think what is unclear is where the block scope starts. Buble throws a parsing exception for this, which is why I ended up looking into it. I guess this is maybe an issue with spec interpretation? Either Buble or Eslint have interpreted the spec incorrectly, but tbh I'm not sure which!

@not-an-aardvark
Copy link
Member

Well, there's actually a bug in ESLint's parser where an error isn't even thrown for the first example. I was basing what I said off the fact that the code is valid syntax in Chrome, but I can look at the spec a bit later to double-check.

@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 11, 2017
@oliverwoodings
Copy link
Author

Ah thats a fair point - Chrome is probably a better indicator than Buble for spec compliance!

@not-an-aardvark
Copy link
Member

Closing this issue because the no-shadow rule correctly reports this case.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 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 Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

3 participants