diff --git a/shared/yeast-macros/src/lower.rs b/shared/yeast-macros/src/lower.rs index 6cf8767e2f1b..0aaa4c38e95f 100644 --- a/shared/yeast-macros/src/lower.rs +++ b/shared/yeast-macros/src/lower.rs @@ -1,5 +1,5 @@ -use proc_macro2::{Ident, Span, TokenStream}; -use quote::quote; +use proc_macro2::{Ident, Span, TokenStream, TokenTree}; +use quote::{quote, ToTokens}; use std::cell::RefCell; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -324,12 +324,16 @@ impl Template { quote! { __nodes.push(#node); } } Template::Splice(block) => { + let uses_ctx = block_uses_ctx(block); let block = rust_block(block); - quote! { - let __value = ::std::iter::IntoIterator::into_iter(#block) - .collect::>(); - ctx.extend_field_ids(__value, &mut __nodes); - } + isolated_interpolation( + uses_ctx, + quote! { + let __value = ::std::iter::IntoIterator::into_iter(#block) + .collect::>(); + ctx.extend_field_ids(__value, &mut __nodes); + }, + ) } Template::Choice(alternatives) => { let pick = Self::lower_choice(alternatives); @@ -355,8 +359,14 @@ impl Template { match self { Template::Node(node) => node.lower(context), Template::Splice(block) => { + let uses_ctx = block_uses_ctx(block); let block = rust_block(block); - quote! { ::std::convert::Into::::into(#block) } + isolated_interpolation( + uses_ctx, + quote! { + ::std::convert::Into::::into(#block) + }, + ) } Template::Choice(_) => { unreachable!("choices are lowered through Template::lower_choice") @@ -453,35 +463,38 @@ impl Node { let node = match content { Content::Static(lit) => quote! { #ctx.literal(#kind_str, #lit) }, Content::Computed(block) => { + let uses_ctx = block_uses_ctx(block); let block = rust_block(block); if let Some(scope) = context.choice_alternative_scope { let value = fresh_prepared_ident("value"); let source_range = fresh_prepared_ident("source_range"); + let expression = isolated_interpolation( + uses_ctx, + 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, + ), + ) + }, + ) + }, + ); scope .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, - ), - ) - }, - ) - } - }, + expression, }); quote! { #ctx.literal_with_source_range( @@ -491,8 +504,9 @@ impl Node { ) } } else { - quote! { - { + isolated_interpolation( + uses_ctx, + quote! { let __expr = #block; let __value = yeast::YeastDisplay::yeast_to_string(&__expr, &#ctx); @@ -503,8 +517,8 @@ impl Node { &__value, __source_range, ) - } - } + }, + ) } } Content::Inherited => quote! { #ctx.inherited_literal(#kind_str) }, @@ -524,12 +538,16 @@ impl Node { for (field_counter, field) in self.fields.iter().enumerate() { match field { Field::Location { block } => { + let uses_ctx = block_uses_ctx(block); let block = rust_block(block); - stmts.push(quote! { - let __value = ::std::iter::IntoIterator::into_iter(#block) - .collect::>(); - #ctx.extend_location_ranges(__value, &mut __location_ranges); - }); + stmts.push(isolated_interpolation( + uses_ctx, + quote! { + let __value = ::std::iter::IntoIterator::into_iter(#block) + .collect::>(); + #ctx.extend_location_ranges(__value, &mut __location_ranges); + }, + )); } Field::LocationNone => {} Field::Named { name, value } => { @@ -544,12 +562,19 @@ impl Node { ); match value { Template::Splice(block) => { + let uses_ctx = block_uses_ctx(block); let block = rust_block(block); + let interpolation = isolated_interpolation( + uses_ctx, + quote! { + let __value = ::std::iter::IntoIterator::into_iter(#block) + .collect::>(); + #ctx.extend_field_ids(__value, &mut #temp); + }, + ); stmts.push(quote! { let mut #temp: Vec = Vec::new(); - let __value = ::std::iter::IntoIterator::into_iter(#block) - .collect::>(); - #ctx.extend_field_ids(__value, &mut #temp); + #interpolation }); field_args.push(quote! { if !#temp.is_empty() { @@ -612,3 +637,33 @@ fn rust_block(block: &syn::Block) -> TokenStream { let statements = &block.stmts; quote!({ #(#statements)* }) } + +fn isolated_interpolation(uses_ctx: bool, body: TokenStream) -> TokenStream { + if uses_ctx { + quote! {{ + let __saved_user_ctx = (*ctx.user_ctx).clone(); + let __result = { #body }; + *ctx.user_ctx = __saved_user_ctx; + __result + }} + } else { + quote! {{ #body }} + } +} + +/// Returns whether `block` directly mentions `ctx`. +/// +/// This lexical check cannot see context access hidden behind a helper or +/// macro. Such an interpolation can opt into isolation by explicitly +/// mentioning `ctx`, for example with `let _ = &ctx;`. +fn block_uses_ctx(block: &syn::Block) -> bool { + fn uses_ctx(tokens: TokenStream) -> bool { + tokens.into_iter().any(|token| match token { + TokenTree::Ident(ident) => ident == "ctx", + TokenTree::Group(group) => uses_ctx(group.stream()), + TokenTree::Punct(_) | TokenTree::Literal(_) => false, + }) + } + + uses_ctx(block.to_token_stream()) +} diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index f0e705c0f139..d802e81e626b 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -943,6 +943,153 @@ fn test_choice_fallback_can_reuse_consumed_splice() { ); } +#[derive(Clone, Default)] +struct InterpolationContext { + counter: u64, +} + +#[test] +fn test_output_template_interpolations_clone_user_context() { + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = InterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + let source = yeast::tree!((identifier "source")); + + let nodes = yeast::trees!( + (call + location: { + ctx.counter += 1; + source + } + method: { + ctx.counter += 1; + let value = ctx.counter.to_string(); + ctx.literal("identifier", &value) + } + receiver: (identifier #{ + ctx.counter += 1; + ctx.counter + }) + ) + { + ctx.counter += 1; + let value = ctx.counter.to_string(); + [ctx.literal("identifier", &value)] + } + ); + + assert_eq!(ctx.counter, 0); + assert_eq!(nodes.len(), 2); + assert_dump_eq( + &dump_ast(ctx.output, nodes[0], input), + r#" + call + method: identifier "1" + receiver: identifier "1" + "#, + ); + assert_dump_eq( + &dump_ast(ctx.output, nodes[1], input), + r#" + identifier "1" + "#, + ); +} + +#[test] +fn test_interpolation_preserves_build_ctx_method_borrowing() { + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = InterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + let source = yeast::tree!((identifier "source")); + + let node = yeast::tree!((call + method: { + ctx.literal("identifier", &ctx.source_text(source)) + } + )); + + assert_dump_eq( + &dump_ast(ctx.output, node, input), + r#" + call + method: identifier "source" + "#, + ); +} + +#[test] +fn test_interpolation_preserves_user_context_assignment() { + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = InterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + + let node = yeast::tree!((identifier #{ + *ctx = InterpolationContext { counter: 1 }; + ctx.counter + })); + + assert_eq!(ctx.counter, 0); + assert_dump_eq( + &dump_ast(ctx.output, node, input), + r#" + identifier "1" + "#, + ); +} + +#[test] +fn test_interpolation_accepts_borrowed_build_ctx() { + fn build_identifier( + ctx: &mut yeast::build::BuildCtx<'_, InterpolationContext>, + ) -> yeast::OutputId { + yeast::tree!((identifier #{ + ctx.counter += 1; + ctx.counter + })) + } + + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = InterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + + let node = build_identifier(&mut ctx); + assert_eq!(ctx.counter, 0); + assert_dump_eq( + &dump_ast(ctx.output, node, input), + r#" + identifier "1" + "#, + ); +} + +#[derive(Clone, Default)] +struct BorrowedInterpolationContext { + locations: Vec, +} + +#[test] +fn test_interpolation_accepts_value_borrowed_from_context() { + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = BorrowedInterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + let location = yeast::tree!((identifier "location")); + ctx.locations.push(location); + let locations = ctx.locations.iter(); + + yeast::tree!((call location: {locations})); +} + #[test] fn test_root_choice_returns_an_optional_node() { let input = "x = 1"; diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 4599cc79f62f..db8f6490a265 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, tree}; +use yeast::{tree, ConcreteDesugarer, DesugaringConfig, Rule}; /// User context propagated from outer rules down to the inner rules that /// emit the corresponding output declarations, so that each emitted node @@ -51,10 +51,9 @@ impl SwiftContext { /// Called before recursively translating a body / initializer /// slot. Most rules mutate `ctx` in place — the framework invokes /// each rule with a private clone of the user context, so - /// mutations are discarded on rule exit anyway. Rules that need - /// the outer context intact *after* the reset-and-translate (see - /// e.g. the `property_binding` willSet/didSet rule) wrap the - /// mutation in `ctx.scoped(...)` instead. + /// mutations are discarded on rule exit anyway. Output-template + /// interpolations additionally restore the context after evaluation, + /// preventing mutations from leaking into sibling fields. fn reset(&mut self) { *self = SwiftContext::default(); } @@ -382,17 +381,13 @@ fn translation_rules() -> Vec> { let mut result = Vec::new(); if let Some(val) = val { // Stored property with observers: the initializer is not part - // of the binding, so translate it in a reset scope. - let val = ctx.scoped(|ctx| { - ctx.reset(); - ctx.translate(val) - }); + // of the binding, so translate it in a reset interpolation. result.push(tree!( (variable_declaration modifier: {ctx.outer_modifiers.clone()} pattern: (identifier #{name}) type: {ty} - value: {val}) + value: {ctx.reset(); val}) )); } for acc in accessors.into_iter() { @@ -747,19 +742,11 @@ fn translation_rules() -> Vec> { trailingClosure: _? @trailing additionalTrailingClosures: _* @additional) => - expr { - // Always translate the callee in non-pattern context. - let callee = ctx.scoped(|ctx| { - ctx.in_pattern = false; - ctx.translate(rawCallee) - }); - tree!((call_expr - callee: {callee} - argument: {args} - argument: {trailing.map(|trailing| tree!((argument value: {trailing})))} - argument: {additional} - )) - } + (call_expr + callee: {ctx.in_pattern = false; rawCallee} + argument: {args} + argument: {trailing.map(|trailing| tree!((argument value: {trailing})))} + argument: {additional}) ), // A call or enum-case pattern argument. rule!(