Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let, const as VariableDeclaration? #113

Closed
rwaldron opened this issue Aug 14, 2014 · 11 comments
Closed

let, const as VariableDeclaration? #113

rwaldron opened this issue Aug 14, 2014 · 11 comments

Comments

@rwaldron
Copy link

Currently let and const nodes have type: "VariableDeclaration", but these syntactic forms aren't defined as VariableDeclaration in the specification. The correct grammar definition is
LexicalDeclaration, defined here: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-let-and-const-declarations

@ArtemGovorov
Copy link
Contributor

LexicalDeclaration is not supported by escodegen, esprima also generates VariableDeclaration for let and const AFAIK. I'd postpone fixing it before escodegen supports it, or do it in sync, otherwise produced AST will break code generation.

@rwaldron
Copy link
Author

@ArtemGovorov

esprima also generates VariableDeclaration for let and const AFAIK.

If it's doing this for ES6 parsing, then it's wrong. let and const have syntactic and runtime semantics that are very different from var.

@getify
Copy link

getify commented Aug 15, 2014

@rwaldron side question: what "syntactics" are different? I totally understand the different runtime semantics.

But moreover, I think it's a valid point that tools which cooperate in an ecosystem have to coordinate evolution. One could argue that a code generator needs to evolve with or before a code parser, because if the other way around, the code generator breaks on unrecognized tree syntax.

@michaelficarra
Copy link

The entire basis of this conversation is ill-founded. If a parser API node's type happens to align with the name of a grammatical production in ECMA-262, it is solely by coincidence. They are unrelated. Representing let/const statements as VariableDeclaration nodes is perfectly acceptable because they are structurally identical. Remember that these nodes have a kind that distinguishes var/let/const. If you would like this changed, the acorn/esprima Github issues are an inappropriate place to have this discussion. File a bug on mozilla's bugzilla. Here is an example proposal: https://bugzilla.mozilla.org/show_bug.cgi?id=1044462

@rwaldron
Copy link
Author

@michaelficarra

The entire basis of this conversation is ill-founded.

Reporting a parsing bug to a parser is not ill-founded.

If a parser API node's type happens to align with the name of a grammatical production in ECMA-262, it is solely by coincidence. They are unrelated.

I see, I thought this practice was to be intentionally aligned with the spec, my mistake.

Representing let/const statements as VariableDeclaration nodes is perfectly acceptable because they are structurally identical.

Except when they're not.

Remember that these nodes have a kind that distinguishes var/let/const.

Yes, I saw that and still filed the bug because it's insufficient.

If you would like this changed, the acorn/esprima Github issues are an inappropriate place to have this discussion.

Again, parser bug reported to a parser.

File a bug on mozilla's bugzilla. Here is an example proposal: https://bugzilla.mozilla.org/show_bug.cgi?id=1044462

Am I to assume that what you actually mean is that Reflect.parse needs to be updated? Surely you didn't actually expect me to report this same bug on a ticket filed for some unrelated syntax.

@getify
Copy link

getify commented Aug 15, 2014

@rwaldron I think his point is that the SpiderMonkey parser API defines the "one true standard" for tree syntax, and all other (conforming) tools simply rely on that syntax. So, if there's a desire to change the syntax of the tree, such as it being insufficient for certain syntax/grammar, then the spidermonkey parser API is what needs to change first.

@rwaldron
Copy link
Author

@getify thanks, that's much clearer.

@marijnh
Copy link
Member

marijnh commented Aug 15, 2014

@rwaldon Since the parser API as described on the Mozilla wiki does not cover ES6, but rather Mozilla's own JS 1.8, there is no standard to follow. If someone would actually fix this, and update the API document, I would happily make Acorn conform to that. For now, I consider Esprima the normative tool, and Acorn is following that. If you can convince them to change their format, ping us, and we'll follow.

@rwaldron
Copy link
Author

@marijnh these are understandable constraints, thanks for the additional clarification.

@RReverser
Copy link
Member

@marijnh I think we can close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants