Conversation
…rations." This reverts commit d456656.
…tes" This reverts commit da77b35.
Will un-revert once the new mechanism is in place. This reverts commit 04865cb.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 4a60ad0.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Gets the node corresponding to the field `parameter`. */ | ||
| final F::Parameter getParameter(int i) { | ||
| final override F::Parameter getParameter(int i) { |
| F::Block getBody() { none() } | ||
|
|
||
| /** Gets the node corresponding to the field `parameter`. */ | ||
| F::Parameter getParameter(int i) { none() } |
|
|
||
| /** Gets the node corresponding to the field `parameter`. */ | ||
| final F::Parameter getParameter(int i) { | ||
| final override F::Parameter getParameter(int i) { |
|
|
||
| /** Gets the node corresponding to the field `parameter`. */ | ||
| final F::Parameter getParameter(int i) { | ||
| final override F::Parameter getParameter(int i) { |
|
|
||
| /** Gets the node corresponding to the field `parameter`. */ | ||
| final F::Parameter getParameter(int i) { unified_function_expr_parameter(this, i, result) } | ||
| final override F::Parameter getParameter(int i) { |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Tests do not cover omitted fields or covariant getter types, despite these being core supported scenarios.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds explicitly declared fields to AST supertypes and generates callable getter APIs.
Changes:
- Supports detailed supertype declarations with fields.
- Generates default supertype getters and concrete overrides.
- Adds schema conversion and generator tests.
| File | Description |
|---|---|
unified/ql/lib/codeql/unified/internal/Ast.qll |
Updates generated callable getters. |
unified/extractor/ast_types.yml |
Declares callable fields. |
shared/yeast-schema/src/node_types_yaml.rs |
Parses and serializes supertype fields. |
shared/tree-sitter-extractor/src/node_types.rs |
Preserves fields on union entries. |
shared/tree-sitter-extractor/src/generator/ql.rs |
Removes abstract predicate-body representation. |
shared/tree-sitter-extractor/src/generator/ql_gen.rs |
Generates supertype getters and overrides. |
shared/tree-sitter-extractor/src/generator/mod.rs |
Handles expanded union entries. |
shared/tree-sitter-extractor/src/extractor/mod.rs |
Handles expanded union entries during matching. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let yaml = r#" | ||
| supertypes: | ||
| callable: | ||
| subtypes: [function] |
Contributor
Author
There was a problem hiding this comment.
I think we can live with this
Contributor
Author
Rerun has been triggered: 4 restarted 🚀 |
This branch has not been deployed
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.
Supertypes in
ast_nodes.ymlcan now have a list of fields to expose in the generated QL class.This replaces the previous rule from #22507 where fields that were common among all subtypes were automatically hoisted to supertypes. The first couple of commits revert the changes from that PR, mainly to make things easier to review and avoid leaving behind weird bits of legacy code.
Compared to automatic hoisting, manually-declared supertype fields work better in a few scenarios:
This PR uses the feature for the
callablesupertype:Previously the
bodyfield was hoisted automatically, but notparameter,because some callables can't have parameters (like top-level).