From e9de618e85cb49d09f263a575b0673f4dc55dcaf Mon Sep 17 00:00:00 2001 From: Taus Date: Tue, 22 Sep 2026 15:27:07 +0000 Subject: [PATCH] yeast: Remove support for repeating phases These were never used in practice (only in tests), and we don't foresee an immediate use for them. If necessary, we can always add them back later. --- shared/yeast-macros/src/lib.rs | 6 +- shared/yeast-macros/src/parse.rs | 7 +- shared/yeast/doc/yeast.md | 70 ++--- shared/yeast/src/build.rs | 6 +- shared/yeast/src/lib.rs | 294 +++-------------- shared/yeast/tests/test.rs | 297 +++++++----------- .../extractor/src/languages/swift/swift.rs | 6 +- 7 files changed, 191 insertions(+), 495 deletions(-) diff --git a/shared/yeast-macros/src/lib.rs b/shared/yeast-macros/src/lib.rs index 568273f9b411..f5637542dc8c 100644 --- a/shared/yeast-macros/src/lib.rs +++ b/shared/yeast-macros/src/lib.rs @@ -164,8 +164,8 @@ pub fn rule(input: TokenStream) -> TokenStream { /// /// 1. A **bare rule body** `(query) => (template)` — the `rule!(...)` /// wrapper is implicit. -/// 2. An explicit `rule!(...)` invocation, possibly chained as -/// `rule!(...).repeated()` or path-prefixed as `yeast::rule!(...)`. +/// 2. An explicit `rule!(...)` invocation, possibly path-prefixed as +/// `yeast::rule!(...)`. /// 3. Any other expression returning a `Rule` (helper-function calls, /// conditionals). /// @@ -176,7 +176,7 @@ pub fn rule(input: TokenStream) -> TokenStream { /// [ /// (source_file (_)* @cs) => (top_level body: {..cs}), /// (simple_identifier) @id => (name_expr identifier: (identifier #{id})), -/// rule!((integer_literal) @lit => (int_literal #{lit})).repeated(), +/// rule!((integer_literal) @lit => (int_literal #{lit})), /// helper_fn(), /// ] /// }; diff --git a/shared/yeast-macros/src/parse.rs b/shared/yeast-macros/src/parse.rs index e0fd63fc4d68..80cca632d76c 100644 --- a/shared/yeast-macros/src/parse.rs +++ b/shared/yeast-macros/src/parse.rs @@ -970,9 +970,6 @@ pub fn parse_rule_top(input: TokenStream) -> Result { // captured node before invoking the user's transform body, // except for `@@name` captures listed in `__skip` which the // body consumes raw. - // For OneShot rules this preserves the legacy behaviour - // (input-schema captures translated to output-schema - // nodes); for Repeating rules it is a no-op. let __skip: &[&str] = &[#(#raw_capture_names),*]; __translator.auto_translate_captures(&mut __captures, __ast, __user_ctx, __skip)?; #(#raw_bindings)* @@ -1141,8 +1138,8 @@ fn expect_repetition(tokens: &mut Tokens) -> Result { /// Each item in the bracketed list can be: /// * a **bare rule body** `(query) => (template)` — wrapped implicitly /// in `yeast::rule! { ... }` for codegen; -/// * an explicit `rule!(...)` (or `rule!(...).repeated()`, -/// `yeast::rule!(...)`, etc.) — passed through verbatim; +/// * an explicit `rule!(...)` (including `yeast::rule!(...)`) — passed +/// through verbatim; /// * any other expression returning a `Rule` (helper-function calls, /// conditionals) — passed through verbatim. /// diff --git a/shared/yeast/doc/yeast.md b/shared/yeast/doc/yeast.md index aabc206ba131..9b7626f45fb4 100644 --- a/shared/yeast/doc/yeast.md +++ b/shared/yeast/doc/yeast.md @@ -53,29 +53,18 @@ A YEAST `Rule` has two parts: pattern language. 2. A **transform** that produces replacement nodes from the match captures. -The `Runner` applies rules by walking the tree top-down. At each node, it -tries each rule in order. If a rule's query matches, the node is replaced by -the transform's output, and the rules are re-applied to the result. If no -rule matches, the node is kept and its children are processed recursively. +The `Runner` translates the root with the first matching rule. Before the +rule's transform runs, captured input nodes are recursively translated with +the same rules. Every visited input node must match a rule; a missing match is +an error. A rule can replace one node with zero nodes (deletion), one node (rewriting), or multiple nodes (expansion). -By default a rule fires **at most once on a given node**: after firing, the -engine will not re-try that same rule on the result root. Other rules may -still fire on the result, and the rule may still fire on different nodes -(including the result's children). To opt into iterative behaviour — when a -rule's output is intentionally re-matched by the same rule — call -`.repeated()` on the constructed `Rule`: - -```rust -let r = yeast::rule!((foo ...) => (foo ...)).repeated(); -``` - -Without `.repeated()`, a rule whose output happens to match its own query -simply fires once and stops. With `.repeated()`, the rule is allowed to -re-match indefinitely; the runner still enforces a global rewrite-depth -limit (currently 100) as a safety net against accidental cycles. +A rule's output is final for the current phase: it is not matched or traversed +again. This allows rule queries to describe the input schema while transforms +build nodes from a different output schema. Use another named phase when the +output of one exhaustive translation must become the input to another. ## Query language @@ -429,10 +418,9 @@ rule!( ### Raw captures (`@@name`) -The default `@name` capture marker is *auto-translated*: in OneShot -phases the macro recursively translates the captured node before -binding it, so `{name}` in the output template splices a node that -already conforms to the output schema. +The default `@name` capture marker is *auto-translated*: the macro recursively +translates the captured node before binding it, so `{name}` in the output +template splices a node that already conforms to the output schema. For rules that need the raw (input-schema) capture — typically to read its source text or to translate it explicitly with mutable context @@ -456,9 +444,7 @@ yeast::rule!( ); ``` -Mix `@` and `@@` freely in the same rule. In a Repeating phase both -markers are equivalent (auto-translation is a no-op for repeating -rules). +Mix `@` and `@@` freely in the same rule. ## The `rule!` macro @@ -574,18 +560,17 @@ Prefer the simplest form that fits: ## Integration with the extractor -A YEAST desugaring pass is configured with a [`DesugaringConfig`], which -carries one or more named [`Phase`]s of rules and an optional output -node-types schema (in YAML format). Each phase is a complete traversal -that runs to completion before the next phase starts; only the current -phase's rules are considered during that traversal. Attach the config to -a language spec -to enable rewriting: +A YEAST translation pass is configured with a [`DesugaringConfig`], which +carries one or more named [`Phase`]s of exhaustive rules and an optional +output node-types schema (in YAML format). Each phase translates the previous +phase's root before the next phase starts; only the current phase's rules are +considered during that translation. Attach the config to a language spec to +enable rewriting: ```rust let desugar = yeast::DesugaringConfig::new() - .add_phase("cleanup", yeast::PhaseKind::Repeating, cleanup_rules()) - .add_phase("translate", yeast::PhaseKind::OneShot, translate_rules()) + .add_phase("normalize", normalization_rules()) + .add_phase("translate", translation_rules()) .with_output_node_types_yaml(include_str!("output-node-types.yml")); let lang = simple::LanguageSpec { @@ -600,14 +585,10 @@ let lang = simple::LanguageSpec { A single-phase config is just `.add_phase(...)` called once. Phase names appear in error messages so you can tell which phase failed. -There are two kinds of phases: -- **Repeating**: - Each node is re-processed until none of the rules in the phase matches. - When a node no longer matches any rules, its children are recursively processed. In practice this is used to desugar or simplify an AST, while staying mostly within the same schema. -- **One-shot**: - Each node is processed by the first matching rule, and the engine panics if no rule matches. - Rules are then recursively applied to every captured node. - In practice this is used when translating from one AST schema to another, where an exhaustive match is required. +Every phase uses one-shot translation: each visited input node is processed by +the first matching rule, captured nodes are recursively translated, and the +phase errors if no rule matches. Output nodes are not reprocessed in the same +phase. The same YAML node-types is used for both the runtime yeast `Schema` (so rules can refer to output-only kinds and fields) and TRAP validation (it @@ -645,8 +626,7 @@ let translation_rules: Vec = yeast::rules! { Each comma-separated item in the bracketed list may be: - A **bare rule body** `(query) => (template)` — no `rule!(...)` wrapper. -- An explicit `rule!(...)` invocation, with optional postfix calls such - as `rule!(...).repeated()`. +- An explicit `rule!(...)` invocation. - Any other expression returning a `Rule` (helper functions, etc.). Schema paths are resolved relative to the consuming crate's diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index 9b895eb837e9..f36922a155a3 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -253,10 +253,8 @@ impl<'a, C> BuildCtx<'a, C> { impl BuildCtx<'_, C> { /// Recursively translate every id in the given iterable via the - /// framework's rule machinery. In a OneShot phase, applies OneShot - /// rules to each id and returns the accumulated resulting node ids - /// in order. In a Repeating phase, errors (translation is not - /// meaningful when input and output share a schema). + /// framework's rule machinery and return the accumulated resulting node + /// ids in order. /// /// The single-`Id` case works too, because `Id: IntoIterator` is a singleton iterator — so `ctx.translate(some_id)?` diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index fd7cbccef831..2e8af818d7f8 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -596,11 +596,7 @@ impl Ast { self.nodes.get(id.0) } - fn source_range_ignoring_fields( - &self, - id: Id, - ignored_fields: &[&str], - ) -> Option { + fn source_range_ignoring_fields(&self, id: Id, ignored_fields: &[&str]) -> Option { let node = self.get_node(id)?; let source_range = node.source_range()?; let ignored_ranges = node @@ -986,12 +982,17 @@ impl From for NodeContent { /// directly) can call [`TranslatorHandle::translate`] selectively on /// specific node ids to control when translation happens. pub struct TranslatorHandle<'a, C> { - inner: TranslatorImpl<'a, C>, + index: &'a RuleIndex<'a, C>, + rewrite_depth: usize, + /// The id of the node the current rule is matching. Used by + /// [`auto_translate_captures`] to avoid infinite recursion when a + /// rule captures its own match root (e.g. via `(_) @_`). + matched_root: Id, } // Manual `Copy` / `Clone` so `TranslatorHandle<'_, C>: Copy` holds -// regardless of whether `C: Copy`. `TranslatorImpl` contains only -// shared references, which are `Copy` unconditionally. +// regardless of whether `C: Copy`. All fields are shared references or +// small `Copy` scalars. impl Copy for TranslatorHandle<'_, C> {} impl Clone for TranslatorHandle<'_, C> { fn clone(&self) -> Self { @@ -999,57 +1000,16 @@ impl Clone for TranslatorHandle<'_, C> { } } -/// Internal phase-specific translation state. Kept private — callers -/// interact with [`TranslatorHandle`] only. -enum TranslatorImpl<'a, C> { - /// OneShot phase translator: recursively applies OneShot rules. - OneShot { - index: &'a RuleIndex<'a, C>, - rewrite_depth: usize, - /// The id of the node the current rule is matching. Used by - /// [`auto_translate_captures`] to avoid infinite recursion when a - /// rule captures its own match root (e.g. via `(_) @_`). - matched_root: Id, - }, - /// Repeating phase translator: translation is not meaningful here - /// (input and output schemas are the same). [`translate`] errors; - /// [`auto_translate_captures`] is a no-op so the macro's auto-prefix - /// works unchanged for Repeating rules. - Repeating, -} - -// Manual `Copy` / `Clone` so `TranslatorImpl<'_, C>: Copy` holds -// regardless of whether `C: Copy`. All variants hold only shared -// references and small `Copy` scalars. -impl Copy for TranslatorImpl<'_, C> {} -impl Clone for TranslatorImpl<'_, C> { - fn clone(&self) -> Self { - *self - } -} - impl<'a, C: Clone> TranslatorHandle<'a, C> { - /// Recursively apply OneShot rules to `id` and return the resulting - /// node ids. Errors in a Repeating phase (where translation is not - /// meaningful). + /// Recursively apply the current phase's rules to `id` and return the + /// resulting node ids. pub fn translate(&self, ast: &mut Ast, user_ctx: &mut C, id: Id) -> Result, String> { - match &self.inner { - TranslatorImpl::OneShot { - index, - rewrite_depth, - .. - } => apply_one_shot_rules_inner(index, ast, user_ctx, id, rewrite_depth + 1), - TranslatorImpl::Repeating => { - Err("translate() is not available in a Repeating phase".into()) - } - } + apply_rules_inner(self.index, ast, user_ctx, id, self.rewrite_depth + 1) } - /// Translate every captured node in `captures` in place (OneShot phase - /// only), except for captures whose name appears in `skip` — those are - /// left as raw (input-schema) ids for the rule body to consume - /// directly. In a Repeating phase this is a no-op — Repeating rules - /// receive raw captures regardless of `skip`. + /// Translate every captured node in `captures` in place, except for + /// captures whose name appears in `skip` — those are left as raw + /// (input-schema) ids for the rule body to consume directly. /// /// Used by the `rule!` macro's generated prefix. `skip` is populated /// from the macro's `@@name` capture markers; for plain `@name` @@ -1064,19 +1024,13 @@ impl<'a, C: Clone> TranslatorHandle<'a, C> { user_ctx: &mut C, skip: &[&str], ) -> Result<(), String> { - match &self.inner { - TranslatorImpl::OneShot { matched_root, .. } => { - let root = *matched_root; - captures.try_map_captures_except(skip, |cid| { - if cid == root { - Ok(vec![cid]) - } else { - self.translate(ast, user_ctx, cid) - } - }) + captures.try_map_captures_except(skip, |cid| { + if cid == self.matched_root { + Ok(vec![cid]) + } else { + self.translate(ast, user_ctx, cid) } - TranslatorImpl::Repeating => Ok(()), - } + }) } } @@ -1090,8 +1044,8 @@ impl<'a, C: Clone> TranslatorHandle<'a, C> { /// /// Transforms produced by [`Rule::new`] receive **raw** captures and must /// translate them themselves (via the handle). Transforms produced by the -/// `rule!` macro have an auto-translation prefix injected for backward -/// compatibility. +/// `rule!` macro have an auto-translation prefix that recursively translates +/// captures. pub type Transform = Box< dyn Fn( &mut Ast, @@ -1116,11 +1070,6 @@ pub struct Rule { guard: Option>, transform: Transform, ignored_location_fields: Vec<&'static str>, - /// If true, after this rule fires on a node the engine will try to - /// re-apply this same rule on the result root. Defaults to false: - /// each rule fires at most once on a given node, which prevents - /// accidental loops where a rule's output matches its own query. - repeated: bool, } impl Rule { @@ -1131,7 +1080,6 @@ impl Rule { guard: None, transform, ignored_location_fields: Vec::new(), - repeated: false, } } @@ -1143,19 +1091,9 @@ impl Rule { guard: Some(guard), transform, ignored_location_fields: Vec::new(), - repeated: false, } } - /// Mark this rule as allowed to fire multiple times on the same node. - /// Use when the rule is intentionally iterative (its output may match - /// its own query). Without this, a rule fires at most once per node; - /// other rules can still fire on the result. - pub fn repeated(mut self) -> Self { - self.repeated = true; - self - } - fn set_ignored_location_fields(&mut self, fields: &[&'static str]) { self.ignored_location_fields = fields.to_vec(); } @@ -1233,130 +1171,20 @@ impl<'a, C> RuleIndex<'a, C> { } } -fn apply_repeating_rules( - rules: &[Rule], - ast: &mut Ast, - user_ctx: &mut C, - id: Id, -) -> Result, String> { - let index = RuleIndex::new(rules); - apply_repeating_rules_inner(&index, ast, user_ctx, id, 0, None) -} - -fn apply_repeating_rules_inner( - index: &RuleIndex, - ast: &mut Ast, - user_ctx: &mut C, - id: Id, - rewrite_depth: usize, - skip_rule: Option<*const Rule>, -) -> Result, String> { - if rewrite_depth > MAX_REWRITE_DEPTH { - return Err(format!( - "Desugaring exceeded maximum rewrite depth ({MAX_REWRITE_DEPTH}). \ - This likely indicates a non-terminating rule cycle." - )); - } - - let node_kind = ast.get_node(id).map(|n| n.kind_name()).unwrap_or(""); - for rule in index.rules_for_kind(node_kind) { - let rule_ptr = *rule as *const Rule; - if Some(rule_ptr) == skip_rule { - continue; - } - let Some(captures) = rule.match_query(ast, id)? else { - continue; - }; - - // Give each structurally-matching rule a private clone of the user - // context before its guard runs. Guard mutations are visible to the - // transform and recursive translation when the guard succeeds, but a - // failed guard drops the clone before trying the next rule. - let mut local = user_ctx.clone(); - if !rule.guard_matches(ast, &captures, &mut local)? { - continue; - } - - // Repeating rules don't need a real translator: their captures - // aren't auto-translated (Repeating preserves the input schema), - // and `ctx.translate(id)` errors if invoked from a Repeating - // transform. - let translator = TranslatorHandle { - inner: TranslatorImpl::Repeating, - }; - let result_nodes = rule.run_transform(ast, captures, id, &mut local, translator)?; - - // For non-repeated rules, suppress further application of *this* - // rule on the result root, so a rule whose output matches its own - // query doesn't loop. Other rules and child traversal are unaffected. - let next_skip = if rule.repeated { None } else { Some(rule_ptr) }; - let mut results = Vec::new(); - for node in result_nodes { - results.extend(apply_repeating_rules_inner( - index, - ast, - &mut local, - node, - rewrite_depth + 1, - next_skip, - )?); - } - return Ok(results); - } - - // Take the parent's fields by ownership: the recursion will rewrite - // each child Id, and we'll write the (possibly mutated) field map back - // when we're done. Avoids cloning the whole BTreeMap and its child - // Vecs on entry. Each child Vec is only re-allocated if a rewrite - // actually changes its contents. - // - // Child traversal does not increment rewrite depth and starts fresh - // (no rule is skipped on child subtrees). - let mut fields = std::mem::take(&mut ast.nodes[id.0].fields); - for children in fields.values_mut() { - let mut new_children: Option> = None; - for (i, &child_id) in children.iter().enumerate() { - let result = - apply_repeating_rules_inner(index, ast, user_ctx, child_id, rewrite_depth, None)?; - let unchanged = result.len() == 1 && result[0] == child_id; - match (&mut new_children, unchanged) { - (None, true) => {} // unchanged so far, no allocation needed - (None, false) => { - // First divergence — copy already-processed Ids and - // start collecting the rewritten sequence. - let mut new = Vec::with_capacity(children.len()); - new.extend_from_slice(&children[..i]); - new.extend(result); - new_children = Some(new); - } - (Some(new), _) => { - new.extend(result); - } - } - } - if let Some(new) = new_children { - *children = new; - } - } - ast.nodes[id.0].fields = fields; - Ok(vec![id]) -} - -/// Apply rules using `OneShot` semantics: the first matching rule fires on -/// each visited node, recursion proceeds only through captured nodes (not -/// through the input node's children directly), and an error is returned if -/// no rule matches a visited node. -fn apply_one_shot_rules( +/// Apply the first matching rule to each visited node. Recursion proceeds +/// only through captured nodes (not through the input node's children +/// directly), and an error is returned if no rule matches a visited node. +fn apply_rules( rules: &[Rule], ast: &mut Ast, user_ctx: &mut C, id: Id, ) -> Result, String> { let index = RuleIndex::new(rules); - apply_one_shot_rules_inner(&index, ast, user_ctx, id, 0) + apply_rules_inner(&index, ast, user_ctx, id, 0) } -fn apply_one_shot_rules_inner( +fn apply_rules_inner( index: &RuleIndex, ast: &mut Ast, user_ctx: &mut C, @@ -1388,56 +1216,34 @@ fn apply_one_shot_rules_inner( // Build the translator handle the transform will use to recursively // translate captures (or, for macro-generated rules, the - // auto-translate prefix uses it to translate every capture up front, - // preserving the legacy behavior). + // auto-translate prefix uses it to translate every capture up front). let translator = TranslatorHandle { - inner: TranslatorImpl::OneShot { - index, - rewrite_depth, - matched_root: id, - }, + index, + rewrite_depth, + matched_root: id, }; let result = rule.run_transform(ast, captures, id, &mut local, translator)?; return Ok(result); } - Err(format!( - "OneShot: no rule matched node of kind '{node_kind}'" - )) -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum PhaseKind { - /// A node is re-processed until none of the rules in the phase matches, - /// albeit a single rule cannot be applied twice in a row unless that rule is also marked as repeating. - /// When a node no longer matches any rules, its children are recursively processed (top down). - Repeating, - - /// A node is processed by the first matching rule, and the engine panics if no rule matches. - /// Rules are then recursively applied to every captured node. - /// In practice this is used when translating from one AST schema to another, where every node must be rewritten, - /// and it would be a type error to match the rule patterns (based on the input schema) against the output nodes (which conform to the output schema). - OneShot, + Err(format!("no rule matched node of kind '{node_kind}'")) } -/// One phase of a desugaring pass: a named bundle of rules that runs to -/// completion (a full traversal applying its rules) before the next phase -/// starts. Rules within a phase compete for matches as usual; rules in -/// different phases never compete because each traversal only considers the -/// current phase's rules. +/// One phase of a translation pass: a named bundle of exhaustive rules that +/// runs before the next phase starts. Rules within a phase compete for matches +/// as usual; rules in different phases never compete because each translation +/// only considers the current phase's rules. pub struct Phase { /// Name used in error messages. pub name: String, pub rules: Vec>, - pub kind: PhaseKind, } impl Phase { - pub fn new(name: impl Into, kind: PhaseKind, rules: Vec>) -> Self { + pub fn new(name: impl Into, rules: Vec>) -> Self { Self { name: name.into(), rules, - kind, } } } @@ -1455,8 +1261,8 @@ impl Phase { /// /// ```ignore /// let config = yeast::DesugaringConfig::new() -/// .add_phase("cleanup", PhaseKind::Repeating, cleanup_rules) -/// .add_phase("desugar", PhaseKind::Repeating, desugar_rules) +/// .add_phase("normalize", normalization_rules) +/// .add_phase("translate", translation_rules) /// .with_output_node_types_yaml(yaml); /// ``` /// @@ -1493,17 +1299,12 @@ impl DesugaringConfig { Self::default() } - /// Append a new phase with the given name, kind, and rules. - pub fn add_phase( - mut self, - name: impl Into, - kind: PhaseKind, - mut rules: Vec>, - ) -> Self { + /// Append a new phase with the given name and exhaustive rules. + pub fn add_phase(mut self, name: impl Into, mut rules: Vec>) -> Self { for rule in &mut rules { rule.set_ignored_location_fields(&self.ignored_location_fields); } - self.phases.push(Phase::new(name, kind, rules)); + self.phases.push(Phase::new(name, rules)); self } @@ -1660,11 +1461,8 @@ impl<'a, C: Clone> Runner<'a, C> { fn run_phases(&self, ast: &mut Ast, user_ctx: &mut C) -> Result<(), String> { let mut root = ast.get_root(); for phase in self.phases { - let res = match phase.kind { - PhaseKind::Repeating => apply_repeating_rules(&phase.rules, ast, user_ctx, root), - PhaseKind::OneShot => apply_one_shot_rules(&phase.rules, ast, user_ctx, root), - } - .map_err(|e| format!("Phase `{}`: {e}", phase.name))?; + let res = apply_rules(&phase.rules, ast, user_ctx, root) + .map_err(|e| format!("Phase `{}`: {e}", phase.name))?; if res.len() != 1 { return Err(format!( "Phase `{}`: expected exactly one result node, got {}", diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index f17e1c11f9be..a8f4144f9fce 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1,5 +1,7 @@ #![cfg(test)] +use std::collections::BTreeMap; + use yeast::dump::{dump_ast, dump_ast_with_type_errors}; use yeast::*; @@ -12,24 +14,79 @@ fn parse_and_dump(input: &str) -> String { dump_ast(&ast, ast.get_root(), input) } -/// Helper: parse Ruby source with a custom output schema and a single -/// phase of rules, return dump. +fn with_passthrough_rules(mut rules: Vec>) -> Vec> { + let program_rule = Rule::new( + yeast::query!((program (_)* @children)), + Box::new(|ast, captures, source_range, user_ctx, translator| { + let mut children = Vec::new(); + for child in captures.get_all("children") { + children.extend(translator.translate(ast, user_ctx, child)?); + } + let kind = ast + .id_for_node_kind("program") + .ok_or("program kind is not registered")?; + let fields = BTreeMap::from([(CHILD_FIELD, children)]); + let program = ast.create_node_with_range( + kind, + NodeContent::DynamicString(String::new()), + fields, + true, + source_range, + ); + Ok(vec![program]) + }), + ); + let left_assignment_list_rule: Rule = yeast::rule!( + (left_assignment_list (identifier)* @items) + => + (left_assignment_list item: {items}) + ); + let assignment_rule: Rule = yeast::rule!( + (assignment left: (_) @left right: (_) @right) + => + (assignment left: {left} right: {right}) + ); + let identifier_rule: Rule = yeast::rule!( + (identifier) @@node + => + identifier { node } + ); + let integer_rule: Rule = yeast::rule!( + (integer) @@node + => + integer { node } + ); + rules.extend([ + program_rule, + left_assignment_list_rule, + assignment_rule, + identifier_rule, + integer_rule, + ]); + rules +} + +/// Helper: translate Ruby source with a custom output schema and rules, then +/// return its dump. The appended pass-through rules make the small input +/// subset used by these tests exhaustive. fn run_and_dump(input: &str, rules: Vec) -> String { - run_phased_and_dump(input, vec![Phase::new("test", PhaseKind::Repeating, rules)]) + run_phased_and_dump( + input, + vec![Phase::new("test", with_passthrough_rules(rules))], + ) } -/// Helper: parse Ruby source with custom rules and return the transformed AST. +/// Helper: translate Ruby source with custom rules and return the transformed AST. fn run_and_ast(input: &str, rules: Vec) -> Ast { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); - let phases = vec![Phase::new("test", PhaseKind::Repeating, rules)]; + let phases = vec![Phase::new("test", with_passthrough_rules(rules))]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); runner.run(input).unwrap() } -/// Helper: parse Ruby source with a custom output schema and multiple -/// rule phases, return dump. +/// Helper: translate Ruby source with multiple rule phases and return its dump. fn run_phased_and_dump(input: &str, phases: Vec) -> String { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = @@ -39,19 +96,6 @@ fn run_phased_and_dump(input: &str, phases: Vec) -> String { dump_ast(&ast, ast.get_root(), input) } -/// Helper: like `run_and_dump`, but returns the runner error (if any) -/// instead of unwrapping. -fn run_and_get_error(input: &str, rules: Vec) -> String { - let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); - let schema = - yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); - let phases = vec![Phase::new("test", PhaseKind::Repeating, rules)]; - let runner: Runner = Runner::with_schema(lang, &schema, &phases); - runner - .run(input) - .expect_err("expected runner to return an error") -} - /// Helper: parse Ruby source with no rules and dump with schema type errors. fn parse_and_dump_typed(input: &str, schema_yaml: &str) -> String { let runner: Runner = Runner::new(tree_sitter_ruby::LANGUAGE.into(), &[]); @@ -71,14 +115,16 @@ fn parse_and_dump_typed_with_language(input: &str, schema_yaml: &str) -> String dump_ast_with_type_errors(&ast, ast.get_root(), input, &schema) } -/// Helper: parse Ruby source with custom rules and dump with schema type errors. +/// Helper: translate Ruby source with custom rules and dump with schema type errors. fn run_and_dump_typed(input: &str, rules: Vec, schema_yaml: &str) -> String { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); - let schema = yeast::node_types_yaml::schema_from_yaml(schema_yaml).unwrap(); - let phases = vec![Phase::new("test", PhaseKind::Repeating, rules)]; - let runner: Runner = Runner::with_schema(lang, &schema, &phases); + let runner_schema = + yeast::node_types_yaml::schema_from_yaml_with_language(schema_yaml, &lang).unwrap(); + let validation_schema = yeast::node_types_yaml::schema_from_yaml(schema_yaml).unwrap(); + let phases = vec![Phase::new("test", with_passthrough_rules(rules))]; + let runner: Runner = Runner::with_schema(lang, &runner_schema, &phases); let ast = runner.run(input).unwrap(); - dump_ast_with_type_errors(&ast, ast.get_root(), input, &schema) + dump_ast_with_type_errors(&ast, ast.get_root(), input, &validation_schema) } /// Assert that a dump equals the expected string, treating the expected @@ -268,8 +314,6 @@ fn test_query_match() { #[test] fn test_run_from_ast_desugars_hand_built_tree() { - use std::collections::BTreeMap; - // Output schema for the desugared tree. Its kind/field names must become // resolvable in the hand-built AST's schema for the rule to build them. let schema_yaml = r#" @@ -289,7 +333,7 @@ named: let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let config = DesugaringConfig::<()>::new() - .add_phase("test", PhaseKind::OneShot, rules) + .add_phase("test", rules) .with_output_node_types_yaml(schema_yaml); let desugarer = ConcreteDesugarer::new(lang, config).unwrap(); @@ -382,8 +426,7 @@ fn test_reachable_nodes_excludes_orphaned_rewrite_nodes() { yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); let phases: Vec = vec![Phase::new( "test", - PhaseKind::Repeating, - vec![yeast::rule!((integer) => (identifier "replaced"))], + with_passthrough_rules(vec![yeast::rule!((integer) => (identifier "replaced"))]), )]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); @@ -925,8 +968,7 @@ fn test_rule_guard_reads_raw_capture_and_user_context() { .unwrap(); let phases = vec![Phase::new( "test", - PhaseKind::Repeating, - guarded_integer_rules(), + with_passthrough_rules(guarded_integer_rules()), )]; let runner = Runner::with_schema(lang, &schema, &phases); let mut user_ctx = GuardTestContext { @@ -991,8 +1033,8 @@ fn test_rule_guard_binds_optional_and_repeated_raw_captures() { } #[test] -fn test_chained_rules_output_only_kind() { - // Exercise rule chaining where an intermediate kind exists only in the +fn test_multiple_translation_phases() { + // Exercise a second translation phase whose input kind exists only in the // output schema (not in the input tree-sitter grammar): // assignment → first_node (input → output-only) // first_node → second_node (output-only → output-only) @@ -1013,7 +1055,13 @@ fn test_chained_rules_output_only_kind() { => (second_node left: {left} right: {right}) ); - let dump = run_and_dump("x = 1", vec![assignment_to_first, first_to_second]); + let dump = run_phased_and_dump( + "x = 1", + vec![ + Phase::new("first", with_passthrough_rules(vec![assignment_to_first])), + Phase::new("second", with_passthrough_rules(vec![first_to_second])), + ], + ); assert_dump_eq( &dump, r#" @@ -1025,9 +1073,6 @@ fn test_chained_rules_output_only_kind() { ); } -// A rule that swaps `assignment.left` and `assignment.right`. Each -// application produces another `assignment` whose query the rule -// matches again, so without the once-per-node default it would loop. fn swap_assignment_rule() -> Rule { yeast::rule!( (assignment @@ -1043,21 +1088,9 @@ fn swap_assignment_rule() -> Rule { } #[test] -fn test_repeated_rule_hits_depth_limit() { - // With `.repeated()` the rule is allowed to fire on its own output, - // which cycles forever and trips the rewrite-depth safety net. - let err = run_and_get_error("x = 1", vec![swap_assignment_rule().repeated()]); - assert!( - err.contains("exceeded maximum rewrite depth"), - "expected depth-limit error, got: {err}" - ); -} - -#[test] -fn test_default_rule_fires_at_most_once_per_node() { - // Without `.repeated()` (the default), a rule fires at most once on a - // given node. The swap therefore happens exactly once and the desugaring - // terminates cleanly. +fn test_rule_output_is_not_reprocessed() { + // Translation applies a rule once to an input node and does not match + // rules against the output node. let dump = run_and_dump("x = 1", vec![swap_assignment_rule()]); assert_dump_eq( &dump, @@ -1070,75 +1103,9 @@ fn test_default_rule_fires_at_most_once_per_node() { ); } -// ---- Phase tests ---- - -#[test] -fn test_phased_desugaring() { - // Two phases that could equally have been a single one with chained - // rules. Splitting them makes the intent (cleanup, then desugar) - // explicit and provides per-phase error messages. - let cleanup: Vec = vec![yeast::rule!( - (assignment - left: (_) @left - right: (_) @right - ) - => (first_node left: {left} right: {right}) - )]; - let desugar: Vec = vec![yeast::rule!( - (first_node - left: (_) @left - right: (_) @right - ) - => (second_node left: {left} right: {right}) - )]; - - let dump = run_phased_and_dump( - "x = 1", - vec![ - Phase::new("cleanup", PhaseKind::Repeating, cleanup), - Phase::new("desugar", PhaseKind::Repeating, desugar), - ], - ); - assert_dump_eq( - &dump, - r#" - program - second_node - left: identifier "x" - right: integer "1" - "#, - ); -} - -#[test] -fn test_phase_error_includes_phase_name() { - // A repeated rule that loops; the error message should identify the - // phase that tripped the depth limit. - let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); - let schema = - yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); - let phases = vec![Phase::new( - "buggy", - PhaseKind::Repeating, - vec![swap_assignment_rule().repeated()], - )]; - let runner: Runner = Runner::with_schema(lang, &schema, &phases); - let err = runner - .run("x = 1") - .expect_err("expected runner to return an error"); - assert!( - err.contains("Phase `buggy`"), - "error should mention the failing phase, got: {err}" - ); - assert!( - err.contains("exceeded maximum rewrite depth"), - "error should mention the depth limit, got: {err}" - ); -} - -/// Helper: an exhaustive set of OneShot rules covering every node reachable -/// (via captures) when translating `"x = 1"`. -fn one_shot_xeq1_rules() -> Vec { +/// Helper: an exhaustive set of rules covering every node reachable via +/// captures when translating `"x = 1"`. +fn translation_xeq1_rules() -> Vec { vec![ yeast::rule!( (program (_)* @stmts) @@ -1156,15 +1123,11 @@ fn one_shot_xeq1_rules() -> Vec { } #[test] -fn test_one_shot_phase() { +fn test_translation_phase() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); - let phases = vec![Phase::new( - "translate", - PhaseKind::OneShot, - one_shot_xeq1_rules(), - )]; + let phases = vec![Phase::new("translate", translation_xeq1_rules())]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; @@ -1183,19 +1146,19 @@ fn test_one_shot_phase() { } #[test] -fn test_one_shot_phase_errors_when_no_rule_matches() { +fn test_translation_phase_errors_when_no_rule_matches() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); // Drop the `integer` rule so the recursion has no rule for `integer`. - let mut rules = one_shot_xeq1_rules(); + let mut rules = translation_xeq1_rules(); rules.pop(); - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let err = runner .run("x = 1") - .expect_err("expected OneShot to error on unmatched node"); + .expect_err("expected translation to error on unmatched node"); assert!( err.contains("Phase `translate`"), "error should name the phase, got: {err}" @@ -1207,7 +1170,7 @@ fn test_one_shot_phase_errors_when_no_rule_matches() { } #[test] -fn test_one_shot_guard_runs_before_capture_translation() { +fn test_guard_runs_before_capture_translation() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); @@ -1217,7 +1180,7 @@ fn test_one_shot_guard_runs_before_capture_translation() { => (program stmt: {stmts}) ), - // There is deliberately no OneShot rule for `identifier`. If this + // There is deliberately no rule for `identifier`. If this // rule translated `@left` before binding it raw in the guard, the run // would fail instead of evaluating the guard and falling through to // the next assignment rule. @@ -1229,7 +1192,7 @@ fn test_one_shot_guard_runs_before_capture_translation() { ), yeast::rule!((assignment) => (identifier "fallback")), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; @@ -1245,7 +1208,7 @@ fn test_one_shot_guard_runs_before_capture_translation() { } #[test] -fn test_one_shot_guard_context_mutation_is_visible_to_transform() { +fn test_guard_context_mutation_is_visible_to_transform() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); @@ -1268,7 +1231,7 @@ fn test_one_shot_guard_context_mutation_is_visible_to_transform() { } ), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner = Runner::with_schema(lang, &schema, &phases); let mut user_ctx = GuardTestContext::default(); @@ -1284,12 +1247,12 @@ fn test_one_shot_guard_context_mutation_is_visible_to_transform() { ); } -/// OneShot recursion must apply rules to *captured* nodes, even if the rule +/// Translation must apply rules to *captured* nodes, even if the rule /// returns a captured child verbatim. A buggy implementation that only /// recurses into the children of the rule's output (rather than into the /// captures) would leave the returned capture untransformed. #[test] -fn test_one_shot_recurses_into_returned_capture() { +fn test_translation_recurses_into_returned_capture() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); @@ -1308,13 +1271,13 @@ fn test_one_shot_recurses_into_returned_capture() { yeast::rule!((identifier) => (identifier "ID")), yeast::rule!((integer) => (integer "INT")), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; let ast = runner.run(input).unwrap(); let dump = dump_ast(&ast, ast.get_root(), input); - // `left` is an `identifier`; OneShot must apply the identifier rule to + // `left` is an `identifier`; translation must apply the identifier rule to // it before the assignment transform returns it verbatim. assert_dump_eq( &dump, @@ -1325,13 +1288,13 @@ fn test_one_shot_recurses_into_returned_capture() { ); } -/// OneShot recursion must NOT descend into the children of the rule's output. +/// Translation must NOT descend into the children of the rule's output. /// A rule may legitimately wrap a captured node in fresh output-schema nodes /// that have no matching rule of their own (since rule patterns target the /// input schema). Recursing into the output would erroneously try to find /// rules for those wrapper kinds and fail. #[test] -fn test_one_shot_does_not_recurse_into_wrapper_output() { +fn test_translation_does_not_recurse_into_wrapper_output() { let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let schema = yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); @@ -1355,7 +1318,7 @@ fn test_one_shot_does_not_recurse_into_wrapper_output() { yeast::rule!((identifier) => (identifier "ID")), yeast::rule!((integer) => (integer "INT")), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; @@ -1408,7 +1371,7 @@ fn test_raw_capture_marker() { yeast::rule!((identifier) => (identifier "ID")), yeast::rule!((integer) => (integer "INT")), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; @@ -1463,7 +1426,7 @@ fn test_raw_capture_marker_explicit_translate() { yeast::rule!((identifier) => (identifier "ID")), yeast::rule!((integer) => (integer "INT")), ]; - let phases = vec![Phase::new("translate", PhaseKind::OneShot, rules)]; + let phases = vec![Phase::new("translate", rules)]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); let input = "x = 1"; @@ -1514,44 +1477,6 @@ fn test_cursor_navigation() { assert!(!cursor.goto_parent()); } -#[test] -fn test_desugar_for_with_multiple_assignment() { - let dump = run_and_dump("for a, b in list do\n x\nend", ruby_rules()); - assert_dump_eq( - &dump, - r#" - program - call - block: - block - body: - block_body - stmt: - assignment - left: identifier "assignment_tmp" - right: identifier "loop_tmp" - assignment - left: identifier "a" - right: - element_reference - object: identifier "assignment_tmp" - index: integer "0" - assignment - left: identifier "b" - right: - element_reference - object: identifier "assignment_tmp" - index: integer "1" - identifier "x" - parameters: - block_parameters - parameter: identifier "loop_tmp" - method: identifier "each" - receiver: identifier "list" - "#, - ); -} - /// Regression test: `#{capture}` in a template must render the *source text* /// of the captured node, not its arena `Id`. Captures are bound as `Id`, /// whose `YeastDisplay` impl resolves to the captured node's source text. @@ -1795,7 +1720,8 @@ fn test_ignored_location_field_is_excluded_from_rule_result_location() { let language: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into(); let config = DesugaringConfig::new() .with_ignored_location_fields(["right"]) - .add_phase("test", PhaseKind::Repeating, vec![rule]); + .add_phase("test", with_passthrough_rules(vec![rule])) + .with_output_node_types_yaml(OUTPUT_SCHEMA_YAML); let runner: Runner = Runner::from_config(language, &config).unwrap(); let ast = runner.run("x = 1").unwrap(); let call = ast @@ -1833,7 +1759,6 @@ fn test_explicit_recursive_translation_keeps_nested_rule_location() { yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap(); let phases = vec![Phase::new( "translate", - PhaseKind::OneShot, vec![program, unwrap, translate_identifier], )]; let runner: Runner = Runner::with_schema(lang, &schema, &phases); diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index c6624d6e2493..9fe9abd4b323 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -1,7 +1,5 @@ use codeql_extractor::extractor::desugaring; -use yeast::{ - ConcreteDesugarer, DesugaringConfig, PhaseKind, Rule, rule, tree, tree_at, tree_spanning, -}; +use yeast::{ConcreteDesugarer, DesugaringConfig, Rule, rule, tree, tree_at, tree_spanning}; /// User context propagated from outer rules down to the inner rules that /// emit the corresponding output declarations, so that each emitted node @@ -1433,7 +1431,7 @@ fn translation_rules() -> Vec> { pub fn language_spec(desugared_ast_schema: &'static str) -> desugaring::LanguageSpec { let config = DesugaringConfig::::new() .with_ignored_location_fields(["trailingComma"]) - .add_phase("translate", PhaseKind::OneShot, translation_rules()) + .add_phase("translate", translation_rules()) .with_output_node_types_yaml(desugared_ast_schema); let desugarer = ConcreteDesugarer::without_language(config).expect("failed to build Swift desugarer");