From 41e5effe4f84b22f474bf2461f5d4237dab6af5b Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 28 Oct 2026 13:43:51 +0000 Subject: [PATCH 1/2] yeast: Add explicit locationless nodes These are needed for Python, where expression contexts (Store/Load/Param/Del) are AST nodes that are explicitly locationless. Thus, for these the default heuristic of giving them an empty range at the start of the enclosing node is not that useful (and the subsequent unioning of child ranges is quite detrimental).. We therefore need a way to specify that some nodes are locationless, and I ended up with the syntax `location: none`. I briefly considered writing this as `location: {None}` instead, but unfortunately `rustc` does not accept this as the type of `None` is not fully specified. It would have to be written as `None::` which is quite ugly. Finally, having explicit location overrides and `location: none` at the same time is an error (that we discover during parsing). --- shared/yeast-macros/src/ast.rs | 2 + shared/yeast-macros/src/lib.rs | 1 + shared/yeast-macros/src/lower.rs | 97 +++++++++++++---------- shared/yeast-macros/src/template_parse.rs | 67 ++++++++++++++-- shared/yeast/doc/yeast.md | 8 ++ shared/yeast/src/build.rs | 8 ++ shared/yeast/src/lib.rs | 12 +++ shared/yeast/tests/test.rs | 18 +++++ 8 files changed, 163 insertions(+), 50 deletions(-) diff --git a/shared/yeast-macros/src/ast.rs b/shared/yeast-macros/src/ast.rs index a08d56e9661d..81cd7210c21b 100644 --- a/shared/yeast-macros/src/ast.rs +++ b/shared/yeast-macros/src/ast.rs @@ -143,4 +143,6 @@ pub(crate) enum Field { /// A Rust block yielding the node or nodes whose locations should be combined. block: syn::Block, }, + /// Explicitly omit source-location metadata: `location: none`. + LocationNone, } diff --git a/shared/yeast-macros/src/lib.rs b/shared/yeast-macros/src/lib.rs index 65c6bde6cf09..f42dc02db5f7 100644 --- a/shared/yeast-macros/src/lib.rs +++ b/shared/yeast-macros/src/lib.rs @@ -67,6 +67,7 @@ pub fn query(input: TokenStream) -> TokenStream { /// field: {expr} - extend a named field with `{expr}`'s ids /// location: {expr} - override this node's source range with the /// span of one or more nodes; may be repeated +/// location: none - explicitly make this node locationless /// field: [(kind ...)] - set the field only if the alternative can /// be constructed (see below) /// field: [(a) (b)] - use the first constructible alternative diff --git a/shared/yeast-macros/src/lower.rs b/shared/yeast-macros/src/lower.rs index a4179a098d38..765e16beb944 100644 --- a/shared/yeast-macros/src/lower.rs +++ b/shared/yeast-macros/src/lower.rs @@ -367,9 +367,7 @@ impl Template { fn lower_choice(alternatives: &[Alternative]) -> TokenStream { let pick = alternatives.iter().rev().fold( quote! { ::std::option::Option::None }, - |fallback, alternative| { - alternative.lower_candidate(fallback) - }, + |fallback, alternative| alternative.lower_candidate(fallback), ); quote! {{ #pick }} } @@ -402,25 +400,26 @@ impl Alternative { // evaluated in source order. Construction happens only after the // resulting option has been matched, keeping it in a separate branch // from the fallback. - let prepared = interpolations.into_iter().rev().fold( - prepared, - |candidate, interpolation| { - let GuardingInterpolation { - value, - source_range, - expression, - } = interpolation; - // Do not use `map`/`and_then` here: `candidate` may contain - // embedded user Rust, and putting it in a closure would change - // the target of `return` and the result type seen by `?`. - quote! { - match #expression { - ::std::option::Option::Some((#value, #source_range)) => #candidate, - ::std::option::Option::None => ::std::option::Option::None, + let prepared = + interpolations + .into_iter() + .rev() + .fold(prepared, |candidate, interpolation| { + let GuardingInterpolation { + value, + source_range, + expression, + } = interpolation; + // Do not use `map`/`and_then` here: `candidate` may contain + // embedded user Rust, and putting it in a closure would change + // the target of `return` and the result type seen by `?`. + quote! { + match #expression { + ::std::option::Option::Some((#value, #source_range)) => #candidate, + ::std::option::Option::None => ::std::option::Option::None, + } } - } - }, - ); + }); let prepared = if let Some(guard) = &self.guard { let guard = rust_block(guard); quote! { @@ -462,27 +461,27 @@ impl Node { .interpolations .borrow_mut() .push(GuardingInterpolation { - value: value.clone(), - source_range: source_range.clone(), - expression: quote! { - { - let __expr = #block; - yeast::MaybeYeastValue::maybe_yeast_value(&__expr).map( - |__value_ref| { - ( - yeast::YeastDisplay::yeast_to_string( - __value_ref, - &#ctx, - ), - yeast::YeastSourceRange::yeast_source_range( - __value_ref, - &#ctx, - ), - ) - }, - ) - } - }, + value: value.clone(), + source_range: source_range.clone(), + expression: quote! { + { + let __expr = #block; + yeast::MaybeYeastValue::maybe_yeast_value(&__expr).map( + |__value_ref| { + ( + yeast::YeastDisplay::yeast_to_string( + __value_ref, + &#ctx, + ), + yeast::YeastSourceRange::yeast_source_range( + __value_ref, + &#ctx, + ), + ) + }, + ) + } + }, }); quote! { #ctx.literal_with_source_range( @@ -516,6 +515,10 @@ impl Node { let mut stmts = Vec::new(); let mut field_args = Vec::new(); + let location_none = self + .fields + .iter() + .any(|field| matches!(field, Field::LocationNone)); for (field_counter, field) in self.fields.iter().enumerate() { match field { @@ -527,6 +530,7 @@ impl Node { #ctx.extend_location_ranges(__value, &mut __location_ranges); }); } + Field::LocationNone => {} Field::Named { name, value } => { let field_name = name.to_string(); let field_str = field_name @@ -591,8 +595,13 @@ impl Node { #node_init #(#stmts)* #build_node - let __source_range = __location_ranges.into_iter().reduce(yeast::Range::union); - #ctx.set_node_source_range(__node, __source_range) + if #location_none { + #ctx.set_node_locationless(__node) + } else { + let __source_range = + __location_ranges.into_iter().reduce(yeast::Range::union); + #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 5d63a1e45c27..cce42fd0ab19 100644 --- a/shared/yeast-macros/src/template_parse.rs +++ b/shared/yeast-macros/src/template_parse.rs @@ -71,6 +71,8 @@ fn parse_node(group: proc_macro2::Group) -> Result { }; let mut fields = Vec::new(); + let mut has_location = false; + let mut has_location_none = false; while peek_is_field(&mut tokens) { let name = expect_ident(&mut tokens, "expected field name")?; let is_location = name == "location"; @@ -82,21 +84,47 @@ fn parse_node(group: proc_macro2::Group) -> Result { } expect_punct(&mut tokens, ':', "expected `:` after field name")?; if is_location { - if !peek_is_group(&mut tokens, Delimiter::Brace) { + if matches!(tokens.peek(), Some(TokenTree::Ident(ident)) if ident == "none") { + let none = tokens.next().unwrap(); + if has_location || has_location_none { + return Err(syn::Error::new_spanned( + none, + "`location: none` cannot be combined with another `location` field", + )); + } + has_location_none = true; + fields.push(Field::LocationNone); + } else if peek_is_group(&mut tokens, Delimiter::Brace) { + if has_location_none { + return Err(syn::Error::new_spanned( + tokens.peek().unwrap().clone(), + "`location: none` cannot be combined with another `location` field", + )); + } + has_location = true; + let block = parse_block(expect_group(&mut tokens, Delimiter::Brace)?)?; + fields.push(Field::Location { block }); + } else { let error = match tokens.peek() { Some(token) => syn::Error::new_spanned( token.clone(), - "`location` field value must be a Rust block", + "`location` field value must be a Rust block or `none`", ), None => syn::Error::new( Span::call_site(), - "`location` field value must be a Rust block", + "`location` field value must be a Rust block or `none`", ), }; return Err(error); } - let block = parse_block(expect_group(&mut tokens, Delimiter::Brace)?)?; - fields.push(Field::Location { block }); + 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", + )); + } + } continue; } @@ -290,6 +318,19 @@ mod tests { assert!(matches!(node.fields.as_slice(), [Field::Location { .. }])); } + #[test] + fn parses_location_none() { + let template = parse_template(quote! { + (identifier "name" location: none) + }) + .unwrap(); + + let Template::Node(node) = template else { + panic!("expected node template"); + }; + assert!(matches!(node.fields.as_slice(), [Field::LocationNone])); + } + #[test] fn rejects_non_block_location_fields() { let result = parse_template(quote! { @@ -300,7 +341,21 @@ mod tests { }; assert_eq!( error.to_string(), - "`location` field value must be a Rust block" + "`location` field value must be a Rust block or `none`" + ); + } + + #[test] + fn rejects_location_none_with_other_locations() { + let result = parse_template(quote! { + (identifier "name" location: none location: {source}) + }); + let Err(error) = result else { + panic!("expected mixed location metadata to be rejected"); + }; + assert_eq!( + error.to_string(), + "`location: none` cannot be combined with another `location` field" ); } diff --git a/shared/yeast/doc/yeast.md b/shared/yeast/doc/yeast.md index a2f28ae6d072..282c46144997 100644 --- a/shared/yeast/doc/yeast.md +++ b/shared/yeast/doc/yeast.md @@ -300,6 +300,14 @@ leaf nodes: The override applies only to the node carrying the `location` field; nested nodes still derive their locations normally. +Use `location: none` for a synthetic node that intentionally has no source +location. This also prevents a returned rule result from inheriting the matched +input range: + +```rust +(metadata_node location: none) +``` + For input fields whose leading or trailing syntax should never belong to rule results, configure them once with `DesugaringConfig::with_ignored_location_fields(...)`. For example, ignoring diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index f3075973be41..ef820224f3aa 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -187,6 +187,14 @@ impl<'a, C> BuildCtx<'a, C> { node } + /// Explicitly make a newly-built node locationless. + #[doc(hidden)] + pub fn set_node_locationless(&mut self, node: OutputId) -> OutputId { + self.output.clear_source_range(node); + self.created_nodes_without_source_range.remove(&node); + node + } + /// Return the immutable input AST. Available inside rule transforms. pub fn ast(&self) -> &Ast { self.ast diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index 9eb9adaee81f..6e869f03bfc8 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -816,6 +816,18 @@ impl Ast { node.source_range = Some(source_range); } + /// Remove the source range from a synthetic node. + pub(crate) fn clear_source_range(&mut self, id: Id) { + let node = self + .nodes + .get_mut(id.index()) + .unwrap_or_else(|| panic!("clear_source_range: invalid node id {}", id.index())); + if matches!(node.content, NodeContent::Range(_)) { + panic!("clear_source_range: cannot modify a parsed node"); + } + node.source_range = None; + } + /// Register a named node kind, returning its id (idempotent). Lets callers /// build an AST in a single pass, registering kinds as nodes are created /// rather than pre-populating the schema. diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index 96ec714ddb69..29c08407d4fc 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1898,6 +1898,24 @@ fn test_location_field_overrides_derived_location() { assert_eq!(call.byte_range(), 4..5); } +#[test] +fn test_location_none_makes_result_root_locationless() { + let rule: Rule = rule!( + (call method: (identifier) @method) + => + (call method: {method} location: none) + ); + + let ast = run_and_ast("foo.bar()", 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.source_range(), None); +} + #[test] fn test_multiple_location_fields_combine_iterable_locations() { let rule: Rule = rule!( From 3f79f52fe59f0b73851107741e932e1241543a04 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 30 Oct 2026 16:55:38 +0100 Subject: [PATCH 2/2] yeast: Update documentation on `location: none` usage and add test Clarified usage of `location: none` for synthetic nodes and its effects on parent node ranges. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- shared/yeast/doc/yeast.md | 9 ++++++--- shared/yeast/tests/test.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/shared/yeast/doc/yeast.md b/shared/yeast/doc/yeast.md index 282c46144997..99d3424c1a52 100644 --- a/shared/yeast/doc/yeast.md +++ b/shared/yeast/doc/yeast.md @@ -300,9 +300,12 @@ leaf nodes: The override applies only to the node carrying the `location` field; nested nodes still derive their locations normally. -Use `location: none` for a synthetic node that intentionally has no source -location. This also prevents a returned rule result from inheriting the matched -input range: +As an exception to the Rust-block requirement above, use `location: none` for a +synthetic node that intentionally has no source location. The field-combination +rules above apply only to block-valued fields: `location: none` cannot be +combined with any other `location` field on the same node. A locationless node +does not contribute to its parent's derived range, and a returned rule result +marked this way does not inherit the matched input range: ```rust (metadata_node location: none) diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index 29c08407d4fc..5e37bf1e8ce0 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -1916,6 +1916,42 @@ fn test_location_none_makes_result_root_locationless() { assert_eq!(call.source_range(), None); } +/// A nested `location: none` node stays locationless and does not widen its +/// parent's derived range with an empty range at the match start. +#[test] +fn test_location_none_nested_node_does_not_widen_parent() { + let rule: Rule = rule!( + (call method: (identifier) @method) + => + (call + method: (identifier "outer") + receiver: (call + method: {method} + arguments: (argument_list location: none))) + ); + + let ast = run_and_ast("foo.bar()", vec![rule]); + let nodes: Vec<_> = ast + .reachable_node_ids() + .into_iter() + .filter_map(|id| ast.get_node(id)) + .collect(); + let arguments = nodes + .iter() + .find(|node| node.kind_name() == "argument_list") + .expect("argument list exists"); + assert_eq!(arguments.source_range(), None); + + let mut call_ranges: Vec<_> = nodes + .iter() + .filter(|node| node.kind_name() == "call") + .map(|node| node.byte_range()) + .collect(); + call_ranges.sort_by_key(|range| (range.start, range.end)); + // Without `location: none`, the inner call would be widened to 0..7. + assert_eq!(call_ranges, [0..9, 4..7]); +} + #[test] fn test_multiple_location_fields_combine_iterable_locations() { let rule: Rule = rule!(