Turns out, `(_)` would match both named and unnamed nodes, as we never
checked the value of the `match_unnamed` field. This is the real reason
why the final catch-all rule we removed in the last commit was
superfluous -- unnamed nodes were being caught by the penultimate rule
instead (and mapped to `unsupported_node`).
Having fixed the bug, we now (correctly) get errors due to unmatched
unnamed nodes in the input. To fix this, we change the catch-all rule to
match unnamed nodes as well. This restores the previous behaviour
exactly.
At some point, we should find a better way to handle unnamed nodes, as
it seems wasteful to map these to `unsupported_node` (since we in
practice only use them for their string content). Perhaps we should not
attempt to translate unnamed nodes at all?
In order to facilitate static type checking of rules (and to make it
easier for human readers as well), rust blocks at the root level (i.e.
rules of the form `... => { ... }`) must now have a type annotation in
front.
All other forms are unaffected: if the right hand side of a rule is a
tree, we can read the type of the root node directly. For interpolations
that happen inside of such a tree, we can recover the type by looking at
what field we're interpolating into, and consulting the output schema.
All existing uses have been updated to have the appropriate type
annotations, though these are of course not checked yet (and so could be
wrong).
Finally, this commit also removes the final catch-all rule `_ @node =>
{node}`. Because of the preceding rule that matches `(_) @node`, this
rule would only ever match unnamed nodes, and I think in practice it did
not match at all (at least not in our current set of tests).
To give it a proper type we would have to add some notion of an "any"
type, which I would like to avoid. If it _does_ turn out to be needed,
we can easily add it back (ideally with a test-case that shows why it's
still needed).
This macro allows the easy addition of multiple rules at the same time.
In addition, it also accepts an input and output schema, which
eventually will be used to check the validity of the rewrite rules.
In order to avoid having context changes bubble up through the tree (or
from one sibling to another) the current framework makes a copy of the
context before calling `translate` recursively, and then restores it
afterwards.
However, this is a bit silly -- after we're done with all of the
translations, there's really no need to restore the context (as it
doesn't get accessed again).
So instead we change it from "save and then restore" to "clone and then
drop". Each rule invocation gets its own copy of the context, and simply
drops it when it's done.
A previous commit added a translate_reset method on BuildCtx, which had
the effect of performing a translation in a completely empty context.
One issue with this is that this is an all-or-nothing proposition -- If
you want to preserve _some_ parts of the context, you have to do
something more complicated. Moreover, if you introduce a contextual
value that _should_ be preserved, all of the existing uses of
translate_reset now silently do the wrong thing.
There are two patterns that we want to address. The first one is "modify
the context in some way, then do a translation". If the translation is
the last step of a Rust block, then we don't actually need
translate_reset -- we could just reset the context and then call
`translate`. The fact that the outer context is restored afterwards
means it's okay to make destructive changes to `ctx.user_ctx` -- none of
these changes will persist.
The second pattern is the same, but where we want to do more
translations using the original context, after having performed a
translation with a modified context. In this case, we cannot just
overwrite the context, since that would invalidate the subsequent
translations.
Instead, we introduce a new method `ctx.scoped` which takes a closure as
an argument. With this we can now write
```
ctx.scoped(|ctx| ctx.reset(); ctx.translate(...));
```
and the closure is run with a copy of `ctx` that has a clone of
`user_ctx` on the inside, so no changes will persist.
(You may wonder: why not just clone `ctx` and use the clone? The answer
is that `ctx` owns mutable pointers to the AST etc., and this makes it
awkward to just "clone" it. The closure circumvents this issue nicely,
since it can borrow these pointers internally.)
For now, this rewrite has the same behaviour as the version that used
translate_reset -- we clear the entire `user_ctx`. However, we could
imagine being more fine-grained in this approach, by implementing, say,
SwiftContext::reset_modifiers (which would only affect what modifiers
are currently in the context, leaving everything else as-is).
The corpus tests interleaved hand-written content (test cases) with
generated content (printed ASTs).
This made merge conflicts hard to resolve because you can't just
regnerate the printed ASTs without potentially throwing away new test
cases that came from either branch (or depending on whether the merge
conflict markers appeared, the corpus test could be ruined completely).
The old design did have one nice advantage: Reviewers could see the
printed ASTs alongside the source code from which it was generated.
To preserve this feature, the source code for the test case is itself
included in the generated output file.
In the initial implementation of yeast, the splice syntax was needed do
distinguish between splicing multiple nodes or just a single node.
However, this was always an ugly "wart" in the syntax, since the user
shouldn't have to worry about these things.
To fix this, we add an `IntoFieldIds` trait that dispatches on the
value's type: `Id` pushes a single id, and a blanket impl for
`IntoIterator<Item: Into<Id>>` handles `Vec<Id>`, `Option<Id>`, and
arbitrary iterator chains.
With this, we no longer need to use the special splice syntax, and hence
we can get rid of it.
Previously, the `Id` type was a bare usize alias. The `NodeRef` newtype
existed solely to carry the AST-aware `YeastDisplay` /
`YeastSourceRange` impls (so that `#{captured_node}` rendered source
text rather than the numeric id) without colliding with the impls for
raw integer types.
This commit promotes `Id` itself to a (transparent) newtype struct and
moves the AST-aware trait impls directly onto it. With `Id` and `usize`
now being different types, the integer-display impl (for `usize`) and
the source-text impl (for `Id`) coexist without conflict, and `NodeRef`
becomes redundant (and so we remove it).
- unified/swift: Mark `binding_kind` as a raw `@@` capture in the
property_declaration rule. It is only used to read its source text
(`ctx.ast.source_text`), never as a translated node. With `@` the
auto-translate prefix would route the unnamed `let`/`var` token
through the catch-all `_ @node => {node}` fallback for a no-op
roundtrip; `@@` makes the intent explicit and removes that reliance.
- shared/yeast/tests: Reword a stale comment in test_raw_capture_marker.
The text claimed a "second assertion" exists in this test, but the
explicit-translation check actually lives in the companion
test_raw_capture_marker_explicit_translate.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
With `@@name` available, there's no longer a need to use `manual_rule!`.
Every place where it is used, we can instead just mark the relevant raw
captures as such. This results in quite a lot of cleanup! (Also, to me
at least, it makes these rules a lot easier to reason about.)
A first iteration of this approach resulted in a lot of
`.map(Into::into)` being needed, because `SwiftContext` stores `Id`s,
but captures produce `NodeRef`s. To avoid this, I swapped it around so
that the context stores `NodeRef`s. This does require adding `.into()`
in a few places, but it makes the rest of the code a lot more ergonomic.
Format the touched Rust crates (shared/tree-sitter-extractor,
shared/yeast, shared/yeast-macros, unified/extractor) so the
tree-sitter-extractor CI fmt check passes. No functional changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cleans up a few places where we were constructing trees piece by piece
rather than using the `tree!` macro.
In the process, Copilot noticed an issue that should probably be
addressed: the labeled_statement rule can never fire, since there are no
such nodes in the input. This is possibly a simple as making
_labeled_statement (which _does_ exist) named, but I haven't attempted
this.
Finally, a small change to yeast makes it so that the contents of a {}
interpolation can be a Rust block (previously it could only be a single
expression). This avoids the need to double-wrap instances where you
want to interpolate a single node produced as the final value of some
block.
(Both reduce_left and map are still supported, but we could remove them
at this point.)
I think this way of writing things makes the intent a lot clearer -- it
avoids extending the yeast rule language with complicated constructs,
pushing the complexity (such as it is) into Rust instead.
Gets rid of the final uses of mutation (via prepend_field). The approach
is the same as in the preceding commits: we set the appropriate fields
on the context when processing the outer node, and then access these
fields on the inner nodes.
The repeated use of `modifier` fields is a _bit_ clunky, but since we're
likely moving to an out-of-band modifier mechanism at some point, I
think it's good enough for now.
