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

fix(rome_js_parser): Assignment in decorator #2386

Merged
merged 1 commit into from
Apr 11, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Apr 11, 2022

Assignments are hard...

This PR fixes an issue where an assignment is used inside a decorator, e.g. @test(--a)

The parser used to panic when rewriting the assignment because any skipped tokens (the case for all tokens inside a decorator) are in the trivia collection and the parser events to ensure all parser methods work correctly.

However, this had the effect that skip_trivia in rewrite_parser moved the offset passed all skipped tokens, passed any not yet processed token (that ultimately get skipped).

This PR makes the rewrite parser aware of skipped tokens and only consumes them when the corresponding token gets bumped.

This prevents the parser from panicking for the following two snipped founds when fuzzing the parser #1945

(-- @ (--(-((QQQQQ $
= 8?0 @ (--(-( A S=

Assignments are hard...

This PR fixes an issue where an assignment is used inside a decorator, e.g. `@test(--a)`

The parser used to panic when rewriting the assignment because any skipped tokens (the case for all tokens inside a decorator) are in the trivia collection and the parser events to ensure all parser methods work correctly.

However, this had the effect that `skip_trivia` in `rewrite_parser` moved the offset passed all skipped tokens, passed any not yet processed token (that ultimately get skipped).

This PR makes the rewrite parser aware of skipped tokens and only consumes them when the corresponding token gets bumped.
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45250 45250 0
Passed 44310 44310 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.92% 97.92% 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%

ts/babel

Test result main count This PR count Difference
Total 584 584 0
Passed 508 508 0
Failed 76 76 0
Panics 0 0 0
Coverage 86.99% 86.99% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 15983 15983 0
Passed 12167 12167 0
Failed 3816 3816 0
Panics 0 0 0
Coverage 76.12% 76.12% 0.00%

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 11, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ef5915d
Status: ✅  Deploy successful!
Preview URL: https://7019e41e.tools-8rn.pages.dev

View logs

@MichaReiser MichaReiser merged commit d4e5259 into main Apr 11, 2022
@MichaReiser MichaReiser deleted the fix/assignment-in-decorators branch April 11, 2022 14:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants