From 59e87faebf3f8250216e99a87e37e3f25b551f47 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 10 Oct 2026 03:58:08 +0800 Subject: [PATCH 1/3] fix(preprocess): attribute macro-expanded tokens to the use site Expanded text does not appear in authored source, so the authored macro name is the only honest provenance. Offsetting generated tokens by the expansion's own width recorded positions past the end of the authored line, and a multi-line expansion attributed its later tokens to unrelated lines. Those fabricated spans surfaced downstream as invalid-span provenance failures. Give every expanded token the use-site span in both object/function macro expansion and script expansion, and update the multi-line span test to pin the new contract. Include/script/settings span translation keeps using shift_position where positions still correspond to real text. Fixes #506 --- crates/opy-rs/src/preprocess.rs | 50 ++++++++++++++++++++++--- crates/opy-rs/src/preprocess/macros.rs | 16 ++++---- crates/opy-rs/src/preprocess/scripts.rs | 2 +- 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/crates/opy-rs/src/preprocess.rs b/crates/opy-rs/src/preprocess.rs index cf73a586..36f525c2 100644 --- a/crates/opy-rs/src/preprocess.rs +++ b/crates/opy-rs/src/preprocess.rs @@ -735,6 +735,41 @@ mod tests { assert_eq!(numbers, vec!["1.5"]); } + #[test] + fn expanded_tokens_carry_the_use_site_span() { + // #506: an expansion wider than its invocation must not record + // positions past the authored line; every expanded token attributes + // to the authored macro name. + let (pre, _) = preprocess( + "#!define T a_much_wider_replacement + another_token\nrule \"r\":\n x = T\n", + "main.opy", + Path::new("."), + ) + .unwrap(); + let use_site = pre + .tokens + .iter() + .find(|token| token.text == "a_much_wider_replacement") + .expect("expanded token") + .span; + for token in pre.tokens.iter().filter(|token| { + matches!( + token.text.as_str(), + "a_much_wider_replacement" | "+" | "another_token" + ) + }) { + assert_eq!( + token.span, use_site, + "{} must attribute to the use site", + token.text + ); + } + // `T` sits at 3:9-3:10 in the authored source; the replacement is far + // wider, so shifted coordinates would have overrun the line. + assert_eq!(use_site.start, crate::diag::Position::new(3, 9)); + assert_eq!(use_site.end, crate::diag::Position::new(3, 10)); + } + #[test] fn function_define_substitutes_params() { let (pre, _) = preprocess( @@ -882,23 +917,28 @@ mod tests { } #[test] - fn multiline_define_preserves_relative_expansion_spans() { + fn multiline_define_expanded_tokens_attribute_to_the_use_site() { + // #506: a multi-line expansion attributes every token to the + // invocation — shifted line numbers would point at unrelated + // authored lines. let (pre, _) = preprocess( "#!define block() A = 1\\\n A = 2\nblock()\n", "main.opy", Path::new("."), ) .expect("multiline define expansion"); + let expected = Span::new( + 0, + crate::diag::Position::new(3, 1), + crate::diag::Position::new(3, 6), + ); let numbers: Vec = pre .tokens .iter() .filter(|token| token.kind == TokenKind::Number) .map(|token| token.span) .collect(); - assert_eq!(numbers.len(), 2); - assert_eq!(numbers[0].start.line, 3); - assert_eq!(numbers[1].start.line, 4); - assert_eq!(numbers[1].start.col, 9); + assert_eq!(numbers, vec![expected; 2]); } #[test] diff --git a/crates/opy-rs/src/preprocess/macros.rs b/crates/opy-rs/src/preprocess/macros.rs index 14b2beac..a4cb4a92 100644 --- a/crates/opy-rs/src/preprocess/macros.rs +++ b/crates/opy-rs/src/preprocess/macros.rs @@ -291,7 +291,7 @@ impl Preprocessor { text: &replacement, })?; out.retain(|token| token.kind != TokenKind::Eof); - shift_expansion_spans(&mut out, use_site); + attribute_expansion_spans(&mut out, use_site); Ok(out) } @@ -430,13 +430,15 @@ fn raw_arg_text(tokens: &[Token]) -> String { out } -pub(super) fn shift_expansion_spans(tokens: &mut [Token], origin: Span) { +/// Attribute every token produced by an expansion to the use-site span. +/// The expanded text does not appear in authored source, so the authored +/// macro name is the only honest provenance: offsetting tokens by the +/// expansion's own width recorded positions past the authored line, and a +/// multi-line expansion attributed its later tokens to unrelated authored +/// lines (#506). +pub(super) fn attribute_expansion_spans(tokens: &mut [Token], origin: Span) { for token in tokens { - token.span = Span::new( - origin.file, - crate::diag::shift_position(token.span.start, origin.start), - crate::diag::shift_position(token.span.end, origin.start), - ); + token.span = origin; } } diff --git a/crates/opy-rs/src/preprocess/scripts.rs b/crates/opy-rs/src/preprocess/scripts.rs index e7fa3fcc..b5feb286 100644 --- a/crates/opy-rs/src/preprocess/scripts.rs +++ b/crates/opy-rs/src/preprocess/scripts.rs @@ -82,7 +82,7 @@ impl Preprocessor { text: &indented, })?; tokens.retain(|token| token.kind != TokenKind::Eof); - super::macros::shift_expansion_spans(&mut tokens, use_site); + super::macros::attribute_expansion_spans(&mut tokens, use_site); Ok(tokens) } } From 430547d387360e531bd118b734395ed338aebb71 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 10 Oct 2026 04:19:06 +0800 Subject: [PATCH 2/3] fix(preprocess): carry expansion layout separately from provenance Uniform use-site spans collapsed the expansion's line/column structure, which the indentation-sensitive parser reads for block decisions: multi-line expansions lost statements or failed to parse (review finding on #508). Split the two roles on Token: `span` is authored provenance (the use-site span for expanded tokens), and a crate-internal `layout` keeps the shifted coordinates the parser uses for indentation and statement-boundary decisions. `layout` anchors at the invocation token's own layout position so nested expansions keep their relative structure. AST spans and diagnostics keep reading `span`, so expanded constructs report the authored use site. Settings-block and f-string fragment tokens keep authored behavior: their layout is the shifted position and provenance clamps into the enclosing string span when there is one, so interpolations inside expanded strings stop fabricating columns past the use site. Coverage: multi-line if/for/rule defines keep their block structure through both macro and script expansion, and a malformed interpolation inside an expanded string reports inside the use-site span. --- .../src/compiler/tests/preprocessing.rs | 72 +++++++++++++++++++ crates/opy-rs/src/lexer.rs | 9 +++ crates/opy-rs/src/lower.rs | 2 +- crates/opy-rs/src/parser.rs | 28 ++++++-- crates/opy-rs/src/parser/declarations.rs | 6 +- crates/opy-rs/src/parser/definitions.rs | 6 +- crates/opy-rs/src/parser/expressions.rs | 36 ++++++---- crates/opy-rs/src/parser/statements.rs | 32 ++++----- crates/opy-rs/src/preprocess.rs | 1 + crates/opy-rs/src/preprocess/directives.rs | 1 + crates/opy-rs/src/preprocess/macros.rs | 48 ++++++------- crates/opy-rs/src/preprocess/scripts.rs | 6 +- .../opy-rs/tests/fixtures/macros/blockgen.js | 1 + .../opy-rs/tests/fixtures/macros/blockgen.opy | 9 +++ crates/opy-rs/tests/macro_integration.rs | 16 +++++ 15 files changed, 203 insertions(+), 70 deletions(-) create mode 100644 crates/opy-rs/tests/fixtures/macros/blockgen.js create mode 100644 crates/opy-rs/tests/fixtures/macros/blockgen.opy diff --git a/crates/opy-rs/src/compiler/tests/preprocessing.rs b/crates/opy-rs/src/compiler/tests/preprocessing.rs index 850374d1..bf343b53 100644 --- a/crates/opy-rs/src/compiler/tests/preprocessing.rs +++ b/crates/opy-rs/src/compiler/tests/preprocessing.rs @@ -85,6 +85,78 @@ fn multiline_object_define_preserves_statement_boundaries() { assert!(matches!(rule.actions[1], Stmt::Assign { .. })); } +#[test] +fn multiline_function_define_preserves_block_structure() { + // #506: expanded tokens keep the expansion's relative layout for the + // indentation-sensitive parser even though their authored provenance is + // the use site — a collapsed layout would let `C = 2` escape the body. + let hir = crate::compile( + "#!define check() if A == 1:\\\n B = 1\\\n C = 2\nglobalvar A\nglobalvar B\nglobalvar C\nrule \"block macro\":\n @Event global\n check()\n", + "main.opy", + std::path::Path::new("."), + ) + .expect("multiline function-like defines must preserve block structure"); + + let RuleEntry::Rule(rule) = &hir.rules[0] else { + panic!("expected a rule"); + }; + assert_eq!(rule.actions.len(), 1); + let Stmt::If { branches, .. } = &rule.actions[0] else { + panic!("expected an if statement, got {:?}", rule.actions[0]); + }; + assert_eq!(branches.len(), 1); + assert_eq!( + branches[0].body.len(), + 2, + "both expansion statements must stay inside the if body" + ); +} + +#[test] +fn multiline_function_define_preserves_for_block() { + let hir = crate::compile( + "#!define gen() for i in range(0, 3):\\\n B = i\nglobalvar B\nglobalvar i\nrule \"for macro\":\n @Event global\n gen()\n", + "main.opy", + std::path::Path::new("."), + ) + .expect("multiline defines must keep expect_block_indent constructs intact"); + + let RuleEntry::Rule(rule) = &hir.rules[0] else { + panic!("expected a rule"); + }; + assert!(matches!(rule.actions[0], Stmt::For { .. })); +} + +#[test] +fn expanded_f_string_interpolation_errors_stay_inside_the_use_site() { + // #506: an interpolation inside an expanded string has no authored + // extent; its derived positions clamp into the use-site span rather + // than fabricating columns past the authored line. + let error = crate::compile( + "globalvar B\nrule \"bad f-string\":\n @Event global\n A = f\"hp: {B +}\"\n", + "main.opy", + std::path::Path::new("."), + ) + .expect_err("the malformed interpolation must fail"); + let span = error.span.expect("the error must carry a span"); + assert_eq!(span.start.line, 4); + + let error = crate::compile( + "#!define M f\"hp: {B +}\"\nglobalvar A\nglobalvar B\nrule \"expanded bad f-string\":\n @Event global\n A = M\n", + "main.opy", + std::path::Path::new("."), + ) + .expect_err("the malformed expanded interpolation must fail"); + let span = error.span.expect("the error must carry a span"); + // `M` is the use site at 6:9-6:10; the fabricated interpolation column + // (string start + index) must clamp into it instead of escaping. + assert_eq!(span.start.line, 6); + assert!( + (9..=10).contains(&span.start.col) && (9..=10).contains(&span.end.col), + "the reported position must stay inside the authored use site, got {span:?}" + ); +} + #[test] fn nested_includes_resolve_relative_to_the_including_file() { let overlay = BTreeMap::from([ diff --git a/crates/opy-rs/src/lexer.rs b/crates/opy-rs/src/lexer.rs index 096bfb05..6a4b7175 100644 --- a/crates/opy-rs/src/lexer.rs +++ b/crates/opy-rs/src/lexer.rs @@ -76,7 +76,15 @@ pub struct Token { /// constructs such as f-string interpolations can recover expression /// spans without losing provenance during preprocessing. pub raw: Option, + /// Authored-source provenance. Tokens produced by macro or script + /// expansion carry the use-site span: the expanded text does not exist + /// in authored source, so the invocation is the only honest position. pub span: Span, + /// The position the indentation-sensitive parser reads for layout + /// decisions. It equals `span` for authored tokens; expanded tokens keep + /// the expansion's relative line and column structure here so blocks and + /// statement boundaries still resolve (#506). + pub(crate) layout: Span, } impl Token { @@ -86,6 +94,7 @@ impl Token { text: text.into(), raw: None, span, + layout: span, } } } diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index bce6bf75..2e554a10 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -467,7 +467,7 @@ impl SettingsLowerer { file: u32, origin: crate::diag::Position, ) -> OpyResult { - let expression = crate::parser::parse_expression_fragment(text, file, origin)?; + let expression = crate::parser::parse_expression_fragment(text, file, origin, None)?; self.lowerer.errors.clear(); self.lowerer.texture_used = false; let lowered = self diff --git a/crates/opy-rs/src/parser.rs b/crates/opy-rs/src/parser.rs index e5738a9a..cf50ad17 100644 --- a/crates/opy-rs/src/parser.rs +++ b/crates/opy-rs/src/parser.rs @@ -93,7 +93,7 @@ impl<'a> Parser<'a> { if token.kind == TokenKind::Newline { let entries = newline_lines.entry(token.span.file).or_default(); let line = token - .span + .layout .start .line .max(entries.last().map_or(0, |&(_, max)| max)); @@ -522,7 +522,7 @@ impl Parser<'_> { self.error_at_current("expected an indented block".to_string()); return None; } - let indent = self.peek().span.start.col; + let indent = self.peek().layout.start.col; if indent <= line_indent { self.error_at_current("expected an indented block after ':'".to_string()); return None; @@ -543,7 +543,7 @@ impl Parser<'_> { let continued_line = self .tokens .get(self.pos.saturating_sub(1)) - .is_some_and(|previous| self.peek().span.start.line > previous.span.end.line); + .is_some_and(|previous| self.peek().layout.start.line > previous.layout.end.line); self.last_statement_continued = continued_line; if matches!(self.peek_kind(), TokenKind::Newline | TokenKind::Eof) || continued_line { Ok(()) @@ -554,19 +554,24 @@ impl Parser<'_> { } } -/// Parse one f-string expression fragment and shift its local token spans -/// into the original source file. +/// Parse one expression fragment and shift its local token spans into the +/// original source file. `bounds` limits the authored provenance of the +/// fragment's tokens: inside an expanded string the interpolation has no +/// authored extent, so derived positions clamp into the string's span +/// rather than fabricating columns past it (#506). pub(crate) fn parse_expression_fragment( text: &str, file: u32, origin: Position, + bounds: Option, ) -> Result { let mut tokens = crate::lexer::lex(crate::lexer::LexInput { file_id: file, text, })?; for token in &mut tokens { - token.span = shift_span(token.span, origin); + token.layout = shift_span(token.span, origin); + token.span = bounds.map_or(token.layout, |bounds| bounded_span(token.layout, bounds)); } let mut parser = Parser::new(&tokens, false); let expression = parser.parse_expr().map_err(|()| { @@ -592,6 +597,17 @@ fn shift_span(span: Span, origin: Position) -> Span { ) } +pub(crate) fn bounded_span(span: Span, bounds: Span) -> Span { + if span.file != bounds.file { + return span; + } + Span::new( + span.file, + span.start.clamp(bounds.start, bounds.end), + span.end.clamp(bounds.start, bounds.end), + ) +} + fn is_string_modifier(text: &str) -> bool { matches!(text, "f" | "w" | "l" | "b" | "c" | "t") } diff --git a/crates/opy-rs/src/parser/declarations.rs b/crates/opy-rs/src/parser/declarations.rs index 0bb6eca3..38878a87 100644 --- a/crates/opy-rs/src/parser/declarations.rs +++ b/crates/opy-rs/src/parser/declarations.rs @@ -97,14 +97,14 @@ impl Parser<'_> { Err(()) => return false, }; let Ok(body_indent) = - self.expect_block_indent(start.span.start.col, "':' after the enum name") + self.expect_block_indent(start.layout.start.col, "':' after the enum name") else { return false; }; let mut members = Vec::new(); loop { self.skip_newlines(); - if self.peek_kind() == TokenKind::Eof || self.peek().span.start.col < body_indent { + if self.peek_kind() == TokenKind::Eof || self.peek().layout.start.col < body_indent { break; } if self.peek_kind() == TokenKind::Ident { @@ -202,7 +202,7 @@ impl Parser<'_> { args.insert(0, "self".to_string()); } let Ok(body_indent) = - self.expect_block_indent(start.span.start.col, "':' after the macro signature") + self.expect_block_indent(start.layout.start.col, "':' after the macro signature") else { return false; }; diff --git a/crates/opy-rs/src/parser/definitions.rs b/crates/opy-rs/src/parser/definitions.rs index 7b1b5893..adb452d1 100644 --- a/crates/opy-rs/src/parser/definitions.rs +++ b/crates/opy-rs/src/parser/definitions.rs @@ -54,7 +54,7 @@ impl Parser<'_> { .max(name_token_span.start.col + 1), ), ); - let line_indent = start.span.start.col; + let line_indent = start.layout.start.col; if self .expect_block_indent(line_indent, "':' after the rule name") .is_err() @@ -69,7 +69,7 @@ impl Parser<'_> { let mut actions = Vec::new(); loop { self.skip_newlines(); - if self.peek_kind() == TokenKind::Eof || self.peek().span.start.col < body_indent { + if self.peek_kind() == TokenKind::Eof || self.peek().layout.start.col < body_indent { break; } if self.peek_kind() == TokenKind::At { @@ -350,7 +350,7 @@ impl Parser<'_> { return false; } let Ok(body_indent) = - self.expect_block_indent(start.span.start.col, "':' after the subroutine signature") + self.expect_block_indent(start.layout.start.col, "':' after the subroutine signature") else { return false; }; diff --git a/crates/opy-rs/src/parser/expressions.rs b/crates/opy-rs/src/parser/expressions.rs index f58adc5c..86a1db4b 100644 --- a/crates/opy-rs/src/parser/expressions.rs +++ b/crates/opy-rs/src/parser/expressions.rs @@ -13,7 +13,7 @@ impl Parser<'_> { let then_value = self.parse_or()?; if self.is_ident("if") && !self.inside_delimiter_group() - && self.peek().span.start.line != then_value.span().end.line + && self.peek().layout.start.line != then_value.span().end.line && self.newline_after_line(then_value.span().file, then_value.span().end.line) { return Ok(then_value); @@ -697,16 +697,19 @@ impl Parser<'_> { self.errors.push(OpyError::at( "parse-error", "f-string interpolation cannot be empty".to_string(), - Span::new( - string_span.file, - Position::new( - string_span.start.line, - string_span.start.col + index as u32 + 1, - ), - Position::new( - string_span.start.line, - string_span.start.col + end as u32 + 1, + crate::parser::bounded_span( + Span::new( + string_span.file, + Position::new( + string_span.start.line, + string_span.start.col + index as u32 + 1, + ), + Position::new( + string_span.start.line, + string_span.start.col + end as u32 + 1, + ), ), + string_span, ), )); return Err(()); @@ -715,10 +718,15 @@ impl Parser<'_> { string_span.start.line, string_span.start.col + index as u32 + 1, ); - let parsed = parse_expression_fragment(&expression, string_span.file, origin) - .map_err(|error| { - self.errors.push(error); - }); + let parsed = parse_expression_fragment( + &expression, + string_span.file, + origin, + Some(string_span), + ) + .map_err(|error| { + self.errors.push(error); + }); let Ok(parsed) = parsed else { return Err(()); }; diff --git a/crates/opy-rs/src/parser/statements.rs b/crates/opy-rs/src/parser/statements.rs index f1968215..e9370a87 100644 --- a/crates/opy-rs/src/parser/statements.rs +++ b/crates/opy-rs/src/parser/statements.rs @@ -8,10 +8,10 @@ impl Parser<'_> { if self.peek_kind() == TokenKind::Eof { break; } - if self.peek().span.start.col < block_indent { + if self.peek().layout.start.col < block_indent { break; } - if self.peek().span.start.col > block_indent { + if self.peek().layout.start.col > block_indent { self.error_at_current("unexpected indentation".to_string()); self.recover_line(); continue; @@ -247,7 +247,7 @@ impl Parser<'_> { } pub(super) fn parse_if(&mut self) -> Result { - let indent = self.peek().span.start.col; + let indent = self.peek().layout.start.col; self.open_if_indents.push(indent); let stmt = self.parse_if_chain(); self.open_if_indents.pop(); @@ -256,7 +256,7 @@ impl Parser<'_> { fn parse_if_chain(&mut self) -> Result { let start = self.advance(); - let line_indent = start.span.start.col; + let line_indent = start.layout.start.col; let condition = self.parse_expr()?; let body = self.expect_colon_body(line_indent, "':' after the if condition")?; let continued_inline_body = self.last_colon_body_continued; @@ -278,7 +278,7 @@ impl Parser<'_> { loop { let save = self.pos; self.skip_newlines(); - let column = self.peek().span.start.col; + let column = self.peek().layout.start.col; if self.peek_kind() == TokenKind::Eof || column > line_indent || (column != line_indent && self.open_if_indents.contains(&column)) @@ -293,7 +293,7 @@ impl Parser<'_> { Err(()) => return Err(()), }; let body = self.expect_colon_body( - branch_start.span.start.col, + branch_start.layout.start.col, "':' after the elif condition", )?; branches.push(IfBranch { @@ -307,7 +307,7 @@ impl Parser<'_> { self.bump(); let condition = self.parse_expr()?; let body = self.expect_colon_body( - branch_start.span.start.col, + branch_start.layout.start.col, "':' after the else-if condition", )?; branches.push(IfBranch { @@ -318,7 +318,7 @@ impl Parser<'_> { continue; } let body = - self.expect_colon_body(branch_start.span.start.col, "':' after `else`")?; + self.expect_colon_body(branch_start.layout.start.col, "':' after `else`")?; else_span = Some(branch_start.span); r#else = Some(body); break; @@ -350,7 +350,7 @@ impl Parser<'_> { if matches!(self.peek_kind(), TokenKind::Newline | TokenKind::Eof) { let save = self.pos; self.skip_newlines(); - if self.peek_kind() != TokenKind::Eof && self.peek().span.start.col == line_indent { + if self.peek_kind() != TokenKind::Eof && self.peek().layout.start.col == line_indent { let statement = self.parse_statement()?; self.expect_statement_end("the inline statement")?; self.last_colon_body_continued = self.last_statement_continued; @@ -386,7 +386,7 @@ impl Parser<'_> { self.bump(); let iterable = self.parse_expr()?; let body_indent = - self.expect_block_indent(start.span.start.col, "':' after the for header")?; + self.expect_block_indent(start.layout.start.col, "':' after the for header")?; let body = self.parse_block(body_indent); Ok(Stmt::For { variable, @@ -400,7 +400,7 @@ impl Parser<'_> { let start = self.advance(); let condition = self.parse_expr()?; let body_indent = - self.expect_block_indent(start.span.start.col, "':' after the while condition")?; + self.expect_block_indent(start.layout.start.col, "':' after the while condition")?; let body = self.parse_block(body_indent); Ok(Stmt::While { condition, @@ -411,7 +411,7 @@ impl Parser<'_> { pub(super) fn parse_do_while(&mut self) -> Result { let start = self.advance(); - let body_indent = self.expect_block_indent(start.span.start.col, "':' after `do`")?; + let body_indent = self.expect_block_indent(start.layout.start.col, "':' after `do`")?; let body = self.parse_block(body_indent); if !self.is_ident("while") { self.error_at_current("expected `while` after the do block".to_string()); @@ -434,14 +434,14 @@ impl Parser<'_> { let start = self.advance(); let value = self.parse_expr()?; let body_indent = - self.expect_block_indent(start.span.start.col, "':' after the switch value")?; + self.expect_block_indent(start.layout.start.col, "':' after the switch value")?; let mut arms = Vec::new(); loop { self.skip_newlines(); - if self.peek_kind() == TokenKind::Eof || self.peek().span.start.col < body_indent { + if self.peek_kind() == TokenKind::Eof || self.peek().layout.start.col < body_indent { break; } - if self.peek().span.start.col != body_indent { + if self.peek().layout.start.col != body_indent { self.error_at_current("unexpected indentation in switch".to_string()); self.recover_line(); continue; @@ -465,7 +465,7 @@ impl Parser<'_> { body: self.parse_block(default_body_indent), span: default_start.span, }); - if default_start.span.start.col != body_indent { + if default_start.layout.start.col != body_indent { self.error_at_current("invalid default indentation".to_string()); return Err(()); } diff --git a/crates/opy-rs/src/preprocess.rs b/crates/opy-rs/src/preprocess.rs index 36f525c2..4038ecde 100644 --- a/crates/opy-rs/src/preprocess.rs +++ b/crates/opy-rs/src/preprocess.rs @@ -568,6 +568,7 @@ impl Preprocessor { let mut tokens = tokens; for token in &mut tokens { token.span = shift_settings_span(token.span, block.text_start); + token.layout = token.span; } let tokens = self.expand(tokens)?; Ok(SettingsBlock { diff --git a/crates/opy-rs/src/preprocess/directives.rs b/crates/opy-rs/src/preprocess/directives.rs index cae909dc..9ce384ec 100644 --- a/crates/opy-rs/src/preprocess/directives.rs +++ b/crates/opy-rs/src/preprocess/directives.rs @@ -40,6 +40,7 @@ impl Preprocessor { text: prefix, raw: None, span: token.span, + layout: token.layout, }); out.push(token.clone()); index += 1; diff --git a/crates/opy-rs/src/preprocess/macros.rs b/crates/opy-rs/src/preprocess/macros.rs index a4cb4a92..94276396 100644 --- a/crates/opy-rs/src/preprocess/macros.rs +++ b/crates/opy-rs/src/preprocess/macros.rs @@ -167,7 +167,7 @@ impl Preprocessor { } else { (Vec::new(), index + 1) }; - let mut expanded = self.expand_macro(mac, args, token.span, line_indent(tokens, index))?; + let mut expanded = self.expand_macro(mac, args, token, line_indent(tokens, index))?; self.expand_into(&mut expanded, &mut Vec::new(), 0)?; Ok((expanded, after)) } @@ -254,7 +254,7 @@ impl Preprocessor { &self, mac: &MacroDef, args: Vec, - use_site: Span, + use_site: &Token, line_indent: u32, ) -> OpyResult> { if mac.is_function && args.len() != mac.params.len() { @@ -266,7 +266,7 @@ impl Preprocessor { mac.params.len(), args.len() ), - use_site, + use_site.span, )); } if let Some(script) = &mac.script { @@ -287,7 +287,7 @@ impl Preprocessor { replacement = replacement.replace('\n', &format!("\n{indent}")); } let mut out = lex(LexInput { - file_id: use_site.file, + file_id: use_site.span.file, text: &replacement, })?; out.retain(|token| token.kind != TokenKind::Eof); @@ -341,12 +341,8 @@ impl Preprocessor { if mac.is_function { if index + 1 < tokens.len() && tokens[index + 1].kind == TokenKind::LParen { let (args, after) = self.collect_args(tokens, index + 1)?; - let mut expanded = self.expand_macro( - mac, - args, - token.span, - line_indent(tokens, index), - )?; + let mut expanded = + self.expand_macro(mac, args, token, line_indent(tokens, index))?; stack.push(name.clone()); self.expand_into(&mut expanded, stack, depth + 1)?; stack.pop(); @@ -354,12 +350,8 @@ impl Preprocessor { index = after; continue; } - let mut expanded = self.expand_macro( - mac, - Vec::new(), - token.span, - line_indent(tokens, index), - )?; + let mut expanded = + self.expand_macro(mac, Vec::new(), token, line_indent(tokens, index))?; stack.push(name.clone()); self.expand_into(&mut expanded, stack, depth + 1)?; stack.pop(); @@ -368,7 +360,7 @@ impl Preprocessor { continue; } let mut expanded = - self.expand_macro(mac, Vec::new(), token.span, line_indent(tokens, index))?; + self.expand_macro(mac, Vec::new(), token, line_indent(tokens, index))?; stack.push(name.clone()); self.expand_into(&mut expanded, stack, depth + 1)?; stack.pop(); @@ -393,7 +385,7 @@ fn line_indent(tokens: &[Token], index: usize) -> u32 { tokens .get(line_start..=index) .and_then(|line| line.iter().find(|token| token.kind != TokenKind::Newline)) - .map_or(0, |token| token.span.start.col.saturating_sub(1)) + .map_or(0, |token| token.layout.start.col.saturating_sub(1)) } fn position_offset(source: &str, position: crate::diag::Position) -> Option { @@ -430,15 +422,23 @@ fn raw_arg_text(tokens: &[Token]) -> String { out } -/// Attribute every token produced by an expansion to the use-site span. -/// The expanded text does not appear in authored source, so the authored -/// macro name is the only honest provenance: offsetting tokens by the +/// Give every token produced by an expansion the use-site span as its +/// authored provenance while keeping the expansion's relative layout for +/// the parser. The expanded text does not appear in authored source, so +/// the authored macro name is the only honest `span`: offsetting it by the /// expansion's own width recorded positions past the authored line, and a /// multi-line expansion attributed its later tokens to unrelated authored -/// lines (#506). -pub(super) fn attribute_expansion_spans(tokens: &mut [Token], origin: Span) { +/// lines (#506). `layout` keeps the shifted coordinates anchored at the +/// invocation's layout position because the indentation-sensitive parser +/// reads it for block structure. +pub(super) fn attribute_expansion_spans(tokens: &mut [Token], use_site: &Token) { for token in tokens { - token.span = origin; + token.layout = Span::new( + use_site.span.file, + crate::diag::shift_position(token.span.start, use_site.layout.start), + crate::diag::shift_position(token.span.end, use_site.layout.start), + ); + token.span = use_site.span; } } diff --git a/crates/opy-rs/src/preprocess/scripts.rs b/crates/opy-rs/src/preprocess/scripts.rs index b5feb286..3f4180db 100644 --- a/crates/opy-rs/src/preprocess/scripts.rs +++ b/crates/opy-rs/src/preprocess/scripts.rs @@ -58,7 +58,7 @@ impl Preprocessor { mac: &MacroDef, script: &ScriptMacro, args: Vec, - use_site: Span, + use_site: &Token, line_indent: u32, ) -> OpyResult> { let macro_args: Vec = mac @@ -72,13 +72,13 @@ impl Preprocessor { let runtime = MacroRuntime::new(Limits::default()); let result = runtime .run_macro(&script.source, ¯o_args, &script.path) - .map_err(|error| map_macro_error(&error, &script.path, use_site))?; + .map_err(|error| map_macro_error(&error, &script.path, use_site.span))?; // Reference indentation rule (`resolveMacro`): every newline in the // replacement is followed by the call line's indentation. let indent = " ".repeat(line_indent as usize); let indented = result.text.replace('\n', &format!("\n{indent}")); let mut tokens = lex(LexInput { - file_id: use_site.file, + file_id: use_site.span.file, text: &indented, })?; tokens.retain(|token| token.kind != TokenKind::Eof); diff --git a/crates/opy-rs/tests/fixtures/macros/blockgen.js b/crates/opy-rs/tests/fixtures/macros/blockgen.js new file mode 100644 index 00000000..7e688b1c --- /dev/null +++ b/crates/opy-rs/tests/fixtures/macros/blockgen.js @@ -0,0 +1 @@ +"if A == 1:\n B = 1\n C = 2"; diff --git a/crates/opy-rs/tests/fixtures/macros/blockgen.opy b/crates/opy-rs/tests/fixtures/macros/blockgen.opy new file mode 100644 index 00000000..bbb51c8b --- /dev/null +++ b/crates/opy-rs/tests/fixtures/macros/blockgen.opy @@ -0,0 +1,9 @@ +#!define gen() __script__("blockgen.js") + +globalvar A +globalvar B +globalvar C + +rule "script block": + @Event global + gen() diff --git a/crates/opy-rs/tests/macro_integration.rs b/crates/opy-rs/tests/macro_integration.rs index a4a853a1..65f7a9c6 100644 --- a/crates/opy-rs/tests/macro_integration.rs +++ b/crates/opy-rs/tests/macro_integration.rs @@ -87,6 +87,22 @@ fn script_macro_helpers_surface_is_available() { ); } +#[test] +fn script_macro_multiline_output_preserves_block_structure() { + // #506: script expansions carry the use-site span as provenance but keep + // the expansion's internal layout for the parser — a collapsed layout + // would let `C = 2` escape the `if` body. + let program = compile_fixture("blockgen.opy").unwrap(); + let opy_rs::hir::RuleEntry::Rule(rule) = &program.rules[0] else { + panic!("expected a rule"); + }; + assert_eq!(rule.actions.len(), 1); + let opy_rs::hir::Stmt::If { branches, .. } = &rule.actions[0] else { + panic!("expected an if statement, got {:?}", rule.actions[0]); + }; + assert_eq!(branches[0].body.len(), 2); +} + #[test] fn missing_script_file_is_a_structured_diagnostic() { // The reference resolves the script path at the define site and fails From 161e8a0f565e7cf2174259cd72e9e9eee4d65a71 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 10 Oct 2026 04:57:23 +0800 Subject: [PATCH 3/3] fix(parser): keep the conditional boundary guard in the layout domain The ternary statement-boundary heuristic compared the if keyword's layout line against the then-value's provenance span line, so a conditional expression on a later expansion line collapsed to the use site and the guard misfired (expected ':' after the if condition). Compare against the just-consumed token's layout, which is exactly what the pre-split span carried. Refs wrightkit/opy-rs#506 --- .../src/compiler/tests/preprocessing.rs | 20 +++++++++++++++++++ crates/opy-rs/src/parser/expressions.rs | 11 ++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/opy-rs/src/compiler/tests/preprocessing.rs b/crates/opy-rs/src/compiler/tests/preprocessing.rs index bf343b53..da521fe1 100644 --- a/crates/opy-rs/src/compiler/tests/preprocessing.rs +++ b/crates/opy-rs/src/compiler/tests/preprocessing.rs @@ -127,6 +127,26 @@ fn multiline_function_define_preserves_for_block() { assert!(matches!(rule.actions[0], Stmt::For { .. })); } +#[test] +fn multiline_function_define_preserves_conditional_expressions() { + // #506 re-review: the conditional-expression statement-boundary guard + // must compare layout lines on both sides; reading the then-value's + // provenance span collapses expanded tokens to the use site and the + // guard misfires, so `expected ':' after the if condition` fails. + let hir = crate::compile( + "#!define E() A = 0\\\n B = 0\\\n y = B if C else D\nglobalvar A\nglobalvar B\nglobalvar C\nglobalvar D\nglobalvar y\nrule \"a\":\n @Event global\n E()\n", + "main.opy", + std::path::Path::new("."), + ) + .expect("a conditional expression on a later expansion line must parse"); + + let RuleEntry::Rule(rule) = &hir.rules[0] else { + panic!("expected a rule"); + }; + assert_eq!(rule.actions.len(), 3); + assert!(matches!(rule.actions[2], Stmt::Assign { .. })); +} + #[test] fn expanded_f_string_interpolation_errors_stay_inside_the_use_site() { // #506: an interpolation inside an expanded string has no authored diff --git a/crates/opy-rs/src/parser/expressions.rs b/crates/opy-rs/src/parser/expressions.rs index 86a1db4b..4183264a 100644 --- a/crates/opy-rs/src/parser/expressions.rs +++ b/crates/opy-rs/src/parser/expressions.rs @@ -11,10 +11,17 @@ impl Parser<'_> { ) -> Result { self.skip_expression_newlines(); let then_value = self.parse_or()?; + // The statement-boundary heuristic compares virtual layout lines on + // both sides; `then_value`'s last token is the one just consumed, and + // its provenance span would collapse every expanded token to the + // macro use site (#506 re-review). if self.is_ident("if") && !self.inside_delimiter_group() - && self.peek().layout.start.line != then_value.span().end.line - && self.newline_after_line(then_value.span().file, then_value.span().end.line) + && self.peek().layout.start.line != self.tokens[self.pos - 1].layout.end.line + && self.newline_after_line( + self.tokens[self.pos - 1].layout.file, + self.tokens[self.pos - 1].layout.end.line, + ) { return Ok(then_value); }