Permalink
Browse files

[[FEAT]] support destructuring in ForIn/Of loops, lint bad ForIn/Of LHS

Adds support for destructuring bindings in for loops

Closes #2341
  • Loading branch information...
caitp committed May 9, 2015
1 parent 162e8f7 commit c0edd9ff84a68946c615198a4a6f188f6fbbd54d
Showing with 804 additions and 43 deletions.
  1. +60 −36 src/jshint.js
  2. +2 −1 src/messages.js
  3. +33 −1 tests/unit/core.js
  4. +709 −5 tests/unit/parser.js
@@ -3475,25 +3475,23 @@ var JSHINT = (function() {
}
}
}
if (prefix) {
break;
}

statement.first = statement.first.concat(names);

if (isConst && state.tokens.next.id !== "=") {
if (!prefix && isConst && state.tokens.next.id !== "=") {
warning("E012", state.tokens.curr, state.tokens.curr.value);
}

if (state.tokens.next.id === "=") {
advance("=");
if (state.tokens.next.id === "undefined") {
if (!prefix && state.tokens.next.id === "undefined") {
warning("W080", state.tokens.prev, state.tokens.prev.value);
}
if (peek(0).id === "=" && state.tokens.next.identifier) {
if (!prefix && peek(0).id === "=" && state.tokens.next.identifier) {
warning("W120", state.tokens.next, state.tokens.next.value);
}
value = expression(10);
// don't accept `in` in expression if prefix is used for ForIn/Of loop.
value = expression(prefix ? 120 : 10);
if (lone) {
tokens[0].first = value;
} else {
@@ -3531,6 +3529,10 @@ var JSHINT = (function() {
var inexport = context && context.inexport;
var tokens, lone, value;

// If the `implied` option is set, bindings are set differently.
var implied = context && context.implied;
var report = !(context && context.ignore);

this.first = [];
for (;;) {
var names = [];
@@ -3554,7 +3556,7 @@ var JSHINT = (function() {
warning("E011", null, t.id);
}
}
if (funct["(global)"]) {
if (!implied && funct["(global)"]) {
if (predefined[t.id] === false) {
warning("W079", t.token, t.id);
} else if (state.option.futurehostile === false) {
@@ -3565,17 +3567,29 @@ var JSHINT = (function() {
}
}
if (t.id) {
addlabel(t.id, { type: "unused",
token: t.token });
if (implied === "for") {
var ident = t.token.value;
switch (funct[ident]) {
case "unused":
funct[ident] = "var";
break;
case "var":
break;
default:
if (!funct["(blockscope)"].getlabel(ident) &&
!(scope[ident] || {})[ident]) {
if (report) warning("W088", t.token, ident);
}
}
} else {
addlabel(t.id, { type: "unused", token: t.token });
}
names.push(t.token);
}
}
}
if (prefix) {
break;
}

if (state.option.varstmt) {
if (!prefix && report && state.option.varstmt) {
warning("W132", this);
}

@@ -3585,16 +3599,17 @@ var JSHINT = (function() {
state.nameStack.set(state.tokens.curr);

advance("=");
if (state.tokens.next.id === "undefined") {
if (!prefix && report && state.tokens.next.id === "undefined") {
warning("W080", state.tokens.prev, state.tokens.prev.value);
}
if (peek(0).id === "=" && state.tokens.next.identifier) {
if (!funct["(params)"] ||
funct["(params)"].indexOf(state.tokens.next.value) === -1) {
if (!prefix && report &&
!funct["(params)"] || funct["(params)"].indexOf(state.tokens.next.value) === -1) {
warning("W120", state.tokens.next, state.tokens.next.value);
}
}
value = expression(10);
// don't accept `in` in expression if prefix is used for ForIn/Of loop.
value = expression(prefix ? 120 : 10);
if (lone) {
tokens[0].first = value;
} else {
@@ -4134,17 +4149,40 @@ var JSHINT = (function() {
var nextop; // contains the token of the "in" or "of" operator
var i = 0;
var inof = ["in", "of"];
var level = 0; // BindingPattern "level" --- level 0 === no BindingPattern
var comma; // First comma punctuator at level 0
var initializer; // First initializer at level 0

// If initial token is a BindingPattern, count it as such.
if (checkPunctuators(state.tokens.next, ["{", "["])) ++level;
do {
nextop = peek(i);
++i;
} while (!_.contains(inof, nextop.value) && nextop.value !== ";" && nextop.type !== "(end)");
if (checkPunctuators(nextop, ["{", "["])) ++level;
else if (checkPunctuators(nextop, ["}", "]"])) --level;
if (level < 0) break;
if (level === 0) {
if (!comma && checkPunctuators(nextop, [","])) comma = nextop;
else if (!initializer && checkPunctuators(nextop, ["="])) initializer = nextop;
}
} while (level > 0 || !_.contains(inof, nextop.value) && nextop.value !== ";" &&
nextop.type !== "(end)"); // Is this a JSCS bug? This looks really weird.

// if we're in a for (… in|of …) statement
if (_.contains(inof, nextop.value)) {
if (!state.option.inESNext() && nextop.value === "of") {
error("W104", nextop, "for of");
}

var ok = !(initializer || comma);
if (initializer) {
error("W133", comma, nextop.value, "initializer is forbidden");
}

if (comma) {
error("W133", comma, nextop.value, "more than one ForBinding");
}

if (state.tokens.next.id === "var") {
advance("var");
state.tokens.curr.fud({ prefix: true });
@@ -4154,24 +4192,10 @@ var JSHINT = (function() {
letscope = true;
funct["(blockscope)"].stack();
state.tokens.curr.fud({ prefix: true });
} else if (!state.tokens.next.identifier) {
error("E030", state.tokens.next, state.tokens.next.type);
advance();
} else {
switch (funct[state.tokens.next.value]) {
case "unused":
funct[state.tokens.next.value] = "var";
break;
case "var":
break;
default:
var ident = state.tokens.next.value;
if (!funct["(blockscope)"].getlabel(ident) &&
!(scope[ident] || {})[ident]) {
warning("W088", state.tokens.next, state.tokens.next.value);
}
}
advance();
// Parse as a var statement, with implied bindings. Ignore errors if an error
// was already reported
Object.create(varstatement).fud({ prefix: true, implied: "for", ignore: !ok });
}
advance(nextop.value);
expression(20);
@@ -207,7 +207,8 @@ var warnings = {
"different variable name to avoid migration issues.",
W130: "Invalid element after rest element.",
W131: "Invalid parameter after rest parameter.",
W132: "`var` declarations are forbidden. Use `let` or `const` instead."
W132: "`var` declarations are forbidden. Use `let` or `const` instead.",
W133: "Invalid for-{a} loop left-hand-side: {b}."
};

var info = {
@@ -633,9 +633,41 @@ exports.testForIn = function (test) {
];

TestRun(test)
.addError(2, "Expected an identifier and instead saw '(string)'.")
.addError(2, "Expected an identifier and instead saw 'i'.")
.test(src);

src = [
"(function (o) {",
"for (i, j in o) { i(); }",
"for (var x, u in o) { x(); }",
"for (z = 0 in o) { z(); }",
"for (var q = 0 in o) { q(); }",
"})();"
];

TestRun(test, "bad lhs errors")
.addError(2, "Invalid for-in loop left-hand-side: more than one ForBinding.")
.addError(3, "Invalid for-in loop left-hand-side: more than one ForBinding.")
.addError(4, "Invalid for-in loop left-hand-side: initializer is forbidden.")
.addError(5, "Invalid for-in loop left-hand-side: initializer is forbidden.")
.test(src);

src = [
"(function (o) {",
"for (let i, j in o) { i(); }",
"for (const x, u in o) { x(); }",
"for (let z = 0 in o) { z(); }",
"for (const q = 0 in o) { q(); }",
"})();"
];

TestRun(test, "bad lhs errors (lexical)")
.addError(2, "Invalid for-in loop left-hand-side: more than one ForBinding.")
.addError(3, "Invalid for-in loop left-hand-side: more than one ForBinding.")
.addError(4, "Invalid for-in loop left-hand-side: initializer is forbidden.")
.addError(5, "Invalid for-in loop left-hand-side: initializer is forbidden.")
.test(src, { esnext: true });

test.done();
};

Oops, something went wrong.

0 comments on commit c0edd9f

Please sign in to comment.