Detect when from in a for loop declaration is an identifier

[GeoffreyBooth]

Per #4390, this should be valid:

lines = [
  {from: 0, to: 10}
  {from: 3, to: 17}
]

for {from, to} in lines
  console.log from

But it throws an error on thinking that from is a keyword. Since from has never been a reserved word (outside the context of an import or export statement), we should try to detect when a from on a for declaration line is really a keyword or rather a variable identifier.

I’ve tried to make the detection by looking at the previous token. If the previous token is an identifier itself, like for a from b I think that that would mean that the present from is always a keyword. If the previous token is { or [ or ,, then we’re inside destructuring and from is always an identifier. If the previous token value is from, we have for from from a or for a from from and the first from is an identifier and the second is a keyword.

Are there any other cases I haven’t thought of?

[lydell]

Are there any other cases I haven’t thought of?

I can't think of more for…from cases, but yield from from came to mind. Not sure if it belongs in this PR, though.

Other than that this looks good to me.

[GeoffreyBooth]

This bug is only because the lexer (previously) was declaring a from token to be FROM (instead of IDENTIFIER) if there was a for anywhere previously on the same line. Now the lexer is more discerning about when from should be considered FROM.

yield from from works already because the lexer there only declares the first from a FROM if the previous tag is yield.

[GeoffreyBooth]

@IvanVergiliev or @vendethiel, can you think of any other cases where from might be a variable name and not a keyword in a for loop declaration, other than the ones in the tests in this PR?

[connec]

Nit: would isForFrom be an acceptable, more concise name?

[GeoffreyBooth]

Yes 👍

[connec]

Nit: if prev[1] in '{[' would save an array allocation (not really sure if that's meaningful).

[GeoffreyBooth]

if prev[1] in '{[' looks confusing to me. The way I wrote it seems to be the style of the codebase.

[IvanVergiliev]

Thanks for the quick fix!

How about destructuring with aliasing (not sure if that's the right term):

for {b: from} in a
  print from

? This is broken right now as well, and I think there might have to be a separate check for it in the lexer. (Also, all this special casing is making me a bit sad, but I guess there's no other way to detect this so it's much better than before.)

[vendethiel]

@IvanVergiliev or @vendethiel, can you think of any other cases where from might be a variable name and not a keyword in a for loop declaration, other than the ones in the tests in this PR?

./bin/coffee -bce 'for {a: from} from 1 then 1'
[stdin]:1:9: error: unexpected from
for {a: from} from 1 then 1
        ^^^^
./bin/coffee -bce 'for [a: from] from 1 then 1'
[stdin]:1:9: error: unexpected from
for [a: from] from 1 then 1
        ^^^^

Somewhat reminds me of satyr/coco#195 (though we don't have this exact issue when you can have a typeless for).

[GeoffreyBooth]

Thanks, anything else?

[IvanVergiliev]

I can't think of any other cases.

[GeoffreyBooth]

@lydell we should probably release 1.12.1. I can prep it tonight.