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

Fails to properly parse for..of loops with const bindings #2334

Closed
jkrems opened this Issue Apr 23, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@jkrems
Copy link

jkrems commented Apr 23, 2015

The following is valid ES6 code that prevents accidental assignments to x inside of the loop body:

/*jshint esnext:true, node:true*/
'use strict';
for (const x of [ 1, 2, 3 ]) { console.log('x', x); }

Running with io.js 1.8:

> node -e "/*jshint esnext:true, node:true*/ 'use strict'; for (const x of [ 1, 2, 3 ]) { console.log('x', x); }"
x 1
x 2
x 3

But jshint doesn't seem to expect a const token in that position, interpreting it as an identifier:

> jshint <(echo "/*jshint esnext:true, node:true*/ 'use strict'; for (const x of [ 1, 2, 3 ]) { console.log(x); }") 
/dev/fd/11: line 1, col 54, Creating global 'for' variable. Should be 'for (var const ...'.
/dev/fd/11: line 1, col 60, Expected 'of' and instead saw 'x'.
/dev/fd/11: line 1, col 68, Expected ']' to match '[' from line 1 and instead saw ','.
/dev/fd/11: line 1, col 70, Expected ')' to match '(' from line 1 and instead saw '2'.
/dev/fd/11: line 1, col 71, Expected an identifier and instead saw ','.
/dev/fd/11: line 1, col 71, Expected an assignment or function call and instead saw an expression.
/dev/fd/11: line 1, col 72, Missing semicolon.
/dev/fd/11: line 1, col 73, Expected an assignment or function call and instead saw an expression.
/dev/fd/11: line 1, col 74, Missing semicolon.
/dev/fd/11: line 1, col 75, Expected an identifier and instead saw ']'.
/dev/fd/11: line 1, col 75, Expected an assignment or function call and instead saw an expression.
/dev/fd/11: line 1, col 76, Missing semicolon.
/dev/fd/11: line 1, col 76, Expected an identifier and instead saw ')'.
/dev/fd/11: line 1, col 76, Expected an assignment or function call and instead saw an expression.
/dev/fd/11: line 1, col 77, Missing semicolon.
/dev/fd/11: line 1, col 62, 'of' is not defined.
/dev/fd/11: line 1, col 92, 'x' is not defined.

17 errors

The same code does work as expected with var or let instead of const.

@caitp caitp self-assigned this Apr 23, 2015

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 23, 2015

I think #2326 is sort of a blocker for this, but I've got a quick patch... 90% sure it just reverts something that was removed because const is broken atm

caitp added a commit to caitp/jshint that referenced this issue Apr 23, 2015

caitp added a commit to caitp/jshint that referenced this issue Apr 23, 2015

caitp added a commit to caitp/jshint that referenced this issue Apr 24, 2015

@caitp caitp closed this in 2b673d9 Apr 24, 2015

@jkrems

This comment has been minimized.

Copy link

jkrems commented Apr 24, 2015

Wow, that was quick! Thanks for taking this on. :)

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 25, 2015

@jkrems it's still going to be broken because they're not correctly block-scoped, so you'll see a lot of "cont variable redeclared" errors until the other thing is fixed (unless you're very careful not to reuse the variable names)

@jkrems

This comment has been minimized.

Copy link

jkrems commented Apr 25, 2015

Thanks for the warning! Will keep that in mind.

@jayphelps

This comment has been minimized.

Copy link

jayphelps commented May 8, 2015

@caitp Even with this fix (working against master), I'm getting the same error when destructuring:

for (const { foo } of bar) {}

babel REPL

Can you confirm if this syntax is suppose to be supported by jshint? If so, I'll open a new ticket or you can repurpose this one...

Nevermind..........it wasn't picking up the new version of the module due to one of my other deps depending on its own jshint as well. i.e. it works correctly in master, I just wasn't using that code.

Carry on... ☺️

jugglinmike added a commit to jugglinmike/jshint that referenced this issue May 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment