Unexected 'Invalid left-hand side in for-in'

[pelchats]

Esprima throws an error when parsing seemingly correct code with a for-in loop. The error seems to be sensitive to the code surrounding it.

Steps to reproduce

esprima.parse(`
  if (a) {
    for(f(); false;) {}
  } else
    for(x in y) {
      g()
    }
`)

Note that this is a ~minimal reproduction. The actual code that I was trying to parse is http://pagead2.googlesyndication.com/pagead/js/r20180312/r20170110/reactive_library.js

Expected output

I think the AST should be parsed successfully. I don't see anything wrong with the above code. In particular, changing the f() call in the first for loop somehow makes the error disappear. Likewise adding curly braces after else also makes the error disappear.

Chrome executes the following code just fine:

  var a = false;
  function f() {
    console.log('f called');
  }
  function g() {
    console.log('g called');
  }
  var c = 0;
  var e = 1;
  var y = [1];
  if (a) {
    for(f(); false;) {}
  } else
    for(x in y) {
      g()
  }

> g called

Actual output

Error: Line 5: Invalid left-hand side in for-in

[JosephPecoraro]

As I understand it, this is because the parseForDeclaration() parses the init portion as an inheritCoverGrammar. And if the init portion causes isAssignmentTarget to be false, then it will affect later productions.

If the init is followed by in or of then the inherit works out as desired. But if it falls through to the classic for loop parse for (<init>;<condition>;<update>), then we really would have wanted to parse the init as an isolated expression. Note it should probably also parse the test and update portions as isolated.

The issue manifests inside the else because the else clause is immediately parsed as a sub-task of the if statement (this.alternate). The improper state coming out of the parseForDeclaration starts parsing the else clause statement with this.context.isAssignmentTarget being false instead of true.

I see two possible fixes:

1. Fix the unexpected context state manipulation inside of parseForDeclaration. (Seems like the most correct solution, but requires a comment).
2. Always adjust the state for else inside of something like parseIfClause. (Easy, but masks the issue.)