should allow duplicate __proto__ keys in patterns

[babel-bot]

Issue originally reported by @babel-bot in babel/babylon#306

Original issue submitted by @gsathya in #5145

Input Code

result = { __proto__: x, __proto__: y } = value;

Babel Configuration (.babelrc, package.json, cli command)

presets - latest

Expected Behavior

No error

Current Behavior

Error: Redefinition of __proto__ property

Possible Solution

Context

Your Environment

software	version
Babel
node
npm
Operating System

[babel-bot]

Comment originally made by @xtuc

@gsathya The spec says that:

It is a Syntax Error if PropertyNameList of PropertyDefinitionList contains any duplicate entries for __proto__ [...]

(source)

[babel-bot]

Comment originally made by @gsathya

When ObjectLiteral appears in a context where ObjectAssignmentPattern is required the Early Error rule is not applied. In addition, it is not applied when initially parsing a CoverParenthesizedExpressionAndArrowParameterList.

[nicolo-ribaudo]

If anyone wants to work on this issue, here are some info 🙂

This parser error is thrown by the checkPropClash method, which has different problems:

• The description in the comment above the method doesn't make any sense 🤔 It should be reworded, and maybe the method can be renamed to checkDuplicatedProto.
• It is called by parseObj, if the isPattern parameters is false. The problem is that isPattern is true only when we are sure in advance that we are parsing a pattern (e.g. var { a } = {}). In cases like ({ a } = {}) we don't know it until after we have parsed the object and we have found =.
  Sometimes it is even harder: consider ({ __proto__: a, __proto__: b }, c, d, e) => {}.

As a good first step, it's enought to allow it in the "easy" case, when it's followed by =. In order to do so, we can wait calling checkDuplicatedProto after having parsed the whole object (in the parseObj method), and only if !isPattern && !this.matches(tt.eq) (this.matches is used to check the upcoming token).

If you have a clever idea (I don't have it yet 😅) to also fix the "hard" case, please do it!

Note that the method is redefined in packages/babel-parser/src/plugins/estree.js. The files inside the plugins folder extend the base parser class to override some methods which needs to be modified when the plugin is enabled. We also need to update that file.

---

If you don't know how to clone Babel, follow these steps: (you need to have make and yarn available on your machine).

1. Write a comment there to let other possible contributors know that you are working on this bug.
2. Fork the repo
3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
4. Run yarn && make bootstrap
5. Wait ⏳
6. Run make watch (or make build whenever you change a file)
7. Add a test in packages/babel-parser/test/fixtures/es2015/destructuring (only input.js; output.js will be automatically generated)
8. Update the code!
9. yarn jest parser to run the tests
   • If some test outputs don't match but the new results are correct, you can delete the bad output.js files (or update options.json) and run the tests again
10. If it is working, run make test to run all the tests
11. Run git push and open a PR!

[awave1]

@nicolo-ribaudo maybe you can help. I tried running tests, using yarn jest parser (or TEST_ONLY=babel-parses make test), but every time I run either of those commands, it doesn't do anything. Any clues what might be wrong?

[nicolo-ribaudo]

Does ./node_modules/.bin/jest print anything?

[JLHwung]

@awave1 If you have questions on building babel, I suggest to join our slack room for help. It is more convenient to communicate and we could keep this thread more focused.

[nicolo-ribaudo]

Note that #10532 only fixes the "simple case" of #6705 (comment)

[biddwan09]

Hi @nicolo-ribaudo can you please assign this issue to me . I would like to contribute on this one

[mk-pmb]

How's progress going? Should I take a look?

[JLHwung]

@mk-pmb Sure, you can also pair with @biddwan09 to see if they has come up with idea towards a solution.

[mk-pmb]

From a cursory look, for the hard case it seems we need to establish a mechanism to bubble up annotations, marking parts of the AST as tainted and carrying a todo list of what needs to be checked as soon as we know enough context. I'll need to read more about how our parser works, this will take a while.