Skip to content

Commit

Permalink
[[FIX]] Improve reporting of "Bad assignment."
Browse files Browse the repository at this point in the history
When an assignment expression has an invalid left-hand side, JSHint
previously issued error E013 ("Bad assignment."), bailed out of
assignment interpretation and instead interpreted the left-hand side as
an ExpressionStatement.

In issuing E013, JSHint interprets the invalid JavaScript as an
assignment expression. Failing to parse the invalid target as the
left-hand side of the expression is inconsistent with this
interpretation. The inconsistency commonly triggers inappropriate (and
potentially confusing) errors such as "Missing semicolon," and "Expected
an assignment or function call and instead saw an expression."

Instead, JSHint should warn about the faulty assignment but continue
parsing as though the expression were a valid assignment target. Doing
so avoids emitting those errors that suggest that the left-hand side
were written as an ExpressionStatement.

1. Interpret faulty assignment targets as the left-hand side of
   assignment expressions in all cases, issuing error E013 and moving on
2. Remove inappropriate errors from affected unit tests (noting that the
   underlying "Bad assignment." error remains unchanged)
  • Loading branch information
jugglinmike committed Jan 16, 2016
1 parent 7311bdd commit 08df19e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 23 deletions.
23 changes: 13 additions & 10 deletions src/jshint.js
Original file line number Diff line number Diff line change
Expand Up @@ -1351,8 +1351,8 @@ var JSHINT = (function() {
state.nameStack.set(state.tokens.prev);
return true;
} else if (left.id === "{" || left.id === "[") {
if (allowDestructuring && state.tokens.curr.left.destructAssign) {
state.tokens.curr.left.destructAssign.forEach(function(t) {
if (allowDestructuring && left.destructAssign) {
left.destructAssign.forEach(function(t) {
if (t.id) {
state.funct["(scope)"].block.modify(t.id, t.token);
}
Expand Down Expand Up @@ -1392,12 +1392,13 @@ var JSHINT = (function() {
var x = infix(s, typeof f === "function" ? f : function(left, that) {
that.left = left;

if (left && checkLeftSideAssign(left, that, { allowDestructuring: true })) {
that.right = expression(10);
return that;
if (!checkLeftSideAssign(left, that, { allowDestructuring: true })) {
error("E031", that);
}

error("E031", that);
that.right = expression(10);

return that;
}, p);

x.exps = true;
Expand Down Expand Up @@ -1426,11 +1427,13 @@ var JSHINT = (function() {
warning("W016", that, that.id);
}

if (left && checkLeftSideAssign(left, that)) {
that.right = expression(10);
return that;
if (!checkLeftSideAssign(left, that)) {
error("E031", that);
}
error("E031", that);

that.right = expression(10);

return that;
}, 20);
}

Expand Down
15 changes: 2 additions & 13 deletions tests/unit/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ exports.assignment = function (test) {
.addError(3, "Bad assignment.")
.addError(4, "Bad assignment.")
.addError(5, "Bad assignment.")
.addError(14, "Bad assignment.")
.addError(14, "Expected an assignment or function call and instead saw an expression.")
.addError(14, "Missing semicolon.");
.addError(14, "Bad assignment.");

run.test(code, { plusplus: true, es3: true });
run.test(code, { plusplus: true }); // es5
Expand Down Expand Up @@ -1021,33 +1019,25 @@ exports["gh-2587"] = function (test) {

exports.badAssignments = function (test) {
TestRun(test)
.addError(1, "Missing semicolon.")
.addError(1, "Bad assignment.")
.addError(1, "Expected an assignment or function call and instead saw an expression.")
.test([
"a() = 1;"
], { });

TestRun(test)
.addError(1, "Missing semicolon.")
.addError(1, "Bad assignment.")
.addError(1, "Expected an assignment or function call and instead saw an expression.")
.test([
"a.a() = 1;"
], { });

TestRun(test)
.addError(1, "Missing semicolon.")
.addError(1, "Bad assignment.")
.addError(1, "Expected an assignment or function call and instead saw an expression.")
.test([
"(function(){}) = 1;"
], { });

TestRun(test)
.addError(1, "Missing semicolon.")
.addError(1, "Bad assignment.")
.addError(1, "Expected an assignment or function call and instead saw an expression.")
.test([
"a.a() &= 1;"
], { });
Expand Down Expand Up @@ -5093,8 +5083,7 @@ exports["regression test for crash from GH-964"] = function (test) {

TestRun(test)
.addError(2, "Bad assignment.")
.addError(2, "Expected an operator and instead saw 'new'.")
.addError(2, "Missing semicolon.")
.addError(2, "Did you mean to return a conditional instead of an assignment?")
.test(code);

test.done();
Expand Down

0 comments on commit 08df19e

Please sign in to comment.