feat(rome_js_formatter): Preprocess AST before formatting

[MichaReiser]

Summary

This PR adds a pre-processing step to the formatter that can be used to bring the tree into a shape that better suits the formatter and adds support for:

• Removing unnecessary parentheses of logical expressions with a nested logical expression in the right hand side
• Preserving parentheses if a node has a leading type cast comment

Rewriter

The JavaScript uses the pre-processing to:

• Remove any parenthesized nodes except:
  • If the expression node has a syntax error (missing required child or has a skipped token trivia)
  • If the inner expression is an unknown node
  • If the expression has a closure type-cast comment
• Re-balances logical expressions: a && (b && c) -> (a && b) && c so that it works nicely with the binary like expression flattening

This logic is implemented inside of the new JsFormatSyntaxRewriter.

Source Map

The second significant part of this PR is the need for a source map to map printer markers to the original source positions and resolve the source text for a transformed node

For example:

// Source			// Transformed
(a + b)				a + b

Resolving the source text of a + b should return (a + b) to ensure that the verbatim output (used when the node can't be formatted or if formatting is suppressed) doesn't drop the parentheses.

Check out the documentation of source map for a more in detail explanation.

This looks complicated

Yes, this PR introduces some complexity by adding a source map and transformation step to the formatter. In my view, the simplification that the transformation brings by no longer having to use resolve and resolve_parent inside specific formatter outweighs the added complexity that is local to the rome_formatter only. I see it as a trade of added complexity in the infrastructure in favor of simplifying individual formatters that tend to be complicated anyway.

This PR is huge

Yes, it is but most changes are

• Removing the ExpressionNode and AssignmentNode implementations
• Removing resolve and resolve_parent calls.
• Updated snapshot

I recommend you focusing on reviewing

• The source map implementation
• The syntax rewrite

Performance

group                                    main                                    parens
formatter/checker.ts                     1.00   226.6±16.42ms    11.5 MB/sec     1.09    247.1±2.08ms    10.5 MB/sec
formatter/compiler.js                    1.00   144.6±10.28ms     7.2 MB/sec     1.11   160.9±13.78ms     6.5 MB/sec
formatter/d3.min.js                      1.00    106.1±1.00ms     2.5 MB/sec     1.26   134.0±12.17ms  2003.1 KB/sec
formatter/dojo.js                        1.00      7.7±0.58ms     8.9 MB/sec     1.08      8.3±0.62ms     8.2 MB/sec
formatter/ios.d.ts                       1.00   167.6±15.02ms    11.1 MB/sec     1.03   173.1±18.89ms    10.8 MB/sec
formatter/jquery.min.js                  1.00     32.5±2.36ms     2.5 MB/sec     1.02     33.2±0.61ms     2.5 MB/sec
formatter/math.js                        1.00   243.6±15.86ms     2.7 MB/sec     1.04   252.7±12.13ms     2.6 MB/sec
formatter/parser.ts                      1.00      5.3±0.43ms     9.1 MB/sec     1.06      5.6±0.46ms     8.7 MB/sec
formatter/pixi.min.js                    1.00    121.9±1.22ms     3.6 MB/sec     1.13    137.6±3.15ms     3.2 MB/sec
formatter/react-dom.production.min.js    1.00     37.3±2.83ms     3.1 MB/sec     1.18     44.1±3.70ms     2.6 MB/sec
formatter/react.production.min.js        1.00  1795.7±12.33µs     3.4 MB/sec     1.14      2.0±0.00ms     3.0 MB/sec
formatter/router.ts                      1.01      4.2±0.35ms    14.7 MB/sec     1.00      4.1±0.05ms    14.8 MB/sec
formatter/tex-chtml-full.js              1.00   325.4±19.21ms     2.8 MB/sec     1.03   335.3±23.16ms     2.7 MB/sec
formatter/three.min.js                   1.00    137.6±1.49ms     4.3 MB/sec     1.12    154.0±1.51ms     3.8 MB/sec
formatter/typescript.js                  1.00   975.5±66.09ms     9.7 MB/sec     1.06  1038.4±75.09ms     9.1 MB/sec
formatter/vue.global.prod.js             1.00     48.6±3.57ms     2.5 MB/sec     1.10     53.6±3.78ms     2.2 MB/sec

Formatting rome-classic:

• main: Formatted 3959 files in 216ms
• this pr: Formatted 3959 files in 236ms

The main bottleneck for formatting remains IO

Test Plan

Average compatibility: 82.68 -> 83.70
Compatible lines: 79.75 -> 80.79

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Pages

Latest commit:	2ef2fca
Status:	✅  Deploy successful!
Preview URL:	https://0b056f4e.tools-8rn.pages.dev
Branch Preview URL:	https://feat-parentheses-transform.tools-8rn.pages.dev

View logs

[github-actions]

Playground for commit b086f65

[github-actions]

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	12397	12397	0
Failed	3860	3860	0
Panics	0	0	0
Coverage	76.26%	76.26%	0.00%

[MichaReiser]

!bench_formatter

[MichaReiser]

@ematipico as discussed offline. I added additional documentation and added more tests to the SyntaxRewriter and SourceMap

[ematipico]

Left some questions. I still need to review some stuff after having answer from my questions

[MichaReiser]

!bench_formatter

[github-actions]

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.00    362.4±3.53ms     7.2 MB/sec    1.33    482.9±3.00ms     5.4 MB/sec
formatter/compiler.js                    1.00    249.2±1.47ms     4.2 MB/sec    1.11    276.5±1.70ms     3.8 MB/sec
formatter/d3.min.js                      1.00    202.5±1.45ms  1325.6 KB/sec    1.17    236.4±2.07ms  1135.4 KB/sec
formatter/dojo.js                        1.00     13.0±0.02ms     5.3 MB/sec    1.10     14.3±0.02ms     4.8 MB/sec
formatter/ios.d.ts                       1.00    233.7±0.88ms     8.0 MB/sec    1.24    290.8±1.19ms     6.4 MB/sec
formatter/jquery.min.js                  1.00     55.1±0.91ms  1534.8 KB/sec    1.09     60.4±1.00ms  1401.8 KB/sec
formatter/math.js                        1.00    381.1±4.65ms  1740.0 KB/sec    1.25    477.6±4.51ms  1388.4 KB/sec
formatter/parser.ts                      1.00      8.7±0.01ms     5.6 MB/sec    1.08      9.4±0.02ms     5.2 MB/sec
formatter/pixi.min.js                    1.00    236.2±1.49ms  1902.5 KB/sec    1.12    263.8±2.12ms  1703.6 KB/sec
formatter/react-dom.production.min.js    1.00     66.5±1.50ms  1772.1 KB/sec    1.13     74.9±1.01ms  1573.4 KB/sec
formatter/react.production.min.js        1.00      3.3±0.00ms  1932.7 KB/sec    1.10      3.6±0.00ms  1758.9 KB/sec
formatter/router.ts                      1.00      6.8±0.01ms     9.1 MB/sec    1.08      7.3±0.03ms     8.4 MB/sec
formatter/tex-chtml-full.js              1.00    544.4±3.65ms  1714.1 KB/sec    1.12    611.1±4.76ms  1527.0 KB/sec
formatter/three.min.js                   1.00    265.6±1.94ms     2.2 MB/sec    1.12    297.2±3.00ms  2023.0 KB/sec
formatter/typescript.js                  1.00   1485.7±4.90ms     6.4 MB/sec    1.24   1842.4±6.40ms     5.2 MB/sec
formatter/vue.global.prod.js             1.00     89.2±1.18ms  1382.9 KB/sec    1.10     98.4±1.14ms  1254.3 KB/sec

[ematipico]

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.00    362.4±3.53ms     7.2 MB/sec    1.33    482.9±3.00ms     5.4 MB/sec
formatter/compiler.js                    1.00    249.2±1.47ms     4.2 MB/sec    1.11    276.5±1.70ms     3.8 MB/sec
formatter/d3.min.js                      1.00    202.5±1.45ms  1325.6 KB/sec    1.17    236.4±2.07ms  1135.4 KB/sec
formatter/dojo.js                        1.00     13.0±0.02ms     5.3 MB/sec    1.10     14.3±0.02ms     4.8 MB/sec
formatter/ios.d.ts                       1.00    233.7±0.88ms     8.0 MB/sec    1.24    290.8±1.19ms     6.4 MB/sec
formatter/jquery.min.js                  1.00     55.1±0.91ms  1534.8 KB/sec    1.09     60.4±1.00ms  1401.8 KB/sec
formatter/math.js                        1.00    381.1±4.65ms  1740.0 KB/sec    1.25    477.6±4.51ms  1388.4 KB/sec
formatter/parser.ts                      1.00      8.7±0.01ms     5.6 MB/sec    1.08      9.4±0.02ms     5.2 MB/sec
formatter/pixi.min.js                    1.00    236.2±1.49ms  1902.5 KB/sec    1.12    263.8±2.12ms  1703.6 KB/sec
formatter/react-dom.production.min.js    1.00     66.5±1.50ms  1772.1 KB/sec    1.13     74.9±1.01ms  1573.4 KB/sec
formatter/react.production.min.js        1.00      3.3±0.00ms  1932.7 KB/sec    1.10      3.6±0.00ms  1758.9 KB/sec
formatter/router.ts                      1.00      6.8±0.01ms     9.1 MB/sec    1.08      7.3±0.03ms     8.4 MB/sec
formatter/tex-chtml-full.js              1.00    544.4±3.65ms  1714.1 KB/sec    1.12    611.1±4.76ms  1527.0 KB/sec
formatter/three.min.js                   1.00    265.6±1.94ms     2.2 MB/sec    1.12    297.2±3.00ms  2023.0 KB/sec
formatter/typescript.js                  1.00   1485.7±4.90ms     6.4 MB/sec    1.24   1842.4±6.40ms     5.2 MB/sec
formatter/vue.global.prod.js             1.00     89.2±1.18ms  1382.9 KB/sec    1.10     98.4±1.14ms  1254.3 KB/sec

Is it just me, or do the numbers got slightly higher compared to the previous bench run?

[MichaReiser]

I compared the numbers again and it's mainly checker and typescript that now differs significantly. I manually compared the commits before adding the Fx hash map for those two tests to see if there's a serious regression

group                      fx                                     map
-----                      --                                     ---
formatter/checker.ts       1.00   276.1±22.30ms     9.4 MB/sec    1.03   283.1±42.19ms     9.2 MB/sec
formatter/typescript.js    1.02  1079.0±74.32ms     8.8 MB/sec    1.00  1060.6±102.66ms     9.0 MB/sec

The numbers are in the normal range which is why I think it's fine to go ahead with this PR to unlock further work.

As discussed offline with @ematipico There are a few things that we want to discuss offline and I'll create a follow up PR addressing the findings from that meeting.