Previously, apply_rules_inner snapshotted a node's fields by cloning
the BTreeMap into a Vec<(FieldId, Vec<Id>)>, then built a fresh
BTreeMap of new_fields for the rewritten Ids. For a node with N
fields, this allocated 2N+1 things per visit (the snapshot Vec, N
cloned children Vecs, the new BTreeMap entries) — even when nothing
in the subtree was rewritten.
Use std::mem::take to swap the parent's fields out by ownership: the
recursion can mutate the AST (including pushing new nodes from rule
firings) without any conflict, since we hold the owned BTreeMap
locally. Iterate values_mut() and only allocate a fresh children Vec
on the first divergence (lazy alloc): unchanged children stay in the
existing slot. When done, swap the fields back.
For a subtree with no rewrites, this is now zero allocations per node
(modulo the recursion itself). For nodes with rewrites, it's one Vec
allocation per field that contains a rewritten child, instead of two
plus the BTreeMap rebuild.
apply_rules_inner used to handle the "child was rewritten, so the
parent needs new field IDs" case by cloning the parent node, swapping
in the new fields, pushing the clone onto the arena, and returning the
new Id. Every ancestor on the path from the rewrite up to the root was
duplicated this way, with the originals retained as garbage in the
arena.
Switch to in-place mutation: assign `ast.nodes[id].fields = new_fields`
and return the same Id. Rule firings still produce genuinely new nodes
via BuildCtx (their structure differs from the input), but the
ancestor-rebuild spine no longer copies anything.
This is safe because apply_rules_inner already works entirely by Id:
the field snapshot is cloned out before recursing, no &Node references
are held across mutations of the arena, and captures are scoped to a
single rule firing so the now-stable Ids do not break anything.
Memory effect: a desugaring pass that rewrites R leaves of a tree of
average depth d previously appended R*d ancestor clones to the arena.
Now appends 0.
With Ids stable for the lifetime of an Ast, the Node::id field becomes
truly redundant and is removed (along with the Node::id() accessor).
AstCursor switches from caching `node: &Node` to tracking `node_id:
Id` and looking the node up via the arena on each access; ChildrenIter
now yields Ids directly. A new AstCursor::node_id() method gives
callers access to the cursor position by Id.
Previously, a bare child pattern in a query took whatever the next
child of the iterator was and either matched or failed: it would not
scan ahead to find a match. So `(foo ("baz"))` against a `foo` whose
implicit `child` field was `["bar", "baz"]` would fail (the pattern
took "bar" first).
Switch to forward-scan semantics: a SingleNode matcher advances through
the iterator until it finds a child that matches its sub-query. Patterns
that are named-only continue to skip past unnamed children for free.
Order is preserved across multiple bare patterns at the same level —
each pattern advances the shared iterator past whatever it consumed —
so a query cannot match children out of source order.
Captures from a failed match attempt are rolled back via a snapshot, so
partial captures from a complex sub-query do not leak across attempts.
Add two regression tests against the `do` body wrapper in a Ruby
for-loop, whose implicit `child` field contains [do, identifier, end]:
- a query for ("end") matches by skipping past `do` and the identifier
- a query for ("end") then ("do") fails, demonstrating order preservation
Schema::from_language registered unnamed kinds via or_insert(id), where
`id` came from iterating 0..node_kind_count. For names with multiple
unnamed IDs (notably "end" in tree-sitter-ruby has IDs 0 and 13, where
ID 0 is the reserved error token), this picked the first encountered
ID — typically the wrong one.
The visitor sets node.kind via language.id_for_node_kind(name, false),
which returns the canonical ID. So a query for ("end") would compare
node.kind=13 against schema=0 and silently fail to match, with no
diagnostic.
Use language.id_for_node_kind(name, false) to obtain the canonical ID
when registering, mirroring the named-kind path that already does the
same with id_for_node_kind(name, true).
Three improvements to the query parser, all aimed at allowing query
patterns to refer to unnamed tokens:
1. Bare-literal capture: `"=" @op` now captures the unnamed `=` token,
matching the parenthesized form `("=") @op`. Previously the literal
branch in parse_query_list skipped the maybe_wrap_capture call, so
the `@op` was a leftover token and would error.
2. Bare `_` matches any node, named or unnamed. Previously bare `_` and
`(_)` both produced QueryNode::Any with the same matches_named_only
behaviour, so bare `_` would skip unnamed children. Now Any carries a
match_unnamed flag: false for `(_)` (named-only, tree-sitter default)
and true for bare `_` (any node).
3. Named fields and bare child patterns may be intermixed in any order.
Previously, once parse_query_fields saw a bare pattern it would stop
accepting named fields. The fix accumulates bare patterns into the
implicit `child` field and keeps parsing.
Each named field independently selects its target field for matching, so
the source-order of fields in the query is purely cosmetic and intermixing
is safe.
Add tests covering parenthesized capture, bare-literal capture, and the
named-vs-any distinction between `(_)` and bare `_`. Update query-syntax
docs to reflect all three.
Extend the desugaring config from a single flat list of rules to an
ordered sequence of named Phases. Each phase runs to completion (a
full traversal applying its rules) before the next phase starts.
Rules in different phases never compete for matches.
The config is built via the new chainable API:
DesugaringConfig::new()
.add_phase("cleanup", cleanup_rules)
.add_phase("desugar", desugar_rules)
.with_output_node_types_yaml(yaml);
Single-phase configs are just .add_phase(...) called once.
A single FreshScope is shared across phases so generated identifier
names (e.g. $tmp-N) are unique throughout the run.
Phase names appear in error messages, e.g. "Phase `desugar`:
exceeded maximum rewrite depth".
Add two regression tests: one verifying basic two-phase chained
desugaring, and one verifying that errors include the failing phase
name.
Previously, after a rule fired the engine would always re-try that
same rule on the result root. A rule whose output matched its own
query (intentionally or by accident) would loop until the global
MAX_REWRITE_DEPTH safety net kicked in.
Make the default behavior fire-once-per-node: after a rule fires on
node N, the engine no longer tries that same rule on the result root.
Other rules and child traversal are unaffected. Rules that
intentionally rewrite iteratively can opt into the old behavior via
the new Rule::repeated() builder method.
Add two regression tests using a self-swapping assignment rule:
- with .repeated(), the swap loops and trips the depth limit
- without it (default), the swap fires once and terminates
Adds a regression test verifying that desugaring rules can chain across
output-only node kinds: a first rule rewrites an input kind to an
output-only kind, and a second rule then rewrites that output-only
kind into another output-only kind. This exercises the schema lookup
for query patterns whose root kind is not present in the input
tree-sitter grammar.
Add BUILD.bazel files for the yeast and yeast-macros crates, register
them as dependencies of the shared tree-sitter extractor, and refresh
the vendored crate dependencies via update_tree_sitter_extractors_deps.sh.
Language and LanguageSpec gain optional output_node_types field.
When set, the generator produces dbscheme/QL from the output types
and the extractor validates TRAP against them.
All existing extractors pass None (no behavior change).
Ruby extract() calls gain vec![] for the new rules parameter.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
extract() gains a rules parameter. When empty, uses tree-sitter native
traversal (no behavior change). When non-empty, runs yeast desugaring
and extracts via traverse_yeast.
Adds AstNode trait abstracting over tree_sitter::Node and yeast::Node,
with minimal changes to existing Visitor methods (Node -> &N in 6
signatures).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
12 tests covering parsing, queries, tree building, desugaring rules,
cursor navigation, and the shorthand rule! syntax.
Tests use a custom output node-types.yml with named fields for all
children (parameter, stmt, index), loaded via
schema_from_yaml_with_language.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Produces indented text showing node kinds, named fields, and leaf
content. Unnamed tokens are hidden unless inside a named field.
Used by tests for readable assertions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Human-friendly YAML alternative to tree-sitter node-types.json with
three sections: supertypes, named, unnamed. Supports bidirectional
conversion and building Schema objects from YAML.
Includes CLI binary (node_types_yaml) and documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- shared/mad/codeql/mad/ModelValidation.qll: shorten the comment
for `path-injection[%]` to `// Java-only currently`, matching the
style of other language-scoped entries and dropping API examples
and the java/zipslip reference.
- java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll: replace
the `File.exists` example in the QLDoc with `FileReader`, since
`File.exists` is still labelled plain `path-injection`, not
`path-injection[read]`.
Consider two implementations of the same trait to be siblings when the
type being implemented by one is an instantiation of the type being
implemented by the other.
Introduce a new Models-as-Data sink sub-kind path-injection[read] for
models that only read from or inspect a path. The general
java/path-injection query and its PathInjectionSanitizer barrier
continue to consider both path-injection and path-injection[read]
sinks, so no alerts are lost. The java/zipslip query deliberately
selects only path-injection sinks, since read-only accesses such as
ClassLoader.getResource or FileInputStream are outside the archive
extraction threat model.
Addresses https://github.com/github/codeql/issues/21606 along the lines
proposed on the issue thread: prefer path-injection[read] over a
[create] sub-kind so that miscategorizing a sink causes a false
positive (easy to spot) rather than a false negative.
- shared/mad/codeql/mad/ModelValidation.qll: allow path-injection[...]
as a valid sink kind.
- java/ql/lib/ext/*.model.yml: relabel the models that PR #12916
migrated from the historical read-file kind (plus the newer
ClassLoader resource-lookup variants that share the same read-only
semantics).
- java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll and
PathSanitizer.qll: select both path-injection and
path-injection[read] sinks/barriers.
- java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll: keep only
path-injection, with a comment explaining why path-injection[read]
is excluded.
- java/ql/test/query-tests/security/CWE-022/semmle/tests/ZipTest.java:
add m7 regression covering the Dubbo-style classpath lookup from
issue #21606 and assert no alert is produced.
- Update TaintedPath.expected for the renamed kinds in the models list.
- Add change-notes under java/ql/lib/change-notes and
java/ql/src/change-notes.
