Skip to content

Commit

Permalink
[[FEAT]] support destructuring in ForIn/Of loops, lint bad ForIn/Of LHS
Browse files Browse the repository at this point in the history
  • Loading branch information
caitp committed Apr 25, 2015
1 parent 2b673d9 commit 2c98590
Show file tree
Hide file tree
Showing 4 changed files with 808 additions and 47 deletions.
104 changes: 64 additions & 40 deletions src/jshint.js
Expand Up @@ -3452,25 +3452,23 @@ var JSHINT = (function() {
}
}
}
if (prefix) {
break;
}

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

if (state.tokens.next.id !== "=") {
if (!prefix && 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 {
Expand All @@ -3494,6 +3492,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 = implied !== "ignore";

this.first = [];
for (;;) {
var names = [];
Expand All @@ -3514,7 +3516,7 @@ var JSHINT = (function() {
if (state.option.inESNext() && funct[t.id] === "const") {
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) {
Expand All @@ -3525,16 +3527,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);
}

Expand All @@ -3543,15 +3558,17 @@ var JSHINT = (function() {
if (state.tokens.next.id === "=") {
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 {
Expand Down Expand Up @@ -3617,21 +3634,19 @@ var JSHINT = (function() {
}
}
}
if (prefix) {
break;
}

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

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 {
Expand Down Expand Up @@ -4175,17 +4190,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 });
Expand All @@ -4195,24 +4233,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: ok ? "for" : "ignore" });
}
advance(nextop.value);
expression(20);
Expand Down
3 changes: 2 additions & 1 deletion src/messages.js
Expand Up @@ -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 = {
Expand Down
34 changes: 33 additions & 1 deletion tests/unit/core.js
Expand Up @@ -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();
};

Expand Down

0 comments on commit 2c98590

Please sign in to comment.