+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_parser): Ignore bindings in desctructuring initializers #3525

Merged
merged 2 commits into from
Oct 31, 2022
Merged

fix(rome_js_parser): Ignore bindings in desctructuring initializers #3525

merged 2 commits into from
Oct 31, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Oct 28, 2022 •
edited
Loading

Rome's parser implements a very limited check for duplicated bindings by testing if there are no duplicate bindings in the same variable declaration.

This PR fixes an issue where the parser flagged a binding with the same name inside a destructuring initializer as a duplicated binding which is incorrect.

Fixes #3521 #2960

@MichaReiser
fix(rome_js_parser): Ignore bindings in desctructuring initializers
efb609c 
Rome's parser implements a very limited check for duplicated bindings by testing if there are no duplicate bindings in the same variable declaration.

This PR fixes an issue where the parser flagged a binding with the same name inside a destructuring initializer as a duplicated binding which is incorrect.

Fixes #3521
@MichaReiser MichaReiser added the A-Parser Area: parser label Oct 28, 2022
@MichaReiser MichaReiser added this to the 10.0.0 milestone Oct 28, 2022
@MichaReiser MichaReiser requested a review from xunilrj as a code owner October 28, 2022 09:23
@netlify
Copy link

netlify bot commented Oct 28, 2022 •
edited
Loading

✅ Deploy Preview for rometools canceled.

Name Link
🔨 Latest commit 601a0a8
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/635b9faea6e52b00091280ae

@MichaReiser MichaReiser temporarily deployed to netlify-playground October 28, 2022 09:23 Inactive
@MichaReiser MichaReiser temporarily deployed to netlify-playground October 28, 2022 09:24 Inactive
@MichaReiser
Copy link
Contributor Author

!bench_parser

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44939 44939 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1621 1621 0
Failed 4325 4325 0
Panics 0 0 0
Coverage 27.26% 27.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12395 12395 0
Failed 3862 3862 0
Panics 0 0 0
Coverage 76.24% 76.24% 0.00%

@github-actions
Copy link

github-actions bot commented Oct 28, 2022 •
edited
Loading

Playground for commit ccd945f

@MichaReiser MichaReiser removed this from the 10.0.0 milestone Oct 28, 2022
@github-actions
Copy link

Parser Benchmark Results

group                                 main                                   pr
-----                                 ----                                   --
parser/checker.ts                     1.01    132.3±1.66ms    19.7 MB/sec    1.00    131.4±1.63ms    19.8 MB/sec
parser/compiler.js                    1.01     75.8±0.96ms    13.8 MB/sec    1.00     75.2±0.80ms    13.9 MB/sec
parser/d3.min.js                      1.00     45.3±0.60ms     5.8 MB/sec    1.00     45.2±0.64ms     5.8 MB/sec
parser/dojo.js                        1.00      3.6±0.01ms    19.1 MB/sec    1.00      3.6±0.01ms    19.1 MB/sec
parser/ios.d.ts                       1.01    112.7±1.42ms    16.6 MB/sec    1.00    112.0±1.18ms    16.7 MB/sec
parser/jquery.min.js                  1.00     11.3±0.07ms     7.3 MB/sec    1.01     11.4±0.12ms     7.2 MB/sec
parser/math.js                        1.00     89.6±0.64ms     7.2 MB/sec    1.00     90.0±0.70ms     7.2 MB/sec
parser/parser.ts                      1.00      2.6±0.01ms    18.6 MB/sec    1.00      2.6±0.00ms    18.7 MB/sec
parser/pixi.min.js                    1.00     56.6±0.98ms     7.7 MB/sec    1.00     56.8±0.72ms     7.7 MB/sec
parser/react-dom.production.min.js    1.00     15.5±0.12ms     7.4 MB/sec    1.01     15.6±0.23ms     7.4 MB/sec
parser/react.production.min.js        1.00    777.6±1.66µs     7.9 MB/sec    1.01    783.7±1.32µs     7.8 MB/sec
parser/router.ts                      1.00      2.1±0.01ms    28.5 MB/sec    1.00      2.2±0.00ms    28.4 MB/sec
parser/tex-chtml-full.js              1.00    121.7±1.14ms     7.5 MB/sec    1.00    121.5±1.21ms     7.5 MB/sec
parser/three.min.js                   1.00     62.9±0.65ms     9.3 MB/sec    1.00     62.8±0.66ms     9.4 MB/sec
parser/typescript.js                  1.02    524.4±5.21ms    18.1 MB/sec    1.00    515.5±4.93ms    18.4 MB/sec
parser/vue.global.prod.js             1.00     19.3±0.30ms     6.3 MB/sec    1.01     19.5±0.28ms     6.2 MB/sec

@leops
Copy link
Contributor

leops commented Oct 28, 2022

Ideally this check should eventually get moved to the syntax pass of the analyzer, since it should be easier to track the scope of declarations in the semantic model than in the parser state

@MichaReiser
Copy link
Contributor Author

Ideally this check should eventually get moved to the syntax pass of the analyzer, since it should be easier to track the scope of declarations in the semantic model than in the parser state

Absolutely. But that's a bit more involved 😃

@MichaReiser MichaReiser merged commit b640821 into main Oct 31, 2022
@MichaReiser MichaReiser deleted the fix/destructuring-initializer-duplicate-binding branch October 31, 2022 08:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leops leops leops approved these changes

@xunilrj xunilrj Awaiting requested review from xunilrj

Assignees
No one assigned
Labels
A-Parser Area: parser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐛 initializer parameter considered as "duplicated binding" in deconstruction
2 participants
@MichaReiser @leops
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载