From 578d4778a4fb437080f58120516a90db25cc1a7b Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 16 Oct 2026 13:50:41 +0000 Subject: [PATCH 1/4] yeast: Add special `location` field When specified on a tree template node, this field is used to override/set the location of the node it is attached to. The field itself can contain an iterable of nodes, in which case the location will be the smallest location spanning all of these nodes. The same behaviour applies if there are multiple `location` fields on the same node. --- shared/yeast-macros/src/ast.rs | 7 +- shared/yeast-macros/src/lib.rs | 2 + shared/yeast-macros/src/lower.rs | 39 ++++++++++- shared/yeast-macros/src/template_parse.rs | 79 +++++++++++++++++++++-- shared/yeast/doc/yeast.md | 26 ++++++++ shared/yeast/tests/test.rs | 69 ++++++++++++++++++++ 6 files changed, 214 insertions(+), 8 deletions(-) diff --git a/shared/yeast-macros/src/ast.rs b/shared/yeast-macros/src/ast.rs index 0901bcc2aed5..f400257fd1d4 100644 --- a/shared/yeast-macros/src/ast.rs +++ b/shared/yeast-macros/src/ast.rs @@ -108,7 +108,7 @@ pub(crate) struct Node { /// Optional leaf content such as `"foo"` or `#{name}` in /// `(identifier "foo")` or `(identifier #{name})`. pub(crate) content: Option, - /// Named fields such as `method: {name}` in `(call method: {name})`. + /// Named fields and source-location metadata. pub(crate) fields: Vec, } @@ -131,4 +131,9 @@ pub(crate) enum Field { /// Whether the field value has a trailing `?`. optional: bool, }, + /// Source-location metadata such as `location: {source}`. + Location { + /// A Rust block yielding the node or nodes whose locations should be combined. + block: syn::Block, + }, } diff --git a/shared/yeast-macros/src/lib.rs b/shared/yeast-macros/src/lib.rs index 02bd4af25466..c63807514703 100644 --- a/shared/yeast-macros/src/lib.rs +++ b/shared/yeast-macros/src/lib.rs @@ -65,6 +65,8 @@ pub fn query(input: TokenStream) -> TokenStream { /// field: {expr} - extend a named field with `{expr}`'s ids /// field: (kind ...)? - set the field only if every `#{expr}` /// beneath it has a value (see below) +/// location: {expr} - override this node's source range with the +/// span of one or more nodes; may be repeated /// ``` /// /// # Optional fields diff --git a/shared/yeast-macros/src/lower.rs b/shared/yeast-macros/src/lower.rs index b02f69e4dd09..5e4abbd8bacf 100644 --- a/shared/yeast-macros/src/lower.rs +++ b/shared/yeast-macros/src/lower.rs @@ -320,7 +320,7 @@ impl Node { let kind_str = self.kind.to_string(); if let Some(content) = &self.content { - return match content { + let node = match content { Content::Static(lit) => quote! { #ctx.literal(#kind_str, #lit) }, Content::Computed(block) => { let block = rust_block(block); @@ -362,6 +362,7 @@ impl Node { } } }; + return self.with_location(node, &ctx); } let mut stmts = Vec::new(); @@ -372,7 +373,10 @@ impl Node { name, value, optional, - } = field; + } = field + else { + continue; + }; let field_name = name.to_string(); let field_str = field_name .strip_prefix("r#") @@ -424,13 +428,42 @@ impl Node { } } - quote! { + let node = quote! { { #(#stmts)* let mut __fields: Vec<(&str, Vec)> = Vec::new(); #(#field_args)* #ctx.node(#kind_str, __fields) } + }; + self.with_location(node, &ctx) + } + + fn with_location(&self, node: TokenStream, ctx: &Ident) -> TokenStream { + let locations = self.fields.iter().filter_map(|field| match field { + Field::Named { .. } => None, + Field::Location { block } => Some(rust_block(block)), + }); + let locations: Vec<_> = locations.collect(); + if locations.is_empty() { + return node; + } + + quote! { + { + let mut __location_ids: Vec = Vec::new(); + #(yeast::IntoFieldIds::extend_into(#locations, &mut __location_ids);)* + let __source_range = __location_ids + .into_iter() + .filter_map(|__source| { + #ctx.ast + .get_node(__source) + .and_then(|__node| __node.source_range()) + }) + .reduce(yeast::Range::union); + let __node: yeast::Id = #node; + #ctx.set_node_source_range(__node, __source_range) + } } } } diff --git a/shared/yeast-macros/src/template_parse.rs b/shared/yeast-macros/src/template_parse.rs index a01c00ab67d3..782407466a46 100644 --- a/shared/yeast-macros/src/template_parse.rs +++ b/shared/yeast-macros/src/template_parse.rs @@ -70,13 +70,41 @@ fn parse_node(group: proc_macro2::Group) -> Result { let mut fields = Vec::new(); while peek_is_field(&mut tokens) { let name = expect_ident(&mut tokens, "expected field name")?; - if content.is_some() { + let is_location = name == "location"; + if content.is_some() && !is_location { return Err(syn::Error::new_spanned( name, - "literal nodes cannot have named fields", + "literal nodes can only have the special `location` field", )); } expect_punct(&mut tokens, ':', "expected `:` after field name")?; + if is_location { + if !peek_is_group(&mut tokens, Delimiter::Brace) { + let error = match tokens.peek() { + Some(token) => syn::Error::new_spanned( + token.clone(), + "`location` field value must be a Rust block", + ), + None => syn::Error::new( + Span::call_site(), + "`location` field value must be a Rust block", + ), + }; + return Err(error); + } + let block = parse_block(expect_group(&mut tokens, Delimiter::Brace)?)?; + if let Some(TokenTree::Punct(punctuation)) = tokens.peek() { + if punctuation.as_char() == '?' { + return Err(syn::Error::new_spanned( + punctuation.clone(), + "`?` is not valid on a `location` field", + )); + } + } + fields.push(Field::Location { block }); + continue; + } + let value = parse_one(&mut tokens)?; let optional = if matches!(value, Template::Splice(_)) { reject_stray_optional(&mut tokens, true)?; @@ -240,6 +268,46 @@ mod tests { assert_eq!(node.fields.len(), 2); } + #[test] + fn parses_location_fields_on_literal_nodes() { + let template = parse_template(quote! { + (identifier "name" location: {source}) + }) + .unwrap(); + + let Template::Node(node) = template else { + panic!("expected node template"); + }; + assert_eq!(node.kind, "identifier"); + assert!(node.content.is_some()); + assert!(matches!(node.fields.as_slice(), [Field::Location { .. }])); + } + + #[test] + fn rejects_non_block_location_fields() { + let result = parse_template(quote! { + (identifier "name" location: (identifier "source")) + }); + let Err(error) = result else { + panic!("expected a non-block location value to be rejected"); + }; + assert_eq!( + error.to_string(), + "`location` field value must be a Rust block" + ); + } + + #[test] + fn rejects_optional_location_fields() { + let result = parse_template(quote! { + (identifier "name" location: {source}?) + }); + let Err(error) = result else { + panic!("expected an optional location field to be rejected"); + }; + assert_eq!(error.to_string(), "`?` is not valid on a `location` field"); + } + #[test] fn parses_lists_and_skips_empty_nodes() { let templates = parse_templates(quote! { @@ -254,13 +322,16 @@ mod tests { } #[test] - fn rejects_fields_on_literal_nodes() { + fn rejects_named_fields_on_literal_nodes() { let result = parse_template(quote! { (identifier "name" child: (identifier "nested")) }); let Err(error) = result else { panic!("expected literal node fields to be rejected"); }; - assert_eq!(error.to_string(), "literal nodes cannot have named fields"); + assert_eq!( + error.to_string(), + "literal nodes can only have the special `location` field" + ); } } diff --git a/shared/yeast/doc/yeast.md b/shared/yeast/doc/yeast.md index 9819cec7fbbc..9609ad6e322c 100644 --- a/shared/yeast/doc/yeast.md +++ b/shared/yeast/doc/yeast.md @@ -271,6 +271,32 @@ same broad range to every synthetic descendant. A transform that simply returns a translated capture does not widen that capture to the wrapper's range. +The special `location` field overrides the location of the node on which it +appears. Its value must be a Rust block and uses the same node-splicing rules +as an ordinary field block, but is metadata and is not added to the output AST: + +```rust +rule!( + (wrapper + first: (_) @first + second: (_) @second + child: (_) @child) + => + (synthetic_node + child: {child} + location: {[first, second]}) +) +``` + +If a `location` value yields several nodes, their locations are combined into +the smallest range containing all of them. Multiple `location` fields on the +same node are combined in the same way. The field can also be used on literal +leaf nodes: + +```rust +(identifier "synthetic" location: {source_node}) +``` + The following macros can be used to explicitly set the location associated with a newly-created node. They assign a location only to the root of their template; nested nodes still derive their locations normally. diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index 099f5ebe7eb9..a238e0b9f76f 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1708,6 +1708,75 @@ fn test_returned_capture_keeps_its_location() { assert_eq!(identifier.byte_range(), 4..7); } +#[test] +fn test_location_field_overrides_derived_location() { + let rule: Rule = rule!( + (assignment + left: (identifier) @left + right: (integer) @right) + => + (call + method: {left} + location: {right}) + ); + + let ast = run_and_ast("x = 1", vec![rule]); + let call = ast + .reachable_node_ids() + .into_iter() + .filter_map(|id| ast.get_node(id)) + .find(|node| node.kind_name() == "call") + .expect("call exists"); + + assert_eq!(call.byte_range(), 4..5); +} + +#[test] +fn test_multiple_location_fields_combine_iterable_locations() { + let rule: Rule = rule!( + (assignment + left: (identifier) @left + right: (integer) @right) + => + (call + method: {left} + arguments: (argument_list + location: {[left]} + location: {[right, left]})) + ); + + let ast = run_and_ast("x = 1", vec![rule]); + let arguments = ast + .reachable_node_ids() + .into_iter() + .filter_map(|id| ast.get_node(id)) + .find(|node| node.kind_name() == "argument_list") + .expect("argument list exists"); + + assert_eq!(arguments.byte_range(), 0..5); +} + +#[test] +fn test_location_field_is_available_on_literal_nodes() { + let rule: Rule = rule!( + (assignment + left: (identifier) @left + right: (integer) @right) + => + (identifier "synthetic" location: {right}) + ); + + let ast = run_and_ast("x = 1", vec![rule]); + let identifier = ast + .reachable_node_ids() + .into_iter() + .filter_map(|id| ast.get_node(id)) + .find(|node| node.kind_name() == "identifier") + .expect("identifier exists"); + + assert_eq!(identifier.byte_range(), 4..5); +} + #[test] fn test_ignored_location_field_is_excluded_from_rule_result_location() { let rule: Rule = rule!( From 2ed43aef0271fecb3ee11868fde06342565dfc0d Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 16 Oct 2026 13:51:04 +0000 Subject: [PATCH 2/4] Unified: Rewrite swift.rs to use nicer location syntax --- .../extractor/src/languages/swift/swift.rs | 78 +++++++++---------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index f8dec4deb032..5ffa912f4c1e 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -1,5 +1,5 @@ use codeql_extractor::extractor::desugaring; -use yeast::{ConcreteDesugarer, DesugaringConfig, Rule, rule, tree, tree_at, tree_spanning}; +use yeast::{ConcreteDesugarer, DesugaringConfig, Rule, rule, tree}; /// User context propagated from outer rules down to the inner rules that /// emit the corresponding output declarations, so that each emitted node @@ -421,16 +421,15 @@ fn translation_rules() -> Vec> { } None => None, }; - tree_spanning!( - std::iter::once(spec).chain(body), - (accessor_declaration - modifier: {binding} - modifier: {chained} - name_node: {name} - type: {ty} - accessor_kind: (accessor_kind #{spec}) - body: {body}) - ) + tree!((accessor_declaration + modifier: {binding} + modifier: {chained} + name_node: {name} + type: {ty} + accessor_kind: (accessor_kind #{spec}) + body: {body} + location: {spec} + location: {body})) } ), // ---- Variables ---- @@ -500,10 +499,11 @@ fn translation_rules() -> Vec> { parameterClause: (enumCaseParameterClause parameters: _* @params) @@clause) => class_like_declaration { - let constructor = tree_spanning!( - [name, clause], - (constructor_declaration parameter: {params} body: (block)) - ); + let constructor = tree!((constructor_declaration + parameter: {params} + body: (block) + location: {name} + location: {clause})); tree!((class_like_declaration modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} @@ -675,12 +675,10 @@ fn translation_rules() -> Vec> { additionalTrailingClosures: _* @additional) => call_expr { - let callee = tree_at!( - array, - (generic_type_expr + let callee = tree!((generic_type_expr base: (identifier "Array") - type_argument: {element}) - ); + type_argument: {element} + location: {array})); tree!((call_expr callee: {callee} argument: {args} @@ -695,12 +693,10 @@ fn translation_rules() -> Vec> { arguments: _* @args) => call_expr { - let callee = tree_at!( - array, - (generic_type_expr + let callee = tree!((generic_type_expr base: (identifier "Array") - type_argument: {element}) - ); + type_argument: {element} + location: {array})); tree!((call_expr callee: {callee} argument: {args})) } ), @@ -772,12 +768,10 @@ fn translation_rules() -> Vec> { declName: (declReferenceExpr baseName: @@member)) => member_access_expr { - let base = tree_at!( - array, - (generic_type_expr - base: (identifier "Array") - type_argument: {element}) - ); + let base = tree!((generic_type_expr + base: (identifier "Array") + type_argument: {element} + location: {array})); tree!((member_access_expr base: {base} member_name_node: (identifier #{member}))) @@ -1065,10 +1059,9 @@ fn translation_rules() -> Vec> { => expr { let op = format!("try{}", m.map(|m| ctx.source_text(m)).unwrap_or_default()); - let operator = tree_spanning!( - std::iter::once(keyword).chain(m), - (prefix_operator #{op}) - ); + let operator = tree!((prefix_operator #{op} + location: {keyword} + location: {m})); tree!((unary_expr operator: {operator} operand: {e})) } ), @@ -1107,25 +1100,24 @@ fn translation_rules() -> Vec> { // As expression (type cast) — as?, as! rule!((asExpr expression: @val asKeyword: @@keyword questionOrExclamationMark: _? @@mark type: @ty) => type_cast_expr { let op = format!("as{}", mark.map(|m| ctx.source_text(m)).unwrap_or_default()); - let operator = tree_spanning!( - std::iter::once(keyword).chain(mark), - (infix_operator #{op}) - ); + let operator = tree!((infix_operator #{op} + location: {keyword} + location: {mark})); tree!((type_cast_expr expr: {val} operator: {operator} type: {ty})) }), // Check expression (`x is T`) → type_test_expr rule!((isExpr expression: @val isKeyword: @@keyword type: @ty) => (type_test_expr expr: {val} - operator: {tree_at!(keyword, (infix_operator "is"))} + operator: (infix_operator "is" location: {keyword}) type: {ty})), // Await expression → unary_expr with operator "await" rule!((awaitExpr awaitKeyword: @@keyword expression: @val) => (unary_expr - operator: {tree_at!(keyword, (prefix_operator "await"))} + operator: (prefix_operator "await" location: {keyword}) operand: {val})), // Force-unwrap (`x!`) → postfix unary_expr, via swift-syntax's dedicated // `forceUnwrapExpr` node. rule!((forceUnwrapExpr expression: @e exclamationMark: @@mark) => (unary_expr - operator: {tree_at!(mark, (postfix_operator "!"))} + operator: (postfix_operator "!" location: {mark}) operand: {e})), // ---- Imports ---- // An import declaration. The dotted path (a list of @@ -1147,7 +1139,7 @@ fn translation_rules() -> Vec> { let last = *parts.last().ok_or("import has no path")?; let pattern = match kind { None => { - let bulk = tree_at!(decl, (bulk_importing_pattern)); + let bulk = tree!((bulk_importing_pattern location: {decl})); tree!((named_pattern name_node: (identifier #{last}) sub_pattern: {bulk})) From 70fb3a643b81c4dbb717ddfbf660029a6c95b645 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 16 Oct 2026 13:57:29 +0000 Subject: [PATCH 3/4] yeast: Remove now-redundant `tree_at!` and `tree_spanning!` macros --- shared/yeast-macros/src/lib.rs | 32 ----------------- shared/yeast-macros/src/parse.rs | 59 -------------------------------- shared/yeast/doc/yeast.md | 41 ++-------------------- shared/yeast/src/lib.rs | 2 +- shared/yeast/tests/test.rs | 21 ++++++------ 5 files changed, 14 insertions(+), 141 deletions(-) diff --git a/shared/yeast-macros/src/lib.rs b/shared/yeast-macros/src/lib.rs index c63807514703..f3cb6e652b02 100644 --- a/shared/yeast-macros/src/lib.rs +++ b/shared/yeast-macros/src/lib.rs @@ -127,38 +127,6 @@ pub fn trees(input: TokenStream) -> TokenStream { } } -/// Build one AST node whose root uses another node's source range. -/// -/// Uses the `BuildCtx` binding named `ctx` from the surrounding scope: -/// -/// ```text -/// tree_at!(source, (kind ...)) -/// ``` -#[proc_macro] -pub fn tree_at(input: TokenStream) -> TokenStream { - let input2: TokenStream2 = input.into(); - match parse::parse_tree_at_top(input2) { - Ok(output) => output.into(), - Err(err) => err.to_compile_error().into(), - } -} - -/// Build one AST node whose root spans a collection of nodes. -/// -/// Uses the `BuildCtx` binding named `ctx` from the surrounding scope: -/// -/// ```text -/// tree_spanning!(sources, (kind ...)) -/// ``` -#[proc_macro] -pub fn tree_spanning(input: TokenStream) -> TokenStream { - let input2: TokenStream2 = input.into(); - match parse::parse_tree_spanning_top(input2) { - Ok(output) => output.into(), - Err(err) => err.to_compile_error().into(), - } -} - /// Define a desugaring rule with query and transform in one declaration. /// /// ```text diff --git a/shared/yeast-macros/src/parse.rs b/shared/yeast-macros/src/parse.rs index 796f3c029ad3..becdd56831f7 100644 --- a/shared/yeast-macros/src/parse.rs +++ b/shared/yeast-macros/src/parse.rs @@ -1,9 +1,5 @@ use proc_macro2::{Span, TokenStream, TokenTree}; use quote::quote; -use syn::{ - parse::{Parse, ParseStream}, - Expr, Token, -}; type Result = std::result::Result; @@ -28,47 +24,6 @@ pub fn parse_trees_top(input: TokenStream) -> Result { Ok(crate::ast::Template::lower_list(&templates)) } -pub fn parse_tree_at_top(input: TokenStream) -> Result { - let LocatedTreeInput { source, template } = syn::parse2(input)?; - let template = crate::template_parse::parse_template(template)?; - let node = template.lower_root(); - let ctx = syn::Ident::new("ctx", Span::call_site()); - - Ok(quote! { - { - let __yeast_source: yeast::Id = { #source }; - let __yeast_source_range = #ctx - .ast - .get_node(__yeast_source) - .and_then(|node| node.source_range()); - let __yeast_node: yeast::Id = #node; - #ctx.set_node_source_range(__yeast_node, __yeast_source_range) - } - }) -} - -pub fn parse_tree_spanning_top(input: TokenStream) -> Result { - let LocatedTreeInput { - source: sources, - template, - } = syn::parse2(input)?; - let template = crate::template_parse::parse_template(template)?; - let node = template.lower_root(); - let ctx = syn::Ident::new("ctx", Span::call_site()); - - Ok(quote! { - { - let __yeast_source_range = ::std::iter::IntoIterator::into_iter({ #sources }) - .filter_map(|source: yeast::Id| { - #ctx.ast.get_node(source).and_then(|node| node.source_range()) - }) - .reduce(yeast::Range::union); - let __yeast_node: yeast::Id = #node; - #ctx.set_node_source_range(__yeast_node, __yeast_source_range) - } - }) -} - pub fn parse_rule_top(input: TokenStream) -> Result { Ok(crate::rule_parse::parse_rule(input)?.lower()) } @@ -127,20 +82,6 @@ pub fn parse_rules_top(input: TokenStream) -> Result { }) } -struct LocatedTreeInput { - source: Expr, - template: TokenStream, -} - -impl Parse for LocatedTreeInput { - fn parse(input: ParseStream<'_>) -> syn::Result { - let source = input.parse()?; - input.parse::()?; - let template = input.parse()?; - Ok(Self { source, template }) - } -} - struct NamedString { value: String, } diff --git a/shared/yeast/doc/yeast.md b/shared/yeast/doc/yeast.md index 9609ad6e322c..a58949db4438 100644 --- a/shared/yeast/doc/yeast.md +++ b/shared/yeast/doc/yeast.md @@ -297,45 +297,8 @@ leaf nodes: (identifier "synthetic" location: {source_node}) ``` -The following macros can be used to explicitly set the location associated -with a newly-created node. They assign a location only to the root of their -template; nested nodes still derive their locations normally. - -`tree_at!` assigns the range of one captured input node to the template root: - -```rust -rule!( - (wrapper - source: (_) @source_node - child: (_) @child) - => - synthetic_node { - tree_at!( - source_node, - (synthetic_node child: (nested value: {child})) - ) - } -) -``` - -`tree_spanning!` assigns the smallest range containing several captured input -nodes: - -```rust -rule!( - (wrapper - first: (_) @first - second: (_) @second - child: (_) @child) - => - synthetic_node { - tree_spanning!( - [first, second], - (synthetic_node child: {child}) - ) - } -) -``` +The override applies only to the node carrying the `location` field; nested +nodes still derive their locations normally. For input fields whose leading or trailing syntax should never belong to rule results, configure them once with diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index 9a533ee12e3f..d971c9427139 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -15,7 +15,7 @@ pub mod schema; mod visitor; pub use range::{Point, Range}; -pub use yeast_macros::{query, rule, rules, tree, tree_at, tree_spanning, trees}; +pub use yeast_macros::{query, rule, rules, tree, trees}; use captures::Captures; use query::QueryNode; diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index a238e0b9f76f..867b81691921 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1841,9 +1841,9 @@ fn test_explicit_recursive_translation_keeps_nested_rule_location() { assert_eq!(identifier.byte_range(), 4..7); } -/// `tree_at!` assigns the captured node's range only to the template root. +/// A `location` field assigns the captured node's range only to its node. #[test] -fn test_tree_at_assigns_capture_range_to_root_only() { +fn test_location_field_assigns_capture_range_to_node_only() { let rule: Rule = rule!( (call method: (identifier) @name @@ -1851,7 +1851,9 @@ fn test_tree_at_assigns_capture_range_to_root_only() { ) @@source => call { - let arguments = tree_at!(source, (argument_list argument: (integer "0"))); + let arguments = tree!((argument_list + argument: (integer "0") + location: {source})); tree!((call method: {name} receiver: {recv} arguments: {arguments})) } ); @@ -1874,10 +1876,9 @@ fn test_tree_at_assigns_capture_range_to_root_only() { assert_eq!(integer.byte_range(), 0..0); } -/// `tree_spanning!` assigns the union of the captured node ranges only to the -/// template root. +/// Multiple `location` fields assign their union only to their node. #[test] -fn test_tree_spanning_assigns_union_to_root_only() { +fn test_multiple_location_fields_assign_union_to_node_only() { let rule: Rule = rule!( (call method: (identifier) @name @@ -1885,10 +1886,10 @@ fn test_tree_spanning_assigns_union_to_root_only() { ) => call { - let arguments = tree_spanning!( - [recv, name], - (argument_list argument: (integer "0")) - ); + let arguments = tree!((argument_list + argument: (integer "0") + location: {recv} + location: {name})); tree!((call method: {name} receiver: {recv} arguments: {arguments})) } ); From f48bd79efc513924a38cfc27b39c8ab7f7081204 Mon Sep 17 00:00:00 2001 From: Taus Date: Tue, 20 Oct 2026 14:51:52 +0000 Subject: [PATCH 4/4] yeast: Evaluate fields and locations in order Instead of filtering out the location overrides and doing them all at the end, we now evaluate them as we encounter them, gathering up all of the node Ids (if any) in a list. This list is then traversed (trivially so, if there are no entries) after we construct the node, to get the final location (or None if no overrides were present), and the newly constructed node's location is extended with this location information. --- shared/yeast-macros/src/lower.rs | 140 +++++++++++++++---------------- shared/yeast/tests/test.rs | 25 ++++++ 2 files changed, 94 insertions(+), 71 deletions(-) diff --git a/shared/yeast-macros/src/lower.rs b/shared/yeast-macros/src/lower.rs index 5e4abbd8bacf..640c55dfc437 100644 --- a/shared/yeast-macros/src/lower.rs +++ b/shared/yeast-macros/src/lower.rs @@ -319,7 +319,7 @@ impl Node { let ctx = Ident::new("ctx", Span::call_site()); let kind_str = self.kind.to_string(); - if let Some(content) = &self.content { + let node_init = if let Some(content) = &self.content { let node = match content { Content::Static(lit) => quote! { #ctx.literal(#kind_str, #lit) }, Content::Computed(block) => { @@ -362,97 +362,96 @@ impl Node { } } }; - return self.with_location(node, &ctx); - } + quote! { let __node: yeast::Id = #node; } + } else { + TokenStream::new() + }; let mut stmts = Vec::new(); let mut field_args = Vec::new(); for (field_counter, field) in self.fields.iter().enumerate() { - let Field::Named { - name, - value, - optional, - } = field - else { - continue; - }; - let field_name = name.to_string(); - let field_str = field_name - .strip_prefix("r#") - .unwrap_or(&field_name) - .to_string(); - let temp = Ident::new( - &format!("__field_{field_str}_{field_counter}"), - Span::call_site(), - ); - - match value { - Template::Splice(block) => { - debug_assert!(!optional); + match field { + Field::Location { block } => { let block = rust_block(block); stmts.push(quote! { - let mut #temp: Vec = Vec::new(); - yeast::IntoFieldIds::extend_into(#block, &mut #temp); - }); - field_args.push(quote! { - if !#temp.is_empty() { - __fields.push((#field_str, #temp)); - } + yeast::IntoFieldIds::extend_into(#block, &mut __location_ids); }); } - Template::Node(child) if *optional => { - let label = fresh_fallible_label(); - let value = child.lower(&LoweringContext { - fallible_scope: Some(&label), - }); - stmts.push(quote! { - #[allow(unused_labels)] - let #temp: ::std::option::Option = #label: { - ::std::option::Option::Some(#value) - }; - }); - field_args.push(quote! { - if let ::std::option::Option::Some(__id) = #temp { - __fields.push((#field_str, vec![__id])); + Field::Named { + name, + value, + optional, + } => { + let field_name = name.to_string(); + let field_str = field_name + .strip_prefix("r#") + .unwrap_or(&field_name) + .to_string(); + let temp = Ident::new( + &format!("__field_{field_str}_{field_counter}"), + Span::call_site(), + ); + + match value { + Template::Splice(block) => { + debug_assert!(!optional); + let block = rust_block(block); + stmts.push(quote! { + let mut #temp: Vec = Vec::new(); + yeast::IntoFieldIds::extend_into(#block, &mut #temp); + }); + field_args.push(quote! { + if !#temp.is_empty() { + __fields.push((#field_str, #temp)); + } + }); } - }); - } - Template::Node(child) => { - let value = child.lower(context); - stmts.push(quote! { let #temp: yeast::Id = #value; }); - field_args.push(quote! { - __fields.push((#field_str, vec![#temp])); - }); + Template::Node(child) if *optional => { + let label = fresh_fallible_label(); + let value = child.lower(&LoweringContext { + fallible_scope: Some(&label), + }); + stmts.push(quote! { + #[allow(unused_labels)] + let #temp: ::std::option::Option = #label: { + ::std::option::Option::Some(#value) + }; + }); + field_args.push(quote! { + if let ::std::option::Option::Some(__id) = #temp { + __fields.push((#field_str, vec![__id])); + } + }); + } + Template::Node(child) => { + let value = child.lower(context); + stmts.push(quote! { let #temp: yeast::Id = #value; }); + field_args.push(quote! { + __fields.push((#field_str, vec![#temp])); + }); + } + } } } } - let node = quote! { - { - #(#stmts)* + let build_node = if self.content.is_none() { + quote! { let mut __fields: Vec<(&str, Vec)> = Vec::new(); #(#field_args)* - #ctx.node(#kind_str, __fields) + let __node: yeast::Id = #ctx.node(#kind_str, __fields); } + } else { + TokenStream::new() }; - self.with_location(node, &ctx) - } - - fn with_location(&self, node: TokenStream, ctx: &Ident) -> TokenStream { - let locations = self.fields.iter().filter_map(|field| match field { - Field::Named { .. } => None, - Field::Location { block } => Some(rust_block(block)), - }); - let locations: Vec<_> = locations.collect(); - if locations.is_empty() { - return node; - } quote! { { let mut __location_ids: Vec = Vec::new(); - #(yeast::IntoFieldIds::extend_into(#locations, &mut __location_ids);)* + #node_init + #(#stmts)* + #build_node let __source_range = __location_ids .into_iter() .filter_map(|__source| { @@ -461,7 +460,6 @@ impl Node { .and_then(|__node| __node.source_range()) }) .reduce(yeast::Range::union); - let __node: yeast::Id = #node; #ctx.set_node_source_range(__node, __source_range) } } diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index 867b81691921..7d62418d2607 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1756,6 +1756,31 @@ fn test_multiple_location_fields_combine_iterable_locations() { assert_eq!(arguments.byte_range(), 0..5); } +#[test] +fn test_location_and_named_fields_are_evaluated_left_to_right() { + let rule: Rule = rule!( + (assignment + left: (identifier) @left + right: (integer) @right) + => + call { + let mut order = Vec::new(); + let call = tree!((call + location: {order.push("first location"); left} + method: {order.push("method"); left} + location: {order.push("second location"); right} + arguments: {order.push("arguments"); right})); + assert_eq!( + order, + ["first location", "method", "second location", "arguments"] + ); + call + } + ); + + run_and_ast("x = 1", vec![rule]); +} + #[test] fn test_location_field_is_available_on_literal_nodes() { let rule: Rule = rule!(