Skip to content

Commit

Permalink
[[FIX]] Correct implementation of spread/rest
Browse files Browse the repository at this point in the history
Previously, the spread/rest operator was implemented as a "prefix"
operator. This incorrectly enabled its use in generic expression
contexts, as in:

    ...x;

Re-implement as an "infix" operator and implement a helper function for
parsing it in only those contexts where it is valid:

- parameter lists
- call expressions
- array initializers
- destructuring element patterns

This change triggers failures in a number of Test262 tests that were
previously interpreted as "valid." These tests concern the "object
spread/rest" proposal, but support for this proposal has not yet been
implemented in JSHint. Therefor, their previous "passing" status was
circumstantial, and the newly-identified error should be marked as
"expected."
  • Loading branch information
jugglinmike committed Dec 23, 2018
1 parent 5ca8b1a commit bd0ae0d
Show file tree
Hide file tree
Showing 3 changed files with 402 additions and 649 deletions.
128 changes: 47 additions & 81 deletions src/jshint.js
Original file line number Diff line number Diff line change
Expand Up @@ -1493,43 +1493,49 @@ var JSHINT = (function() {
return val;
}

/**
* Consume the "..." token which designates "spread" and "rest" operations if
* it is present. If the operator is repeated, consume every repetition, and
* issue a single error describing the syntax error.
*
* @returns {boolean} a value describing whether or not any tokens were
* consumed in this way
*/
function spreadrest() {
if (!checkPunctuator(state.tokens.next, "...")) {
return false;
}

if (!state.inES6(true)) {
warning("W119", state.tokens.next, "spread/rest operator", "6");
}
advance();

if (checkPunctuator(state.tokens.next, "...")) {
warning("E024", state.tokens.next, "...");
while (checkPunctuator(state.tokens.next, "...")) {
advance();
}
}

return true;
}

// prop means that this identifier is that of an object property
function identifier(prop) {
var i = optionalidentifier(prop, false);
if (i) {
return i;
}

// parameter destructuring with rest operator
if (state.tokens.next.value === "...") {
if (!state.inES6(true)) {
warning("W119", state.tokens.next, "spread/rest operator", "6");
}
advance();

if (checkPunctuator(state.tokens.next, "...")) {
warning("E024", state.tokens.next, "...");
while (checkPunctuator(state.tokens.next, "...")) {
advance();
}
}

if (!state.tokens.next.identifier) {
warning("E024", state.tokens.curr, state.tokens.next.id);
return;
}

return identifier(prop);
} else {
error("E030", state.tokens.next, state.tokens.next.value);
error("E030", state.tokens.next, state.tokens.next.value);

// The token should be consumed after a warning is issued so the parser
// can continue as though an identifier were found. The semicolon token
// should not be consumed in this way so that the parser interprets it as
// a statement delimeter;
if (state.tokens.next.id !== ";") {
advance();
}
// The token should be consumed after a warning is issued so the parser
// can continue as though an identifier were found. The semicolon token
// should not be consumed in this way so that the parser interprets it as
// a statement delimeter;
if (state.tokens.next.id !== ";") {
advance();
}
}

Expand Down Expand Up @@ -2259,51 +2265,7 @@ var JSHINT = (function() {
return this;
});

prefix("...", function() {
if (!state.inES6(true)) {
warning("W119", this, "spread/rest operator", "6");
}

// TODO: Allow all AssignmentExpression
// once parsing permits.
//
// How to handle eg. number, boolean when the built-in
// prototype of may have an @@iterator definition?
//
// Number.prototype[Symbol.iterator] = function * () {
// yield this.valueOf();
// };
//
// var a = [ ...1 ];
// console.log(a); // [1];
//
// for (let n of [...10]) {
// console.log(n);
// }
// // 10
//
//
// Boolean.prototype[Symbol.iterator] = function * () {
// yield this.valueOf();
// };
//
// var a = [ ...true ];
// console.log(a); // [true];
//
// for (let n of [...false]) {
// console.log(n);
// }
// // false
//
if (!state.tokens.next.identifier &&
state.tokens.next.type !== "(string)" &&
!checkPunctuators(state.tokens.next, ["[", "("])) {

error("E030", state.tokens.next, state.tokens.next.value);
}
this.right = expression(150);
return this;
});
infix("...");

prefix("!", function() {
this.arity = "unary";
Expand Down Expand Up @@ -2467,6 +2429,8 @@ var JSHINT = (function() {

if (state.tokens.next.id !== ")") {
for (;;) {
spreadrest();

p[p.length] = expression(10);
n += 1;
if (state.tokens.next.id !== ",") {
Expand Down Expand Up @@ -2789,6 +2753,8 @@ var JSHINT = (function() {
break;
}

spreadrest();

this.first.push(expression(10));
if (state.tokens.next.id === ",") {
parseComma({ allowTrailing: true });
Expand Down Expand Up @@ -2914,7 +2880,7 @@ var JSHINT = (function() {
}
}
} else {
if (checkPunctuator(state.tokens.next, "...")) pastRest = true;
pastRest = spreadrest();
ident = identifier();
if (ident) {
paramsIds.push(ident);
Expand Down Expand Up @@ -3464,8 +3430,6 @@ var JSHINT = (function() {
nextInnerDE();
advance(")");
} else {
var is_rest = checkPunctuator(state.tokens.next, "...");

if (isAssignment) {
var assignTarget = expression(20);
if (assignTarget) {
Expand All @@ -3482,9 +3446,7 @@ var JSHINT = (function() {
if (ident) {
identifiers.push({ id: ident, token: state.tokens.curr });
}
return is_rest;
}
return false;
};
var assignmentProperty = function() {
var id;
Expand Down Expand Up @@ -3525,12 +3487,16 @@ var JSHINT = (function() {
}
var element_after_rest = false;
while (!checkPunctuator(state.tokens.next, "]")) {
if (nextInnerDE() && !element_after_rest &&
var isRest = spreadrest();

nextInnerDE();

if (isRest && !element_after_rest &&
checkPunctuator(state.tokens.next, ",")) {
warning("W130", state.tokens.next);
element_after_rest = true;
}
if (checkPunctuator(state.tokens.next, "=")) {
if (!isRest && checkPunctuator(state.tokens.next, "=")) {
if (checkPunctuator(state.tokens.prev, "...")) {
advance("]");
} else {
Expand Down

0 comments on commit bd0ae0d

Please sign in to comment.