Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ungrammar, codegen, css_parser): support "dynamic" unordered node fields #1438

Merged
merged 9 commits into from
Jan 5, 2024

Conversation

faultyserver
Copy link
Contributor

@faultyserver faultyserver commented Jan 5, 2024

Summary

This PR implements the idea behind #1407, which is the ability to define, parse, represent, and interact with unordered fields on a node.

Without repeating too much of that discussion, the idea is that the grammar can define a set of fields that are unordered: each field can appear at most once in the node, but they can appear in any order in the set. In an AST, this is simple, but in a CST we need to be able to map the access for a specific field into the ordered, linear stream of tokens and nodes of the untyped syntax tree.

To do this, AstNodes with one or more unordered fields also include a slot_map member that maps a defined slot index (the index of the field in the grammar) to a concrete index (the index of the field in the source code token stream). When converting from an untyped node to a typed node, the constructor builds the slot map by comparing the children against the specified grammar and assigning the slots. This also happens when up-casting the raw token stream to the SyntaxNode, but only for validation and filling empty slots to ensure missing and required empties are filled.

Test Plan

There's nothing that uses this yet, but I made a sample grammar with unordered nodes, ran codegen, and tested a parse result that you can see in this commit.

Copy link

netlify bot commented Jan 5, 2024

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit c5a3fbb
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6598616bc943960008ecad31

@github-actions github-actions bot added A-Core Area: core A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS labels Jan 5, 2024
Copy link
Contributor

github-actions bot commented Jan 5, 2024

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 49701 49701 0
Passed 48721 48721 0
Failed 980 980 0
Panics 0 0 0
Coverage 98.03% 98.03% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6322 6322 0
Passed 2036 2036 0
Failed 4286 4286 0
Panics 0 0 0
Coverage 32.20% 32.20% 0.00%

ts/babel

Test result main count This PR count Difference
Total 662 662 0
Passed 592 592 0
Failed 70 70 0
Panics 0 0 0
Coverage 89.43% 89.43% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17646 17646 0
Passed 13452 13452 0
Failed 4192 4192 0
Panics 2 2 0
Coverage 76.23% 76.23% 0.00%

Some(CombinatorKind::DoublePipe) => Rule::UnorderedSome(rules),
Some(CombinatorKind::Pipe) => Rule::Alt(rules),
None | Some(CombinatorKind::NonCombinator) => {
unreachable!("Matched more than one rule but didn't determine a combinator")
Copy link
Member

Choose a reason for hiding this comment

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

What should we do in case we mistakenly reach this code path?

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 should be genuinely unreachable, since we break out of the loop before parsing a second rule if we encounter a non-combinator, and we also bail the parse entirely if we encounter mixed combinators, so I think this is safe as-is. Reaching this state would mean there's both a bug in the parsing logic above it and an unexpected input.

@github-actions github-actions bot added L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages labels Jan 5, 2024
@faultyserver faultyserver force-pushed the faulty/ungrammar-unordered-syntax branch from 5642944 to e890ec5 Compare January 5, 2024 19:55
@faultyserver faultyserver merged commit 4607357 into main Jan 5, 2024
18 checks passed
@faultyserver faultyserver deleted the faulty/ungrammar-unordered-syntax branch January 5, 2024 20:28
@Conaclos Conaclos added the A-Changelog Area: changelog label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Core Area: core A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants