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

feat(rome_js_formatter): Function parameter & return type grouping #2990

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Aug 2, 2022

Summary

This PR refines the formatting of function parameters and return type annotations by adopting Prettier's behaviour to first break the return type annotation in some situations before breaking the parentheses (see shouldGroupCallArguments).

Implementing this change required to extract the shared method formatting logic into a FormatMethodMember helper struct.

Test Plan

Verified that e.g. object types now break before the parentheses.

File Based Average Prettier Similarity: 78.03% -> 78.10%
Line Based Average Prettier Similarity: 73.10% -> 73.19%

@MichaReiser MichaReiser temporarily deployed to aws August 2, 2022 13:24 Inactive
@MichaReiser MichaReiser changed the base branch from main to refactor/extract-any-function-formatting August 2, 2022 13:24
@MichaReiser MichaReiser force-pushed the refactor/extract-any-function-formatting branch from a47eadd to 41be225 Compare August 2, 2022 13:27
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 2, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 08e3d40
Status: ✅  Deploy successful!
Preview URL: https://4de140af.tools-8rn.pages.dev
Branch Preview URL: https://feature-function-formatting.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

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%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 395 395 0
Failed 5551 5551 0
Panics 0 0 0
Coverage 6.64% 6.64% 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 12393 12393 0
Failed 3864 3864 0
Panics 0 0 0
Coverage 76.23% 76.23% 0.00%

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

@MichaReiser MichaReiser changed the title refactor(rome_js_formatter): Extract JsAnyFunction formatter into `… feat(rome_js_formatter): Format function parameters Aug 2, 2022
@MichaReiser MichaReiser marked this pull request as ready for review August 2, 2022 15:36
@MichaReiser MichaReiser added the A-Formatter Area: formatter label Aug 2, 2022
Base automatically changed from refactor/extract-any-function-formatting to main August 3, 2022 06:55
@MichaReiser MichaReiser requested a review from a team August 3, 2022 06:55
@MichaReiser MichaReiser changed the title feat(rome_js_formatter): Format function parameters feat(rome_js_formatter): Function parameter & return type grouping Aug 3, 2022
@MichaReiser MichaReiser temporarily deployed to aws August 3, 2022 06:59 Inactive
Implement the same heuristic as Prettier for grouping parameters with the return type annotation.

This ensures that a return type will break FIRST before the parameters and the parameters only break if they don't fit on the same line together with the expanded return type annotation.

This PR further unifies the formatting of methods and functions to achieve better code reuse.
@MichaReiser MichaReiser temporarily deployed to aws August 3, 2022 07:26 Inactive
@MichaReiser
Copy link
Contributor Author

!bench_formatter

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.00    367.4±1.61ms     7.1 MB/sec    1.00    367.5±1.56ms     7.1 MB/sec
formatter/compiler.js                    1.01    229.0±1.44ms     4.6 MB/sec    1.00    226.2±0.99ms     4.6 MB/sec
formatter/d3.min.js                      1.01    182.3±1.90ms  1472.2 KB/sec    1.00    180.5±1.89ms  1487.0 KB/sec
formatter/dojo.js                        1.00     12.3±0.06ms     5.6 MB/sec    1.00     12.3±0.02ms     5.6 MB/sec
formatter/ios.d.ts                       1.01    240.7±1.41ms     7.8 MB/sec    1.00    239.4±0.94ms     7.8 MB/sec
formatter/jquery.min.js                  1.01     47.0±0.19ms  1800.0 KB/sec    1.00     46.7±0.13ms  1810.7 KB/sec
formatter/math.js                        1.01    381.1±1.35ms  1740.0 KB/sec    1.00    377.5±1.32ms  1756.7 KB/sec
formatter/parser.ts                      1.00      8.2±0.01ms     5.9 MB/sec    1.01      8.3±0.01ms     5.8 MB/sec
formatter/pixi.min.js                    1.04    209.1±1.61ms     2.1 MB/sec    1.00    201.9±1.37ms     2.2 MB/sec
formatter/react-dom.production.min.js    1.00     57.6±0.29ms  2044.8 KB/sec    1.00     57.5±0.34ms     2.0 MB/sec
formatter/react.production.min.js        1.00      3.0±0.01ms     2.1 MB/sec    1.00      3.0±0.01ms     2.0 MB/sec
formatter/router.ts                      1.00      6.3±0.00ms     9.7 MB/sec    1.01      6.4±0.02ms     9.6 MB/sec
formatter/tex-chtml-full.js              1.04    508.3±6.74ms  1835.7 KB/sec    1.00    490.7±1.19ms  1901.8 KB/sec
formatter/three.min.js                   1.01    240.0±1.90ms     2.4 MB/sec    1.00    238.6±1.80ms     2.5 MB/sec
formatter/typescript.js                  1.01   1542.9±4.64ms     6.2 MB/sec    1.00   1533.5±3.21ms     6.2 MB/sec
formatter/vue.global.prod.js             1.00     76.3±0.68ms  1616.2 KB/sec    1.00     76.4±1.36ms  1615.9 KB/sec

@ematipico ematipico added this to the 0.9.0 milestone Aug 3, 2022
@MichaReiser MichaReiser temporarily deployed to aws August 3, 2022 11:35 Inactive
@MichaReiser MichaReiser merged commit 49ad49a into main Aug 3, 2022
@MichaReiser MichaReiser deleted the feature/function-formatting branch August 3, 2022 11:47
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants