Parser refactoring #11871

JLHwung · 2020-07-23T00:24:33Z

Q	A
Fixed Issues?	REPL 1, REPL 2
Patch: Bug Fix?	Yes
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

Refactors @babel/parser a bit. Focused on code readability and syntax upstream spec links.
Happens to fix a bug when adding grammar production comments.

codesandbox-ci · 2020-07-23T00:27:12Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

babel-bot · 2020-07-23T00:30:52Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/26584/

JLHwung · 2020-07-23T14:53:07Z

packages/babel-parser/test/fixtures/flow/type-annotations/20/output.json

@@ -34,7 +34,6 @@
                },
                "computed": false,
                "kind": "set",
-                "variance": null,


The variance property exists in ClassProperty, and we don't have variance for ObjectMethod. This PR removes variance of fooProp accessor for the following cases:

a={set fooProp(value:number){}}

I think the new behaviour is preferred and it aligns to what we did for non-accessors.

JLHwung · 2020-07-23T23:11:10Z

packages/babel-parser/src/parser/statement.js

@@ -1281,10 +1297,7 @@ export default class StatementParser extends ExpressionParser {
      prop.static = false;
      classBody.body.push(this.parseClassProperty(prop));
      return true;
-    } else if (containsEsc) {


This branch is unreachable because parseClassMemberFromModifier is guarded by isContextual call, which must return false when containsEsc.

JLHwung · 2020-07-23T23:15:31Z

packages/babel-parser/src/parser/expression.js

+    ) {
+      // https://tc39.es/ecma262/#prod-AsyncMethod
+      // https://tc39.es/ecma262/#prod-AsyncGeneratorMethod
+      if (prop.key.name === "async" && !this.hasPrecedingLineBreak()) {


Parsing async and set, getter have been unified in parsePropertyDefinition -- so they can share same sanity checks. Thus we don't need passing containsEsc (A tokenizing level states) to parseObjectMethod, instead isAccessor is pass.

JLHwung · 2020-07-23T23:18:10Z

packages/babel-parser/src/parser/expression.js


-    if (this.prodParam.hasYield && this.eat(tt.dot)) {
+    if (this.prodParam.hasYield && this.match(tt.dot)) {
+      const meta = this.createIdentifier(


Avoid creating an extra meta node when tt.dot is not met, instead we create meta when we have to. The same technique is also applied for parseNewOrNewTarget.

JLHwung · 2020-07-23T23:24:54Z

packages/babel-parser/src/parser/expression.js

@@ -594,163 +602,205 @@ export default class ExpressionParser extends LValParser {
    state: N.ParseSubscriptState,


parseSubscript has been broken into smaller pieces. I find it more easier to go through after refactoring: https://github.com/JLHwung/babel/blob/ee4d63ef00e5090f44ffedee718dd18fdc6e21e0/packages/babel-parser/src/parser/expression.js#L597

JLHwung · 2020-07-23T23:27:47Z

I have changed the PR label because the bug fixed here is edging cases. It is more of a refactoring which is very likely invisible to users.

nicolo-ribaudo

I left a few nits, but overall I find that this improves the readability 👍

nicolo-ribaudo · 2020-07-26T20:13:23Z

packages/babel-parser/src/parser/lval.js

-    startLoc = startLoc || this.state.startLoc;
-    startPos = startPos || this.state.start;
-    left = left || this.parseBindingAtom();
+    startLoc = startLoc ?? this.state.startLoc;


~~Maybe now we can use ??=? 😄~~

Flow 😞

nicolo-ribaudo · 2020-07-26T20:19:52Z

packages/babel-parser/src/parser/expression.js

-
-  parseMaybeUnary(refExpressionErrors: ?ExpressionErrors): N.Expression {
+  // https://tc39.es/ecma262/#prod-UnaryExpression
+  parseUnary(refExpressionErrors: ?ExpressionErrors): N.Expression {


I think that this was called parseMaybeUnary (like parseMaybeConditional, parseMaybeAssign) because it could return something which is not a unary expression, like a simple identifier.

I know that technically Identifier is a child of UnaryExpression in the spec, but it's just a side effect of how it's specified.

packages/babel-parser/src/parser/expression.js

kaicataldo

Nice refactor!

existentialism

👍

JLHwung force-pushed the parser-refactoring branch from 0621f12 to 5dd4f7c Compare July 23, 2020 00:27

JLHwung added pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jul 23, 2020

JLHwung force-pushed the parser-refactoring branch from 5dd4f7c to a4578f4 Compare July 23, 2020 01:42

JLHwung commented Jul 23, 2020

View reviewed changes

JLHwung marked this pull request as ready for review July 23, 2020 23:12

JLHwung commented Jul 23, 2020

View reviewed changes

JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories PR: Bug Fix 🐛 A type of pull request used for our changelog categories and removed PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jul 23, 2020

nicolo-ribaudo approved these changes Jul 26, 2020

View reviewed changes

JLHwung added 12 commits July 29, 2020 18:34

refactor: parseMaybeUnary => parseUnary

c2601f8

refactor: extract shouldExitDescending method

1556700

refactor: add parseUpdate

6e154c4

refactor: avoid comparing with hardcoded token value

f352747

refactor: add ParseNewOrNewTarget

6285541

refactor: add parseCoverCallAndAsyncArrowHead

a2eb7be

add parseBind

569860b

refactor: polish parseTaggedTemplateExpression interface

82c2de2

refactor: add parseMember method

4d03961

refactor: add parseSuper method

7044d1e

refactor: add parseAsyncArrowUnaryFunction method

0d3843b

fix: disallow line break before async binding arrow

5d5d539

JLHwung added 16 commits July 29, 2020 18:34

refactor: simplify tt.name logic

acad7a0

refactor: add parseDo method

27ee426

refactor: misc

531ac56

refactor: rename parseObjectMember by parsePropertyDefinition

66466c5

refactor: unify set/get/async keyword parsing in ObjectMethod

7828557

refactor: misc

1afec6e

refactor: add parseArrayLike method

b537755

refactor: move fsharp epilogure and prologue inside parseObjectLike

3e2aa7b

fixup

01f8fa9

refactor: rename parseFunctionExpression to parseFunctionOrFunctionSent

0b4deb1

refactor: remove redundant logic

ca1dc02

refactor: rename parseClassPropertyName by parseClassElementName

addcb40

refactor: avoid unecessary lookahead when parsing tt._export

f94d383

fix: export-default-from should support escaped async as export binding

4af6c9a

address review comments

02fe9a7

parseUnary -> parseMaybeUnary

1d693d8

JLHwung force-pushed the parser-refactoring branch from 206786e to 1d693d8 Compare July 29, 2020 22:35

kaicataldo approved these changes Jul 31, 2020

View reviewed changes

existentialism approved these changes Jul 31, 2020

View reviewed changes

JLHwung merged commit a4ebe29 into babel:main Aug 1, 2020

JLHwung deleted the parser-refactoring branch August 1, 2020 00:36

JLHwung mentioned this pull request Aug 5, 2020

fix: do not eat get/set after async is parsed #11916

Merged

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 31, 2020

github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser refactoring #11871

Parser refactoring #11871

JLHwung commented Jul 23, 2020 •

edited by gitpod-io bot

codesandbox-ci bot commented Jul 23, 2020 •

edited

babel-bot commented Jul 23, 2020 •

edited

JLHwung Jul 23, 2020

JLHwung Jul 23, 2020

JLHwung Jul 23, 2020

JLHwung Jul 23, 2020

JLHwung Jul 23, 2020

JLHwung commented Jul 23, 2020

nicolo-ribaudo left a comment

nicolo-ribaudo Jul 26, 2020

nicolo-ribaudo Jul 26, 2020

kaicataldo left a comment

existentialism left a comment

		@@ -594,163 +602,205 @@ export default class ExpressionParser extends LValParser {
		state: N.ParseSubscriptState,

Parser refactoring #11871

Parser refactoring #11871

Conversation

JLHwung commented Jul 23, 2020 • edited by gitpod-io bot

codesandbox-ci bot commented Jul 23, 2020 • edited

babel-bot commented Jul 23, 2020 • edited

JLHwung Jul 23, 2020

Choose a reason for hiding this comment

JLHwung Jul 23, 2020

Choose a reason for hiding this comment

JLHwung Jul 23, 2020

Choose a reason for hiding this comment

JLHwung Jul 23, 2020

Choose a reason for hiding this comment

JLHwung Jul 23, 2020

Choose a reason for hiding this comment

JLHwung commented Jul 23, 2020

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo Jul 26, 2020

Choose a reason for hiding this comment

nicolo-ribaudo Jul 26, 2020

Choose a reason for hiding this comment

kaicataldo left a comment

Choose a reason for hiding this comment

existentialism left a comment

Choose a reason for hiding this comment

JLHwung commented Jul 23, 2020 •

edited by gitpod-io bot

codesandbox-ci bot commented Jul 23, 2020 •

edited

babel-bot commented Jul 23, 2020 •

edited