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

refactor: Stabilize formatter #2597

Merged
merged 2 commits into from
May 23, 2022
Merged

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented May 19, 2022

Summary

Our JS Formatter is over-fitting on the test suite and it's very easy to introduce some stability issue when making any change to the formatter/printer.

I was able to narrow down the main sources of the instability issues and fixed them as best I could. However, this PR introduces two regressions that I want to address in follow up PRs.

  1. Trailing commas in call arguments: call-arguments now always print a trailing comma. Fixing this requires more extensive rules that use BestFitting. See Call expression arguments formatting #2421 for the follow-up PR.
  2. Formatting of inline comments. Inline comments aren't moved to the end anymore. This isn't ideal, I'm working on a follow up PR that re-defines the comment printing

This PR removes HardGroups. The main purpose of HardGroups was to more deterministically handle inline comments. This is no longer needed because this PR removes this inline comment printing and we need to come up with a comment printing that doesn't require every node to be wrapped inside of a HardGroup.

The "non-break" behaviour can be achieved by either just creating a sequence of tokens separated by spaces or using BestFitting where needed.

Test Plan

cargo test

@MichaReiser MichaReiser temporarily deployed to aws May 19, 2022 15:48 Inactive
@MichaReiser MichaReiser changed the base branch from main to feature/best-fitting-ir-element May 19, 2022 15:48
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 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%

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 12391 12391 0
Failed 3866 3866 0
Panics 0 0 0
Coverage 76.22% 76.22% 0.00%

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 19, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9cc2df6
Status: ✅  Deploy successful!
Preview URL: https://3bfb4da4.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented May 19, 2022

@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from bdc1b69 to 89f5d4b Compare May 19, 2022 15:55
@MichaReiser MichaReiser changed the base branch from feature/best-fitting-ir-element to main May 19, 2022 15:55
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from 89f5d4b to 8eb3a13 Compare May 19, 2022 15:58
@MichaReiser MichaReiser temporarily deployed to aws May 19, 2022 15:58 Inactive
@MichaReiser MichaReiser requested a review from leops May 19, 2022 16:00
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from 8eb3a13 to 93ca6dc Compare May 19, 2022 16:01
@MichaReiser MichaReiser temporarily deployed to aws May 19, 2022 16:01 Inactive
@MichaReiser MichaReiser marked this pull request as ready for review May 19, 2022 16:12
@MichaReiser MichaReiser marked this pull request as draft May 20, 2022 06:03
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from 93ca6dc to edf719e Compare May 20, 2022 06:56
@MichaReiser MichaReiser temporarily deployed to aws May 20, 2022 06:56 Inactive
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from edf719e to c41c26b Compare May 20, 2022 07:03
@MichaReiser MichaReiser temporarily deployed to aws May 20, 2022 07:03 Inactive
@MichaReiser MichaReiser marked this pull request as ready for review May 20, 2022 07:24
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from c41c26b to 0bbc87d Compare May 20, 2022 07:49
@MichaReiser MichaReiser temporarily deployed to aws May 20, 2022 07:49 Inactive
@cpojer cpojer changed the title refactor: Stablize formatter refactor: Stabilize formatter May 22, 2022
@MichaReiser MichaReiser temporarily deployed to aws May 23, 2022 06:13 Inactive
@MichaReiser MichaReiser force-pushed the refactor/stabilize-formatter branch from 9cc2df6 to 64810e5 Compare May 23, 2022 06:14
@MichaReiser MichaReiser temporarily deployed to aws May 23, 2022 06:15 Inactive
@MichaReiser MichaReiser merged commit 9dd6b97 into main May 23, 2022
@MichaReiser MichaReiser deleted the refactor/stabilize-formatter branch May 23, 2022 06:20
@MichaReiser MichaReiser changed the title refactor: Stabilize formatter refactor: Stabalize formatter Jun 4, 2022
@MichaReiser MichaReiser changed the title refactor: Stabalize formatter refactor: Stabilize formatter Jun 4, 2022
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