Uh oh!
There was an error while loading. Please reload this page.
yeast: Miscellaneous cleanup - #22084
Merged
Merged
Conversation
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).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.
`Captures::map_captures`, `Captures::map_captures_to`, and `Captures::try_map_all_captures` had no callers. The last one was subsumed by `try_map_captures_except` (which takes a skip list and degenerates to the old behaviour when the list is empty). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`translate_opt` was a convenience for the manual_rule! body code, collapsing `Option<I>` to `Option<Id>` via `translate`. Since the `@@` raw-capture migration replaced manual_rule! with rule!, no callers remain — the auto-translate prefix handles `Option<Id>` captures directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`BuildCtx::prepend_field` and the underlying `Ast::prepend_field_child` existed to support the create-then-mutate pattern in swift.rs (build an output node, then prepend modifiers to its `modifier:` field). The SwiftContext-based refactor on the previous branches eliminated all such call sites: every emitted declaration now carries its modifiers from birth, so the in-place prepend operation has no users. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both accessors returned the same private `kind_name: &'static str` field; `kind_name()` is widely used (mainly by dump.rs and schema diagnostics) and `kind()` had only 2 internal callers in lib.rs and a handful in tests. Pick the more descriptive name and update the callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…_ctx_or_implicit` The empty error string passed to `expect_ident` was dead code (the preceding lookahead has already confirmed the token is an ident), but it would have been a confusing message if it ever fired. Replace with an explicit "unreachable" string that makes the intent clearer to readers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `{expr}.map(p -> tpl)` and `{expr}.reduce_left(first -> init, acc,
elem -> fold)` post-fix chains on `{expr}` placeholders had no
remaining users in the codebase: `.map` was never used, and the
4 `.reduce_left` sites in `swift.rs` were rewritten to plain
`Iterator::reduce` via an `and_chain` helper in an earlier commit.
Removes the entire `parse_chain_suffix` function (~90 lines) and the
`has_chain` detection / dispatch branches at the two call sites
(field-position in `parse_direct_node_inner` and body-position in
`parse_direct_list`). The remaining `{expr}` path is the
trait-dispatched one introduced by the splice-syntax cleanup, which
handles single ids and iterables uniformly via `IntoFieldIds`.
Also strips the chain syntax from the `tree!` macro doc comment.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>The trait had a single implementor (`AstCursor`), three type parameters of which one (`T`) was never used in any method signature, and one external consumer that needed `use yeast::Cursor;` in scope just to call methods on the cursor. The abstraction was overhead without a second implementor to justify it. Move the six trait methods to an inherent `impl AstCursor` block; delete `shared/yeast/src/cursor.rs`, the `pub mod cursor;` and `pub use cursor::Cursor;` lines in `lib.rs`, and the `use yeast::Cursor;` in `tree-sitter-extractor`'s `traverse_yeast`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a Yeast-wide cleanup that unifies node identity on a single Id newtype and simplifies macro/template interpolation by introducing IntoFieldIds, allowing {expr} to uniformly handle single ids, iterables, and optional captures without a separate splice syntax.
Changes:
- Replace
NodeRef/usize-based ids with a dedicatedyeast::Idnewtype and propagate it through AST building, traversal, dumping, and translation code. - Simplify
tree!/trees!/rule!template interpolation by removing{..expr}splice + chain support and usingIntoFieldIdsdispatch for{expr}. - Update Swift Yeast translation rules, extractor traversal, and Yeast unit tests/docs to the new id and interpolation APIs.
Show a summary per file
| File | Description |
|---|---|
| unified/extractor/src/languages/swift/swift.rs | Updates Swift translation rules to use Id and {expr}-based interpolation throughout. |
| shared/yeast/tests/test.rs | Migrates tests from kind() to kind_name() and from usize/NodeRef usage to Id. |
| shared/yeast/src/visitor.rs | Updates visitor-built AST ids to the Id newtype and fixes indexing accordingly. |
| shared/yeast/src/lib.rs | Introduces Id newtype + IntoFieldIds, removes NodeRef and Cursor trait, and updates core AST APIs. |
| shared/yeast/src/dump.rs | Updates dump APIs to take Id instead of usize and adjusts imports/signatures. |
| shared/yeast/src/cursor.rs | Removes the now-unneeded generic Cursor trait. |
| shared/yeast/src/captures.rs | Removes dead capture-mapping helpers and documents/keeps the one needed mapping API. |
| shared/yeast/src/build.rs | Removes dead prepend_field/translate_opt helpers and updates docs around translate. |
| shared/yeast/doc/yeast.md | Updates documentation examples/explanations for {expr} interpolation and raw captures. |
| shared/yeast-macros/src/parse.rs | Implements {expr} trait-dispatched extension via IntoFieldIds and removes splice/chain parsing logic. |
| shared/yeast-macros/src/lib.rs | Updates macro docs to describe the new {expr} interpolation model. |
| shared/tree-sitter-extractor/src/extractor/mod.rs | Updates Yeast node-kind access and removes reliance on the deleted Cursor trait import. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Low
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
A variety of cleanups and improvements, most notably unifying
NodeRefandIdinto the latter (first commit), which enables us to avoid having to explicitly specify whether we're splicing in a single tree or an iterable of such using the{..foo}syntax (the second commit).In addition to this, Copilot found a bunch of dead code and other simplifications, which resulted in the remaining commits on this PR.
Needless to say, you'll probably want to review this PR commit-by-commit.