It seems that using swift_version_file has some issues when `codeql` is
consumed as a dependency module. I'm hoping hardcoding the version
instead will fix this, but ideally we should find a more robust
solution.
- BUILD.bazel: require x86_64 on the Linux branch of the compatibility
select. The only registered standalone Linux Swift toolchain is
x86_64-only, so without the CPU constraint Linux/aarch64 targets
stayed
"compatible" and then failed toolchain resolution instead of being
skipped cleanly.
- build.rs: emitting `rerun-if-changed` disables Cargo's default
whole-package scan, so list every input to `swift build` — the package
manifests, `Package.resolved`, `.swift-version`, and the whole Sources
tree — so stale shim/toolchain output can't linger. (`.build/` is left
unwatched, as it is this build's own output.)
- src/lib.rs: a null result from the shim is not a parse failure —
SwiftParser recovers from invalid syntax and always yields a tree — so
it
means the shim failed to serialize/allocate the JSON. Fix the error
message and the variant doc accordingly.
- README.md: `.swift-version` is not honored by the macOS Bazel build
(which uses the host Xcode toolchain); document that it pins the Linux
and local builds only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Also gathers these into a separate side-channel during the initial AST
construction. That way, we don't encounter these as weird extra nodes
while running yeast.
Introduces new yeast types for `Point`s and `Range`s (corresponding to
the tree-sitter equivalents), since it seemed silly to have
`swift-syntax-rs` pull in `tree-sitter` just to have those types
available.
This _does_ mean there is a slight overhead in the shared extractor when
converting between these types, but I think this is negligible.
Adds a preliminary mapping that decodes the JSON into the yeast AST. The
JSON AST itself is still very verbose, but it would be useful to see if
it actually contains all of the things we want it to contain.
Adds an initial prototype of an interface from Rust to Swift, which
enables us to use the `swift-syntax` package for parsing.
At present, the parsed AST is passed between Swift and Rust as a JSON
string.
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).
