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

chore(rome_js_syntax): Delete the unused JsConstructorParameter kind #3228

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

MichaReiser
Copy link
Contributor

Summary

This PR deletes the unused JsSyntaxKind::JS_CONSTRUCTOR_PARAMETER.

Test Plan

cargo build

@MichaReiser MichaReiser requested a review from a team September 15, 2022 11:50
@MichaReiser MichaReiser temporarily deployed to netlify-playground September 15, 2022 11:50 Inactive
@netlify
Copy link

netlify bot commented Sep 15, 2022

Deploy Preview for rometools ready!

Name Link
🔨 Latest commit cf46bbb
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/63231173a8dead0009d15406
😎 Deploy Preview https://deploy-preview-3228--rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

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 12395 12395 0
Failed 3862 3862 0
Panics 0 0 0
Coverage 76.24% 76.24% 0.00%

@github-actions
Copy link

@MichaReiser MichaReiser added this to the 0.10.0 milestone Sep 15, 2022
Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way to generate these kinds from the grammar.. or make sure there aren't dangling keywords like in this example.

@MichaReiser
Copy link
Contributor Author

I wonder if there's a way to generate these kinds from the grammar.. or make sure there aren't dangling keywords like in this example.

Yes, that would be a nice improvement. @IWANABETHATGUY would this be something you have an interest in?

@MichaReiser MichaReiser merged commit 7744815 into main Sep 15, 2022
@MichaReiser MichaReiser deleted the chore/delete-js-constructor-parameter branch September 15, 2022 12:25
@IWANABETHATGUY
Copy link
Contributor

Yeah, let us open an issue to track this task.

@IWANABETHATGUY
Copy link
Contributor

Could we guarantee that all the node in KIND_SRC.nodes are defined in xx.ungram ?
More specifically, SET(KIND_SRC.nodes) == SET(xx.ungram.definition) ?

@MichaReiser
Copy link
Contributor Author

KIND_SRC

I'm not entirely sure what you're saying. What I understand is if it is guaranteed that all nodes are defined in the .ungram file and there are no other nodes (or tokens).

I think this is safe to assume because any node that isn't defined in the ungram file won't have a corresponding AST node.

@IWANABETHATGUY
Copy link
Contributor

defined

I mean if some node_kind defined in KIND_SRC.nodes

but not in xx.ungram file, vice versa, it will not be safe to apply your advice the generating code would be broken.

@IWANABETHATGUY
Copy link
Contributor

sorry for my poor expression.

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.

3 participants