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

refactor(rome_rowan): Remove AstNode constraint from BatchMutation #3026

Merged
merged 4 commits into from
Aug 8, 2022

Conversation

MichaReiser
Copy link
Contributor

Summary

This PR removes the N: AstNode<Language=L> constraint from the BatchMutation and changes the root to a SyntaxNode<L>.

The main motivation for this change is that I want to use the BatchMutation for removing all ParenthesizedExpressions in the formatter but this isn't possible because:

  • The formatter accepts any Js node as the root and there's no JsAnyNode
  • It isn't guaranteed that the root type doesn't change in case the root node is a JsParenthesizedExpression that gets removed.

Tests

cargo test && rustcs

@MichaReiser MichaReiser requested a review from a team August 8, 2022 10:42
@MichaReiser MichaReiser temporarily deployed to aws August 8, 2022 10:43 Inactive
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

@cloudflare-workers-and-pages
Copy link

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

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: c477afd
Status: ✅  Deploy successful!
Preview URL: https://1fbecc4a.tools-8rn.pages.dev
Branch Preview URL: https://refactor-batch-mutation-synt.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Aug 8, 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 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

## Summary

This PR removes the `N: AstNode<Language=L>` constraint from the `BatchMutation` and changes the `root` to a `SyntaxNode<L>`.

The main motivation for this change is that I want to use the `BatchMutation` for removing all `ParenthesizedExpression`s in the formatter but this isn't possible because:

* The formatter accepts any Js node as the root and there's no `JsAnyNode`
* It isn't guaranteed that the `root` type doesn't change in case the `root` node is a `JsParenthesizedExpression` that gets removed.

## Tests

`cargo test && rustcs`
@MichaReiser MichaReiser force-pushed the refactor/batch-mutation-syntax-node branch from aafbc0e to 6dd962f Compare August 8, 2022 10:51
@MichaReiser MichaReiser temporarily deployed to aws August 8, 2022 10:51 Inactive
@MichaReiser MichaReiser temporarily deployed to aws August 8, 2022 13:32 Inactive
@MichaReiser MichaReiser temporarily deployed to aws August 8, 2022 13:38 Inactive
@MichaReiser MichaReiser temporarily deployed to aws August 8, 2022 14:00 Inactive
@MichaReiser MichaReiser merged commit aaab879 into main Aug 8, 2022
@MichaReiser MichaReiser deleted the refactor/batch-mutation-syntax-node branch August 8, 2022 14:50
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Aug 10, 2022
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载