Skip to content

[parser] Add support for slice notation#10596

Closed
JLHwung wants to merge 23 commits intobabel:masterfrom
JLHwung:syntax-slice-notation
Closed

[parser] Add support for slice notation#10596
JLHwung wants to merge 23 commits intobabel:masterfrom
JLHwung:syntax-slice-notation

Conversation

@JLHwung
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung commented Oct 23, 2019

Q                       A
Fixed Issues? babel/proposals#41
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR adds parser support for Slice Notation A new node type SliceExpression is introduced to separate slice notation from MemberExpression.

Note that the proposal does not support free-form range, e.g. [1:4:2] and slice arguments other than identifier or numeric literals, e.g.a[1:(a.length - 2)]. Support for negative number is also added for completeness because it is modelled as an UnaryExpression instead of a NumericLiteral.

# Conflicts:
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-bigint-literal/input.js
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-bigint-literal/options.json
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-binary-expression/input.js
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-binary-expression/options.json
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-unary-expression-plus/input.js
#	packages/babel-parser/test/fixtures/experimental/slice-notation/invalid-arg-unary-expression-plus/options.json
@JLHwung JLHwung added area: experimental PR: New Feature 🚀 A type of pull request used for our changelog categories labels Oct 23, 2019
@JLHwung JLHwung mentioned this pull request Oct 23, 2019
2 tasks
validate: assertNodeType("Expression"),
},
lower: {
validate: assertNodeType(
Copy link
Copy Markdown
Contributor Author

@JLHwung JLHwung Oct 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider to use a custom validator function which aligns with parser.checkSliceArgument.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize that @babel/parser and @babel/types does not depends on each other, which means it is currently not feasible to share routines between them. I would leave it as-is.

(expr.type === "UnaryExpression" &&
expr.operator === "-" &&
expr.extra?.parenthesized !== true &&
this.isSliceSimpleArgument(expr.argument))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is -foo valid?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think -foo is allowed, but -Infinity should be allowed, although they are syntactically equivalent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that -Infinity is valid, since it is currently restricted to Identifiers and DecimalDigits (https://github.com/tc39/proposal-slice-notation#can-the-upper-bound-lower-bound-or-the-step-argument-be-an-arbitrary-expression).
Althought, the readme itself contracdits it by using negative numbers. Also, there is tc39/proposal-slice-notation#16 which didn't receive a response.

I am ok either with the current status of this PR, or with making it more restrictive and only allow - NumericLiteral. Since this is a stage 1 proposal, it's expected that we will fix it when the semantics are clearer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If - NumericLiteral is allowed, it is strange that - Infinity is not allowed given that Infinity is allowed.

I tend to making it restrictive without the loss of expressive power. Since - Infinity could not be expressed further simple, I suggest we support it unless the proposal explicitly bans -Infinity.

@nicolo-ribaudo nicolo-ribaudo added this to the v7.7.0 milestone Oct 23, 2019
@nicolo-ribaudo nicolo-ribaudo added PR: Needs Review A pull request awaiting more approvals and removed PR: Needs Review A pull request awaiting more approvals labels Oct 23, 2019
@nicolo-ribaudo nicolo-ribaudo removed this from the v7.7.0 milestone Oct 27, 2019
@existentialism existentialism self-requested a review November 12, 2019 22:58
@JLHwung
Copy link
Copy Markdown
Contributor Author

JLHwung commented Jul 3, 2020

Recently Slice annotation has drafted minimal spec text: tc39/proposal-slice-notation#35

Abandoning this PR due to syntax changes.

@JLHwung JLHwung closed this Jul 3, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: experimental outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants