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

feat(rome_css_syntax): codegen for CSS grammar #2363

Merged
merged 7 commits into from
Apr 7, 2022
Merged

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Apr 6, 2022

Summary

This PR is part of #2350

It starts to spin the ball around multi language support, by generating the code for the CSS grammar. The scope of this PR is exclusively having a code generation that works and emits code that compiles.

There are few things won't be tackled in this PR:

  • repeated code between rome_js_syntax and rome_css_syntax; for AstNode is repeated at the moment and it's intentional for now;
  • how valid is the css.ungram; it is still incomplete and it doesn't hold the grammar, the code was shown to Nicholas and Micha few weeks back to show them the differences from the JS grammar and if we can achieve that;

What's been done:

  • created a new rome_css_syntax crate that contains the generated code and some other code related to the syntax. Some code is copied and repeated;
  • the code generation now uses a LanguageKind to generate different code accordingly to the value of the variant;
  • handled some edge cases that we now have with CSS. We have the $= which causes issues inside Rust ($ is mistaken as a pattern of a macro), so I had to play with proc_macro2 to generate the correct code;
  • created an empty crate called rome_syntax where we will have our shared syntax, such as AstNode, AstSeparatedList, etc.

Test Plan

cargo check should compile

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 6, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ea41599
Status: ✅  Deploy successful!
Preview URL: https://18da30b6.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Apr 6, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45250 45250 0
Passed 44310 44310 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.92% 97.92% 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 584 584 0
Passed 508 508 0
Failed 76 76 0
Panics 0 0 0
Coverage 86.99% 86.99% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 15983 15983 0
Passed 12167 12167 0
Failed 3816 3816 0
Panics 0 0 0
Coverage 76.12% 76.12% 0.00%

@ematipico ematipico force-pushed the rfc/css-grammar branch 2 times, most recently from 8020b75 to 487970d Compare April 6, 2022 14:51
xtask/codegen/src/generate_syntax_kinds.rs Outdated Show resolved Hide resolved
crates/rome_css_syntax/src/syntax_node.rs Show resolved Hide resolved
xtask/codegen/src/css_kinds_src.rs Outdated Show resolved Hide resolved

impl From<u16> for CssSyntaxKind {
fn from(d: u16) -> CssSyntaxKind {
assert!(d <= (CssSyntaxKind::__LAST as u16));
Copy link
Contributor

Choose a reason for hiding this comment

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

Equals here make sense? Given that __LAST is just a marker and not a node.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using equals ensures it's symmetric with the impl From<CssSyntaxKind> for u16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a copy paste from rome_js_syntax. I think it would make senses to keep debug_assert, or remove it. Although the same change should be applied to the other crate too. I would leave it out of the scope for now, for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay as an assert! since it's guarding unsafe code, if something goes wrong with this conversion a panic will be a lot easier to debug than some random undefined behavior down the line when the code matches on an invalid enum

crates/rome_syntax/src/lib.rs Outdated Show resolved Hide resolved

impl From<u16> for CssSyntaxKind {
fn from(d: u16) -> CssSyntaxKind {
assert!(d <= (CssSyntaxKind::__LAST as u16));
Copy link
Contributor

Choose a reason for hiding this comment

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

Using equals ensures it's symmetric with the impl From<CssSyntaxKind> for u16

crates/rome_css_syntax/src/lib.rs Show resolved Hide resolved
crates/rome_css_syntax/src/lib.rs Show resolved Hide resolved
crates/rome_css_syntax/src/lib.rs Show resolved Hide resolved
crates/rome_css_syntax/src/util.rs Show resolved Hide resolved
xtask/codegen/src/css_kinds_src.rs Outdated Show resolved Hide resolved
xtask/codegen/src/generate_nodes.rs Outdated Show resolved Hide resolved
xtask/codegen/src/generate_syntax_kinds.rs Outdated Show resolved Hide resolved
@ematipico ematipico merged commit 1d3bda0 into main Apr 7, 2022
@ematipico ematipico deleted the rfc/css-grammar branch April 7, 2022 11:46
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