Unified: Get rid of all $children fields#21841
Open
tausbn wants to merge 31 commits into
Open
Conversation
Part 1 of N of "getting rid of $children" in node-types.yml Note: in one of the cases the affected node still has the $children field present. This is because there's some weirdness about recording multiline comments as class member separators that I did not want to figure out how to address right now.
I ended up also aliasing `_async_keyword` to a named node to make it more consistent with the other node kinds that can be in this field (as it would be awkward to have two named types and a token here). Elsewhere in the node types, we'll still have `async?: "async"`, and I think that's okay.
A lot of changes, but for the most part these are just adding named fields in places where they make sense. After this, there are still ~20 instances of unnamed children appearing.
This required a change in a different place, due to aliasing.
Some nodes with a single child (arguably redundant to do, but I think it's nice to have the types be consistent), and also an instance of ensuring that all branches of a `choice` expose consistent field names.
Not entirely happy about the mixed nature of the `kind` filed (having both tokens and the named node `throw_keyword` in there), but that's a problem for a different time.
Doing this involved materialising a lot of previously anonymous nodes, and I'm not entirely sure it's the best solution, but the node types look decent enough.
Of note: this involved un-inlining where_clause.
Not entirely sure about the `binding?` field on `pattern`, but it looks like that might actually be useful.
Mostly by materialising a bunch of (useful) intermediate nodes.
I'm not entirely happy about this solution, but it seemed to be the most straightforward way of avoiding various kinds of token bleeding.
This named node (which is in fact emitted by the scanner as an `external`) was appearing as a child of `class_body` because of inlining via `_class_member_separator`. This, in itself, appears to be somewhat of a hack, to handle cases where a multiline comment signals the end of a class member. To fix this, we make the external node _unnamed_, but keep the `extras` node _named_ (so we can still extract it from the parse tree), and we add a new rule `multiline_comment` that mediates between the two. That way, the use inside `_class_member_separator` can use the unnamed variant, and no node is pushed into $children.
Introduces (by making it named) a `block` node, and conversely makes `statements` anonymous. This enables us to sensibly distinguish between the "then" and "else" branch of an `if_statement`, which we were not able to previously.
The field representation would have made it difficult to figure out which parameters correspond to which default values and attributes, so instead we now encapsulate these in a new `function_parameter` node.
Same as in the preceding commit, these items do not make sense as separate fields on the parent node, so we materialise (or create new) intermediate nodes to group them together.
Same as in the preceding commit.
This groups together a bunch of related values that would otherwise be impossible to match up correctly.
All of these fields have contents that are uniquely determined by the node they appear on, so they convey no information.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the unified Swift AST shape to eliminate generic $children lists in favor of named, typed fields (and a few new wrapper nodes), aligning the tree-sitter grammar, node-types metadata, and the CodeQL dbscheme.
Changes:
- Replaces many Swift node
$children/childrelations with explicit named fields/relations inunified.dbscheme. - Updates
node-types.ymlto describe the new fielded AST structure (e.g.,block.statement*,call_expression.function/suffix,tuple_expression.element+, etc.). - Adjusts the Swift tree-sitter grammar to emit the new fields and new node types (e.g.,
block,simple_user_type,parenthesized_type,tuple_expression_item,dictionary_literal_item).
Show a summary per file
| File | Description |
|---|---|
| unified/ql/lib/unified.dbscheme | Reshapes the Swift schema to remove $children-style tables and introduce explicit field relations and new node types. |
| unified/extractor/tree-sitter-swift/node-types.yml | Updates node field definitions to match the new named-field AST (removing $children usage). |
| unified/extractor/tree-sitter-swift/grammar.js | Updates parsing rules/fields and introduces/renames nodes to produce the new AST shape. |
Copilot's findings
- Files reviewed: 2/4 changed files
- Comments generated: 1
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A grab bag of changes, mostly making nodes named, or adding fields, and in a few cases doing some more advanced rewrites. Can be reviewed commit-by-commit.