Skip to content

Allow duplicate __proto__ keys in patterns, simple case (#6705)#10532

Merged
nicolo-ribaudo merged 4 commits intobabel:masterfrom
alejo90:duplicate-proto-pattern
Oct 14, 2019
Merged

Allow duplicate __proto__ keys in patterns, simple case (#6705)#10532
nicolo-ribaudo merged 4 commits intobabel:masterfrom
alejo90:duplicate-proto-pattern

Conversation

@alejo90
Copy link
Copy Markdown
Contributor

@alejo90 alejo90 commented Oct 9, 2019

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

This is a fix for an error being thrown when there is a duplicate __proto__ in a destructuring pattern. It only fixes the simple case when the pattern is immediately followed by a =. I renamed the checkPropClash function to checkDuplicatedProto, as advised in the issue. Instead of relying exclusively on a boolean flag to determine whether __proto__ had appeared before or not, I store the amount of times it has appeared and the position of the first duplicated occurrence. In parseObj, I check for the existence of the first duplicated occurrence property I stored before and raise an error if it's there and the next token is not a =.
Please let me know if there are any concerns or improvements I can make. I'm thinking about the more complicated case as well but I'll need a little more time with the code before I can tackle it.
Thanks @nicolo-ribaudo for the hints on how to get started.

@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Oct 9, 2019

We also need to update the test262 whitelist. Could you run make bootstrap-test262 and make test-test262-update-whitelist?

@nicolo-ribaudo nicolo-ribaudo added pkg: parser PR: Spec Compliance 👓 A type of pull request used for our changelog categories labels Oct 9, 2019
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

In this PR we have fixed the simple case of #6705, I suggest we submit a new issue to cover the following cases:

// argument destructuring
({ __proto__: x, __proto__: y }) => {}
// forX destructuring
for ({ __proto__: x, __proto__: y } of []);
// nested destructuring
({ a: { __proto__: x, __proto__: y } } = { a: {} }) 

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@JLHwung I have repoened the issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Spec Compliance 👓 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

should allow duplicate __proto__ keys in patterns

4 participants