Rephrase "assigning/binding to rvalue" errors to include context (#119)…

… (#123)

* Rephrase "assigning/binding to rvalue" error messages with context (#119)

* Fix code style in parser/lval.js

* istanbul ignore some unused branches in parser/lval.js

* Fix code style again in parser/lval.js

src/parser/expression.js

@@ -113,10 +113,10 @@ pp.parseMaybeAssign = function (noIn, refShorthandDefaultPos, afterLeftParse, re
   if (this.state.type.isAssign) {
     let node = this.startNodeAt(startPos, startLoc);
     node.operator = this.state.value;
-    node.left = this.match(tt.eq) ? this.toAssignable(left) : left;
+    node.left = this.match(tt.eq) ? this.toAssignable(left, undefined, "assignment expression") : left;
     refShorthandDefaultPos.start = 0; // reset because shorthand default was used correctly
 
-    this.checkLVal(left);
+    this.checkLVal(left, undefined, undefined, "assignment expression");
 
     if (left.extra && left.extra.parenthesized) {
       let errorMsg;
@@ -232,7 +232,7 @@ pp.parseMaybeUnary = function (refShorthandDefaultPos) {
     }
 
     if (update) {
-      this.checkLVal(node.argument);
+      this.checkLVal(node.argument, undefined, undefined, "prefix operation");
     } else if (this.state.strict && node.operator === "delete" && node.argument.type === "Identifier") {
       this.raise(node.start, "Deleting local variable in strict mode");
     }
@@ -248,7 +248,7 @@ pp.parseMaybeUnary = function (refShorthandDefaultPos) {
     node.operator = this.state.value;
     node.prefix = false;
     node.argument = expr;
-    this.checkLVal(expr);
+    this.checkLVal(expr, undefined, undefined, "postfix operation");
     this.next();
     expr = this.finishNode(node, "UpdateExpression");
   }
@@ -855,7 +855,7 @@ pp.parseMethod = function (node, isGenerator, isAsync) {
 
 pp.parseArrowExpression = function (node, params, isAsync) {
   this.initFunction(node, isAsync);
-  node.params = this.toAssignableList(params, true);
+  node.params = this.toAssignableList(params, true, "arrow function parameters");
   this.parseFunctionBody(node, true);
   return this.finishNode(node, "ArrowFunctionExpression");
 };
@@ -911,13 +911,13 @@ pp.parseFunctionBody = function (node, allowExpression) {
     let oldStrict = this.state.strict;
     if (isStrict) this.state.strict = true;
     if (node.id) {
-      this.checkLVal(node.id, true);
+      this.checkLVal(node.id, true, undefined, "function name");
     }
     for (let param of (node.params: Array<Object>)) {
       if (isStrict && param.type !== "Identifier") {
         this.raise(param.start, "Non-simple parameter in strict mode");
       }
-      this.checkLVal(param, true, nameHash);
+      this.checkLVal(param, true, nameHash, "function parameter list");
     }
     this.state.strict = oldStrict;
   }

src/parser/lval.js

@@ -9,7 +9,7 @@ const pp = Parser.prototype;
 // Convert existing expression atom to assignable pattern
 // if possible.
 
-pp.toAssignable = function (node, isBinding) {
+pp.toAssignable = function (node, isBinding, contextDescription) {
   if (node) {
     switch (node.type) {
       case "Identifier":
@@ -28,13 +28,13 @@ pp.toAssignable = function (node, isBinding) {
               this.raise(prop.key.start, "Object pattern can't contain methods");
             }
           } else {
-            this.toAssignable(prop, isBinding);
+            this.toAssignable(prop, isBinding, "object destructuring pattern");
           }
         }
         break;
 
       case "ObjectProperty":
-        this.toAssignable(node.value, isBinding);
+        this.toAssignable(node.value, isBinding, contextDescription);
         break;
 
       case "SpreadProperty":
@@ -43,7 +43,7 @@ pp.toAssignable = function (node, isBinding) {
 
       case "ArrayExpression":
         node.type = "ArrayPattern";
-        this.toAssignableList(node.elements, isBinding);
+        this.toAssignableList(node.elements, isBinding, contextDescription);
         break;
 
       case "AssignmentExpression":
@@ -58,16 +58,19 @@ pp.toAssignable = function (node, isBinding) {
       case "MemberExpression":
         if (!isBinding) break;
 
-      default:
-        this.raise(node.start, "Assigning to rvalue");
+      default: {
+        const message = "Invalid left-hand side" +
+          (contextDescription ? " in " + contextDescription : /* istanbul ignore next */ "expression");
+        this.raise(node.start, message);
+      }
     }
   }
   return node;
 };
 
 // Convert list of expression atoms to binding list.
 
-pp.toAssignableList = function (exprList, isBinding) {
+pp.toAssignableList = function (exprList, isBinding, contextDescription) {
   let end = exprList.length;
   if (end) {
     let last = exprList[end - 1];
@@ -76,7 +79,7 @@ pp.toAssignableList = function (exprList, isBinding) {
     } else if (last && last.type === "SpreadElement") {
       last.type = "RestElement";
       let arg = last.argument;
-      this.toAssignable(arg, isBinding);
+      this.toAssignable(arg, isBinding, contextDescription);
       if (arg.type !== "Identifier" && arg.type !== "MemberExpression" && arg.type !== "ArrayPattern") {
         this.unexpected(arg.start);
       }
@@ -85,7 +88,7 @@ pp.toAssignableList = function (exprList, isBinding) {
   }
   for (let i = 0; i < end; i++) {
     let elt = exprList[i];
-    if (elt) this.toAssignable(elt, isBinding);
+    if (elt) this.toAssignable(elt, isBinding, contextDescription);
   }
   return exprList;
 };
@@ -198,7 +201,7 @@ pp.parseMaybeDefault = function (startPos, startLoc, left) {
 // Verify that a node is an lval — something that can be assigned
 // to.
 
-pp.checkLVal = function (expr, isBinding, checkClashes) {
+pp.checkLVal = function (expr, isBinding, checkClashes, contextDescription) {
   switch (expr.type) {
     case "Identifier":
       if (this.state.strict && (reservedWords.strictBind(expr.name) || reservedWords.strict(expr.name))) {
@@ -234,26 +237,33 @@ pp.checkLVal = function (expr, isBinding, checkClashes) {
     case "ObjectPattern":
       for (let prop of (expr.properties: Array<Object>)) {
         if (prop.type === "ObjectProperty") prop = prop.value;
-        this.checkLVal(prop, isBinding, checkClashes);
+        this.checkLVal(prop, isBinding, checkClashes, "object destructuring pattern");
       }
       break;
 
     case "ArrayPattern":
       for (let elem of (expr.elements: Array<Object>)) {
-        if (elem) this.checkLVal(elem, isBinding, checkClashes);
+        if (elem) this.checkLVal(elem, isBinding, checkClashes, "array destructuring pattern");
       }
       break;
 
     case "AssignmentPattern":
-      this.checkLVal(expr.left, isBinding, checkClashes);
+      this.checkLVal(expr.left, isBinding, checkClashes, "assignment pattern");
       break;
 
     case "RestProperty":
+      this.checkLVal(expr.argument, isBinding, checkClashes, "rest property");
+      break;
+
     case "RestElement":
-      this.checkLVal(expr.argument, isBinding, checkClashes);
+      this.checkLVal(expr.argument, isBinding, checkClashes, "rest element");
       break;
 
-    default:
-      this.raise(expr.start, (isBinding ? "Binding" : "Assigning to") + " rvalue");
+    default: {
+      const message = (isBinding ? /* istanbul ignore next */ "Binding invalid" : "Invalid") +
+        " left-hand side" +
+        (contextDescription ? " in " + contextDescription : /* istanbul ignore next */ "expression");
+      this.raise(expr.start, message);
+    }
   }
 };

src/parser/statement.js

@@ -259,8 +259,9 @@ pp.parseForStatement = function (node) {
   let refShorthandDefaultPos = {start: 0};
   let init = this.parseExpression(true, refShorthandDefaultPos);
   if (this.match(tt._in) || this.isContextual("of")) {
-    this.toAssignable(init);
-    this.checkLVal(init);
+    const description = this.isContextual("of") ? "for-of statement" : "for-in statement";
+    this.toAssignable(init, undefined, description);
+    this.checkLVal(init, undefined, undefined, description);
     return this.parseForIn(node, init, forAwait);
   } else if (refShorthandDefaultPos.start) {
     this.unexpected(refShorthandDefaultPos.start);
@@ -371,7 +372,7 @@ pp.parseTryStatement = function (node) {
 
     this.expect(tt.parenL);
     clause.param = this.parseBindingAtom();
-    this.checkLVal(clause.param, true, Object.create(null));
+    this.checkLVal(clause.param, true, Object.create(null), "catch clause");
     this.expect(tt.parenR);
 
     clause.body = this.parseBlock();
@@ -564,7 +565,7 @@ pp.parseVar = function (node, isFor, kind) {
 
 pp.parseVarHead = function (decl) {
   decl.id = this.parseBindingAtom();
-  this.checkLVal(decl.id, true);
+  this.checkLVal(decl.id, true, undefined, "variable declaration");
 };
 
 // Parse a function declaration or literal (depending on the
@@ -1021,7 +1022,7 @@ pp.parseImportSpecifiers = function (node) {
     this.next();
     this.expectContextual("as");
     specifier.local = this.parseIdentifier();
-    this.checkLVal(specifier.local, true);
+    this.checkLVal(specifier.local, true, undefined, "import namespace specifier");
     node.specifiers.push(this.finishNode(specifier, "ImportNamespaceSpecifier"));
     return;
   }
@@ -1038,14 +1039,14 @@ pp.parseImportSpecifiers = function (node) {
     let specifier = this.startNode();
     specifier.imported = this.parseIdentifier(true);
     specifier.local = this.eatContextual("as") ? this.parseIdentifier() : specifier.imported.__clone();
-    this.checkLVal(specifier.local, true);
+    this.checkLVal(specifier.local, true, undefined, "import specifier");
     node.specifiers.push(this.finishNode(specifier, "ImportSpecifier"));
   }
 };
 
 pp.parseImportSpecifierDefault = function (id, startPos, startLoc) {
   let node = this.startNodeAt(startPos, startLoc);
   node.local = id;
-  this.checkLVal(node.local, true);
+  this.checkLVal(node.local, true, undefined, "default import specifier");
   return this.finishNode(node, "ImportDefaultSpecifier");
 };

src/plugins/flow.js

@@ -904,25 +904,25 @@ export default function (instance) {
   });
 
   instance.extend("toAssignable", function (inner) {
-    return function (node, isBinding) {
+    return function (node, isBinding, contextDescription) {
       if (node.type === "TypeCastExpression") {
-        return inner.call(this, this.typeCastToParameter(node), isBinding);
+        return inner.call(this, this.typeCastToParameter(node), isBinding, contextDescription);
       } else {
-        return inner.call(this, node, isBinding);
+        return inner.call(this, node, isBinding, contextDescription);
       }
     };
   });
 
   // turn type casts that we found in function parameter head into type annotated params
   instance.extend("toAssignableList", function (inner) {
-    return function (exprList, isBinding) {
+    return function (exprList, isBinding, contextDescription) {
       for (let i = 0; i < exprList.length; i++) {
         let expr = exprList[i];
         if (expr && expr.type === "TypeCastExpression") {
           exprList[i] = this.typeCastToParameter(expr);
         }
       }
-      return inner.call(this, exprList, isBinding);
+      return inner.call(this, exprList, isBinding, contextDescription);
     };
   });
 

test/fixtures/core/uncategorised/367/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in assignment expression (1:0)"
 }

test/fixtures/core/uncategorised/368/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in assignment expression (1:0)"
 }

test/fixtures/core/uncategorised/369/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:1)"
+  "throws": "Invalid left-hand side in assignment expression (1:1)"
 }

test/fixtures/core/uncategorised/370/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in postfix operation (1:0)"
 }

test/fixtures/core/uncategorised/371/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in postfix operation (1:0)"
 }

test/fixtures/core/uncategorised/372/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:2)"
+  "throws": "Invalid left-hand side in prefix operation (1:2)"
 }

test/fixtures/core/uncategorised/373/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:2)"
+  "throws": "Invalid left-hand side in prefix operation (1:2)"
 }

test/fixtures/core/uncategorised/374/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:5)"
+  "throws": "Invalid left-hand side in for-in statement (1:5)"
 }

test/fixtures/core/uncategorised/383/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in assignment expression (1:0)"
 }

test/fixtures/core/uncategorised/384/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in assignment expression (1:0)"
 }

test/fixtures/core/uncategorised/417/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:5)"
+  "throws": "Invalid left-hand side in for-in statement (1:5)"
 }

test/fixtures/core/uncategorised/418/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:5)"
+  "throws": "Invalid left-hand side in for-in statement (1:5)"
 }

test/fixtures/es2015/uncategorised/220/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:0)"
+  "throws": "Invalid left-hand side in assignment expression (1:0)"
 }

test/fixtures/es2015/uncategorised/221/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:1)"
+  "throws": "Invalid left-hand side in assignment expression (1:1)"
 }

test/fixtures/es2015/uncategorised/222/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:7)"
+  "throws": "Invalid left-hand side in object destructuring pattern (1:7)"
 }

test/fixtures/es2015/uncategorised/251/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:1)"
+  "throws": "Invalid left-hand side in arrow function parameters (1:1)"
 }

test/fixtures/es2015/uncategorised/252/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:1)"
+  "throws": "Invalid left-hand side in arrow function parameters (1:1)"
 }

test/fixtures/es2015/uncategorised/284/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:3)"
+  "throws": "Invalid left-hand side in arrow function parameters (1:3)"
 }

test/fixtures/es2015/uncategorised/288/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:1)"
+  "throws": "Invalid left-hand side in assignment expression (1:1)"
 }

test/fixtures/es2015/uncategorised/37/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:2)"
+  "throws": "Invalid left-hand side in arrow function parameters (1:2)"
 }

test/fixtures/esprima/es2015-destructuring-assignment-object-pattern/invalid-lhs-01/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:4)"
+  "throws": "Invalid left-hand side in object destructuring pattern (1:4)"
 }

test/fixtures/esprima/es2015-destructuring-assignment-object-pattern/invalid-lhs-02/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "Assigning to rvalue (1:5)"
+  "throws": "Invalid left-hand side in object destructuring pattern (1:5)"
 }