Comments (7)
Hi! I'm trying to implement this and learn some Babylon internals at the same time.
tl;dr: I have a question about a specific implementation idea - see the very end
💭 Larger discussion: await
spec
The spec says here under note 1 that (emphasis mine)
yield
is Identifier or YieldExpression [ ... ] depending on context.await
is always parsed as an AwaitExpression but is a Syntax Error via static semantics. [ ... ]
Does this mean we can skip trying to disambiguate await
and just treat it as starting an AwaitExpression everywhere (i.e. immediately an error if not in an async context)? Probably not. I must be missing something. But it would make fixing this issue quite trivial. Moving on...
👉 The issue at hand
I'm assuming the intent here is to optimistically parse await
as Identifier in non-async contexts, the way it's already being done. But then if the parse turns out to be a dead end, we should "remember" this choice and emit a descriptive error on the await
- not at the dead end.
I came up with the following logic:
- When an Identifier with
name === "await"
is parsed, if we didn't enterparseAwait
because we're in a non-async context, save the identifier's position instate.potentialIllegalAwaitAt
and keep parsing normally. - When parsing a node fails via
unexpected()
, and ifstate.potentialIllegalAwaitAt >= node.start
, raise an "await can only be used in async function" error instead of the default "unexpected token".
I'm not 100% sure how to intercept the unexpected()
occurrences elegantly while still having access to the current node - which is needed for the check in (2) above. My first approach involved patching a lot of node types and looked a bit like this:
pp.checkIllegalAwaitAndRethrow = function (err, node) {
if (this.state.potentialIllegalAwaitAt >= node.start) {
this.raise(this.state.potentialIllegalAwaitAt, "await can only be used in async functions");
} else {
throw err;
}
};
pp.semicolonCheckIllegalAwait = function (node) {
try {
this.semicolon();
} catch (err) {
this.checkIllegalAwaitAndRethrow(err, node);
}
};
// Then for *many, many* node types do something like this:
pp.parseReturnStatement = function (node) {
// ... snip ...
if (this.isLineTerminator()) {
node.argument = null;
} else {
node.argument = this.parseExpression();
this.semicolonCheckIllegalAwait(node); // instead of this.semicolon();
}
// ... snip ...
};
But it got out of hand very quickly.
So here's my new recipe, plus some questions.
- Manage a "current node" stack in
startNode
/finishNode
(Is this a viable approach? Does something like this already exist?) - In
unexpected()
itself, check forthis.state.potentialIllegalAwaitAt >= currentNode.start
and override the message iftrue
.
Honestly, it still seems a bit of a heavy solution that I'm not sure is justifiable here. But at least this way the problem gets treated as a "cross cutting concern", with a lot less code.
I'd appreciate any feedback/ideas! 😀
from babylon.
Just realized there actually is a PR open already (sorry, thought that only discussion had occurred), so I'll go ahead and close mine.
from babylon.
@motiz88 Actually, looked at your PR and the solution I came up with is very different. Don't want to step on your toes here, but let me know what you think of this: https://github.com/babel/babylon/compare/master...kaicataldo:betterawaiterrormessaging?expand=1
If you think it looks good, I can make a PR with this - just wanted to check in with you since you've already done work on this.
These changes would also fix part of this issue: #134, warning for when enum
and await
are used as identifiers when source type is module. The implementation of better error messages for await
is kind of tied to disallowing await
as a reserved word (for AssignmentExpressions, since that all happens in expression.parseExprAtom()
), and making a generic solution for both reserved words made more sense to me than only implementing await
.
Would love to hear what @danez and @hzoo think too!
from babylon.
I'm actually going to see if I can extract the reserved identifier checks out so it's easier to see how all this fits together. Running into some challenges with my own solution - it works great when source type is module, but might actually have to do some kind of flag in state when an await
keyword is found for when the source type is script, since identifiers are allowed to be called await
. The easiest thing is to have smarter warnings for source type module and keep the same warnings for source type script.
from babylon.
Hey @kaicataldo, apparently GitHub lost a reply I sent via e-mail so I'll try to address everything again.
For one thing, don't worry about my toes 😄 Especially since my PR is a work-in-progress and in a bit of stasis at the moment. But I think allowing await
only as the LHS of AssignmentExpression is incorrect, as you've already noticed.
My own solution can probably be simplified and made more correct with the use of lookahead()
, which I wasn't actually aware of when I started. I might not get back to it for a while though, so feel free to clone that branch and pull anything of interest from it. e.g.
some kind of flag in state when an await keyword is found
is exactly what I did.
from babylon.
Hey @kaicataldo, don't worry about my toes 😄 But doesn't your current
solution preclude other valid uses of await
as an identifier? For example
await.foo
, await[foo]
, await / foo
, await(foo)
, etc. There's a
partial list in my PR.
On Oct 23, 2016 06:08, "Kai Cataldo" [email protected] wrote:
@motiz88 https://github.com/motiz88 Actually, looked at your PR and the
solution I came up with is very different. Don't want to step on your toes
here, but let me know what you think of this: kaicataldo/babylon@8358cff
kaicataldo@8358cffMaybe I'm missing something, but it seems to get the job done. If you
think it looks good, I can make a PR with this - just wanted to check in
with you since you've already done work on this.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#113 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACJHpVY0WNZPTHrhffrxdtE-uEq9076gks5q2s-1gaJpZM4J4F4v
.
from babylon.
This issue has been moved to babel/babel#6720.
from babylon.
Related Issues (20)
- Can't use object-rest-spread in a function HOT 6
- Should we allow Flowtype `import type` statements when `sourceType:script`? HOT 2
- How to get the "root" path HOT 5
- ES2016 bind syntax does not work with functions HOT 5
- Flow interfaces & `declare class` do not adhere to ASI HOT 2
- Warning if tests fail that if you need to generate fixtures. HOT 2
- React 16 SyntaxError: Adjacent JSX elements must be wrapped in an enclosing tag HOT 7
- Set id to null for ArrowFunctionExpressions? HOT 3
- babylon super.xxx will cause performance is very slow HOT 1
- Add sourceType 'function' HOT 12
- Typescript: ExportNamedDeclaration raises SyntaxError when exportExtensions plugin enabled HOT 1
- Unable to parse a class get method with a decorator HOT 2
- static field initializer as an arrow function containing new.target is not allowed
- Unexpected AST for pipeline operator with arrow function HOT 2
- Flow comment parsing HOT 8
- typescript: Support binding pattern in signature HOT 2
- Existential return type annot in arrow function confused with `*=` operator HOT 5
- Error w/ generics in return type annots inside JSXExpressionContainers HOT 3
- [Presets: React + Typescript] Complex method with generic type fails during transpilation HOT 2
- PSA: this repo has been moved to the main babel repo
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from babylon.