Conversation
These were inadvertently being translated into `unsupported_node`s.
This node type will be used to represent nodes that -- during translation -- manage to reach the catch-all rule at the end. This almost certainly indicates something went wrong. In contrast, `unsupported_node` will now be used for nodes that we're aware of, but which have not proper translation just yet.
Adds rules for each of the node types that we currently do not support. This means if the fallback clause actually fires, then we've encountered a truly new node type (or something that shouldn't have been translated, like a token). In this case, we now emit an error message (but this does not affect extraction -- it'll succeed with an `unhandled_node` in the output).
tausbn
force-pushed
the
tausbn/unified-distinguish-unhandled-from-unsupported-nodes
branch
from
September 24, 2026 15:01
97df2bf to
ebac209
Compare
tausbn
marked this pull request as ready for review
September 24, 2026 19:18
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new diagnostic fallback lacks regression coverage demonstrating that extraction continues with an unhandled_node.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Introduces a distinct AST node for unexpected Swift syntax while retaining unsupported_node for recognized but intentionally unmapped syntax.
Changes:
- Adds
unhandled_nodeto the Unified AST and generated QL schema. - Reports unexpected syntax nodes while continuing extraction.
- Corrects token captures to bypass recursive translation.
| File | Description |
|---|---|
unified/extractor/ast_types.yml |
Defines the new node type. |
unified/extractor/src/languages/swift/swift.rs |
Classifies fallbacks and fixes captures. |
unified/ql/lib/codeql/unified/internal/Ast.qll |
Exposes UnhandledNode to QL. |
unified/ql/lib/unified.dbscheme |
Adds the generated database type. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| rule!( | ||
| _ @@node | ||
| => | ||
| unhandled_node { |
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.
Essentially, unsupported nodes are nodes that we are aware of, but do not currently map to anything useful in the unified AST. Unhandled nodes indicate an error (and we report it as such, but continue the extraction anyway) -- either a new node type that we're not handling, or an accidental translation of a token.
The addition of this mechanism revealed a bunch of places where we were using translating captures instead of raw captures for a bunch of different tokens. These have all been fixed.
Can be reviewed commit-by-commit.