Make sure that exports \{default\} throws an error

[nzakas]

This checks if the local name of an export specifier is a keyword or reserved word only when there's no from clause (the case with a from clause appears to already work, though I'm not sure why).

Fixes #326

[RReverser]

Are you sure that such check would be enough? (compared to https://github.com/marijnh/acorn/blob/master/src/expression.js#L642-L644)

[nzakas]

Based on the tests I added, I think so. parseIdent is always called first, so this code seems to be triggered already.

[RReverser]

parseIdent is called with liberal === true when keyword is default so those conditions are not triggered https://github.com/marijnh/acorn/blob/master/src/statement.js#L551.

[nzakas]

Yeah, that's for the form:

export default foo;

I haven't addressed that form in this PR as the bug was about non-default exports. Do you want me to look at that too?

[RReverser]

No, that line with parseIdent covers exactly export { default } ....

[nzakas]

Ah, I see. Still, I'm having trouble coming up with a test case that behaves incorrectly with this code. Do you have one in mind?

[RReverser]

In fact you're right, there shouldn't be as default is a keyword so falls into another condition, and others are called with liberal = false anyway. It's me just being over-careful :)

[nzakas]

No worries, I'd rather have you being over careful. :)

[RReverser]

@nzakas Can you please put some space around operators? Thanks.

[nzakas]

Done!

[nzakas]

Any other changes needed here? @marijnh

[marijnh]

Nope, looks great. But this airport wifi won't allow me to push to github, and this branch isn't based on the current master, so I don't want to use github's magic merge button which I expect will introduce needless merge commits. I'll merge this when I get home later today.

[marijnh]

Merged as 37efe0b!

[nzakas]

Thanks!