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

[[FIX]] parse `const` declarations in ForIn/Of loops #2335

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@caitp
Copy link
Member

caitp commented Apr 23, 2015

Closes #2334

@@ -4189,8 +4189,8 @@ var JSHINT = (function() {
if (state.tokens.next.id === "var") {
advance("var");
state.tokens.curr.fud({ prefix: true });
} else if (state.tokens.next.id === "let") {
advance("let");
} else if (state.tokens.next.id === "let" || state.tokens.next.id === "const") {

This comment has been minimized.

@caitp

caitp Apr 23, 2015

Member

TODO: should only be treating let/const specially if esnext es3 or es5 mode is used

} else if (state.tokens.next.id === "let") {
advance("let");
} else if (state.tokens.next.id === "let" || state.tokens.next.id === "const") {
advance(state.tokens.next.id);

This comment has been minimized.

@caitp

caitp Apr 23, 2015

Member

other linting rules: initializers are illegal in es6 here, as are multiple declarations

@caitp caitp force-pushed the caitp:issue-2334 branch from 89b0b26 to 590014e Apr 23, 2015

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 23, 2015

self-reviewed, think it should wait on fixing the behaviour of const though.

@rwaldron

This comment has been minimized.

Copy link
Member

rwaldron commented Apr 23, 2015

I'm mobile atm so can't compare with #2326 is there any overlap? (Sorry if that seems lazy)

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 23, 2015

not that I can see in the diff

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 23, 2015

it looks like coveralls.io is having issues

@lukeapage

This comment has been minimized.

Copy link
Member

lukeapage commented Apr 24, 2015

I don't think it conflicts with my pr :)

@caitp

This comment has been minimized.

Copy link
Member

caitp commented Apr 24, 2015

it's not conflicts that's the worry, it's that the behaviour of const is pretty broken atm =)

@lukeapage

This comment has been minimized.

Copy link
Member

lukeapage commented Apr 24, 2015

@caitp caitp force-pushed the caitp:issue-2334 branch from 590014e to c7b3121 Apr 24, 2015

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 24, 2015

Coverage Status

Coverage increased (+0.03%) to 96.36% when pulling c7b3121 on caitp:issue-2334 into dd768c2 on jshint:master.

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

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

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