diff --git a/crates/opy-rs/src/compiler/tests/preprocessing.rs b/crates/opy-rs/src/compiler/tests/preprocessing.rs index 850374d1..da521fe1 100644 --- a/crates/opy-rs/src/compiler/tests/preprocessing.rs +++ b/crates/opy-rs/src/compiler/tests/preprocessing.rs @@ -85,6 +85,98 @@ 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 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 + // 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..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().span.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); } @@ -697,16 +704,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 +725,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 cf73a586..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 { @@ -735,6 +736,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 +918,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/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 14b2beac..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,11 +287,11 @@ 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); - shift_expansion_spans(&mut out, use_site); + attribute_expansion_spans(&mut out, use_site); Ok(out) } @@ -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,13 +422,23 @@ fn raw_arg_text(tokens: &[Token]) -> String { out } -pub(super) fn shift_expansion_spans(tokens: &mut [Token], origin: Span) { +/// 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). `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 = Span::new( - origin.file, - crate::diag::shift_position(token.span.start, origin.start), - crate::diag::shift_position(token.span.end, origin.start), + 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 e7fa3fcc..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,17 +72,17 @@ 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); - super::macros::shift_expansion_spans(&mut tokens, use_site); + super::macros::attribute_expansion_spans(&mut tokens, use_site); Ok(tokens) } } 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