-
-
Notifications
You must be signed in to change notification settings - Fork 669
Allow keywords in named import/export syntax #107
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
Allow keywords in named import/export syntax #107
Conversation
Progress toward AssemblyScript#98 See the imports and exports grammar from the spec: https://www.ecma-international.org/ecma-262/8.0/index.html#sec-imports https://www.ecma-international.org/ecma-262/8.0/index.html#sec-exports In particular, ImportSpecifier and ExportSpecifier use IdentifierName to identify the import/export name rather than the more restrictive ImportedBinding (or just Identifier). That means any keyword is allowed. There are (at least) three contexts to think about in JS identifier parsing: * In most contexts, identifiers like variable names can be any name except a reserved word. * Some keywords, like "as", are normally not keywords but have special keyword-like meaning in specific situations. Acorn and Babel call these "contextual keywords", and `tokenIsAlsoIdentifier` and `preferIdentifier` in AssemblyScript appear to handle that case. * In some contexts, like object keys and named imports/exports, any name is allowed, even keywords. This change adds that third case in order to handle imports/exports. I added a `forceIdentifier` flag that tells the tokenizer that any identifier name should be treated as a plain identifier, then used it in the import and export cases.
Now that there are effectively three modes, what do you think of adding an enum that indicates how to handle identifiers? Something like enum IdentifierHandling {
DEFAULT,
PREFER,
ALWAYS
} |
Sure, sounds reasonable to me. (It wouldn't surprise me if it got more complicated than that in more obscure cases, but the three modes seems fine for now.) |
src/tokenizer.ts
Outdated
/** | ||
* Skip any name token, whether or not it is a keyword. | ||
*/ | ||
skipIdentifierName(): bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this become an IdentifierHandling
option on skip()
, maybe? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense, yeah this copied function is a bit ugly. I guess it felt a bit weird given the special case in skip
.
How about this: I get rid of the special case in skip
and specify it using an optional param like you suggested. Then, to make the common case the most natural to write, I'll make skipIdentifier
as a shorthand for skip(Token.IDENTIFIER, IdentifierHandling.PREFER)
and keep skipIdentifierName
as a shorthand for skip(Token.IDENTIFIER, IdentifierHandling.ALWAYS)
. Let me know what you think.
Great, thank you! :) |
Progress toward #98
See the imports and exports grammar from the spec:
https://www.ecma-international.org/ecma-262/8.0/index.html#sec-imports
https://www.ecma-international.org/ecma-262/8.0/index.html#sec-exports
In particular, ImportSpecifier and ExportSpecifier use IdentifierName to
identify the import/export name rather than the more restrictive ImportedBinding
(or just Identifier). That means any keyword is allowed.
There are (at least) three contexts to think about in JS identifier parsing:
reserved word.
keyword-like meaning in specific situations. Acorn and Babel call these
"contextual keywords", and
tokenIsAlsoIdentifier
andpreferIdentifier
inAssemblyScript appear to handle that case.
allowed, even keywords.
This change adds that third case in order to handle imports/exports. I added a
forceIdentifier
flag that tells the tokenizer that any identifier name shouldbe treated as a plain identifier, then used it in the import and export cases.