From 13044ac0a59772de03ebc94dc62970de681a470c Mon Sep 17 00:00:00 2001 From: Teakowa Date: Tue, 29 Sep 2026 20:20:51 +0800 Subject: [PATCH 1/5] feat(api): expose recorded identifier and nested-value provenance Refs #324. Add public accessors on Program for the identifier spans the raw parser already records: - global_variable_name_span, player_variable_name_span, and subroutine_name_span return the declared-name span of each declaration (an attached identifier span when a provider supplies one, else the recorded declaration span, which raw parses record as the name). - action_identifier_span returns the span recorded for the variable or subroutine an action names: an infix-assignment/for target or a Call Subroutine callee. - condition_value_span and action_argument_value_span return the recorded span of a value nested inside a condition or action argument, addressed by a path into the public Value tree. Spans the parser never records (standard-form write targets, Call Subroutine callees, Global.name/Global Variable(...) read identifiers) return None; recording them is parser-side follow-up. Nested value spans are also written back through to_wir so diagnostics keep them. --- crates/workshop-rs/src/program.rs | 289 +++++++++++++++---- crates/workshop-rs/src/program/source_map.rs | 4 +- crates/workshop-rs/src/wir/action.rs | 6 - crates/workshop-rs/tests/program_model.rs | 179 +++++++++++- docs/source-preservation.md | 30 +- 5 files changed, 445 insertions(+), 63 deletions(-) diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index 6b38fe08..6fc7a091 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -39,14 +39,25 @@ struct DeclarationProvenance { #[derive(Debug, Clone, Default)] struct RuleProvenance { span: Option, - conditions: Vec>, + conditions: Vec, actions: Vec, } #[derive(Debug, Clone, Default)] struct ActionProvenance { span: Option, - arguments: Vec>, + /// The recorded span of the variable or subroutine the action names: a + /// set/modify/for target or a `Call Subroutine` callee. + identifier: Option, + arguments: Vec, +} + +/// The recorded span of one value node and its children, mirroring the +/// structure of the public [`Value`] tree. +#[derive(Debug, Clone, Default)] +struct ValueProvenance { + span: Option, + children: Vec, } /// A failure while attaching source mappings to a public [`Program`]. @@ -182,7 +193,7 @@ impl Program { } let rule_data = self.rule_provenance_mut(rule)?; fit(&mut rule_data.conditions, condition_count); - rule_data.conditions[condition] = span; + rule_data.conditions[condition].span = span; Ok(()) } @@ -234,8 +245,10 @@ impl Program { }); } let action_data = self.action_provenance_mut(rule, action)?; - action_data.arguments.resize(argument + 1, None); - action_data.arguments[argument] = span; + action_data + .arguments + .resize_with(argument + 1, ValueProvenance::default); + action_data.arguments[argument].span = span; Ok(()) } @@ -307,11 +320,7 @@ impl Program { /// Return the authored span of a public rule condition value. pub fn condition_span(&self, rule: usize, condition: usize) -> Option { - let recorded = self.rule_provenance(rule)?; - if recorded.conditions.len() != self.rules[rule].conditions.len() { - return None; - } - recorded.conditions.get(condition).copied().flatten() + self.condition_provenance(rule, condition)?.span } /// Return the authored span of a public action in its linear rule order. @@ -328,9 +337,107 @@ impl Program { ) -> Option { self.action_provenance(rule, action)? .arguments - .get(argument) - .copied() - .flatten() + .get(argument)? + .span + } + + /// Return the authored span of the declared name of a global variable. + /// + /// Source-language providers attach an explicit identifier span through + /// [`set_global_variable_spans`](Self::set_global_variable_spans). For raw + /// Workshop parses the recorded declaration span already covers exactly + /// the declared name and is returned as the identifier span. + pub fn global_variable_name_span(&self, variable: usize) -> Option { + self.declaration_name_span( + |provenance| &provenance.global_variables, + self.global_variables.len(), + variable, + ) + } + + /// Return the authored span of the declared name of a player variable. + /// See [`global_variable_name_span`](Self::global_variable_name_span). + pub fn player_variable_name_span(&self, variable: usize) -> Option { + self.declaration_name_span( + |provenance| &provenance.player_variables, + self.player_variables.len(), + variable, + ) + } + + /// Return the authored span of the declared name of a subroutine. + /// See [`global_variable_name_span`](Self::global_variable_name_span). + pub fn subroutine_name_span(&self, subroutine: usize) -> Option { + self.declaration_name_span( + |provenance| &provenance.subroutines, + self.subroutines.len(), + subroutine, + ) + } + + /// Return the span recorded for the variable or subroutine identifier a + /// public action names: the target of set/modify and for-variable actions, + /// or the callee of a [`Call Subroutine`](Action::CallSubroutine) action. + /// + /// Only the spans the parser actually recorded are returned: raw Workshop + /// records a target span for `Global.name` and `Event Player.name` infix + /// assignments — the `Global.name` form's span covers the qualified name + /// including the `Global.` qualifier — while standard-form `Set`/`Modify` + /// variable actions and `Call Subroutine` carry no recorded identifier and + /// return `None`. Other action forms always return `None`. + pub fn action_identifier_span(&self, rule: usize, action: usize) -> Option { + self.action_provenance(rule, action)?.identifier + } + + /// Return the authored span of a value nested inside a public rule + /// condition. + /// + /// `path` walks the public [`Value`] tree: each element selects a child by + /// position — `Value::Array` elements and `Value::Call` arguments by + /// index, `Value::Vector` components as `0`/`1`/`2` for x/y/z, and a + /// `Value::PlayerVariable` player at `0`. An empty path returns the + /// condition value's own span, matching [`condition_span`](Self::condition_span). + /// + /// For a variable or subroutine reference the returned span is whatever + /// the parser recorded for that node: raw Workshop records the declared + /// name for `Event Player.name`, bare-name, and `... At Index` argument + /// spellings, while `Global.name` and `Global/Player Variable(...)` reads + /// span their leading keyword rather than the identifier. + pub fn condition_value_span( + &self, + rule: usize, + condition: usize, + path: &[usize], + ) -> Option { + let mut value = self.condition_provenance(rule, condition)?; + for &index in path { + value = value.children.get(index)?; + } + value.span + } + + /// Return the authored span of a value nested inside a direct value + /// argument of a public action. + /// + /// `argument` selects the same direct argument as + /// [`action_argument_span`](Self::action_argument_span) and `path` walks + /// into it the way [`condition_value_span`](Self::condition_value_span) + /// describes; an empty path returns the argument's own span. + pub fn action_argument_value_span( + &self, + rule: usize, + action: usize, + argument: usize, + path: &[usize], + ) -> Option { + let mut value = self + .action_provenance(rule, action)? + .arguments + .get(argument)?; + for &index in path { + value = value.children.get(index)?; + } + value.span } /// Create a checked source edit through the authored source attached to @@ -389,6 +496,24 @@ impl Program { recorded.actions.get(action) } + fn condition_provenance(&self, rule: usize, condition: usize) -> Option<&ValueProvenance> { + let recorded = self.rule_provenance(rule)?; + if recorded.conditions.len() != self.rules[rule].conditions.len() { + return None; + } + recorded.conditions.get(condition) + } + + fn declaration_name_span( + &self, + recorded: impl Fn(&ProgramProvenance) -> &[DeclarationProvenance], + count: usize, + position: usize, + ) -> Option { + let declaration = self.declaration_provenance(recorded, count, position); + declaration.name_span.or(declaration.span) + } + fn declaration_provenance( &self, recorded: impl Fn(&ProgramProvenance) -> &[DeclarationProvenance], @@ -531,12 +656,7 @@ impl Program { conditions: rule .conditions .iter() - .map(|condition| { - storage - .values - .get(condition.value) - .and_then(|value| value.span) - }) + .map(|condition| value_provenance(&storage, condition.value)) .collect(), actions: action_provenance, }); @@ -622,8 +742,10 @@ impl Program { &subroutines, ) .inspect(|&value| { - if let Some(span) = self.condition_span(rule_index, condition_index) { - storage.values.get_mut(value).unwrap().span = Some(span); + if let Some(provenance) = + self.condition_provenance(rule_index, condition_index) + { + apply_value_provenance(&mut storage, value, provenance); } }) .map(|value| wir::Condition { @@ -995,21 +1117,24 @@ fn public_action_provenance( .actions .get(id) .ok_or_else(|| malformed_id("action", id.index()))?; + let identifier = action_identifier(action); let push = |output: &mut Vec, arguments: &[wir::ValueId]| { output.push(ActionProvenance { span: action.span(), + identifier, arguments: arguments .iter() - .map(|value| storage.values.get(*value).and_then(|value| value.span)) + .map(|value| value_provenance(storage, *value)) .collect(), }); }; let push_without_span = |output: &mut Vec, arguments: &[wir::ValueId]| { output.push(ActionProvenance { span: None, + identifier: None, arguments: arguments .iter() - .map(|value| storage.values.get(*value).and_then(|value| value.span)) + .map(|value| value_provenance(storage, *value)) .collect(), }); }; @@ -1313,11 +1438,9 @@ fn apply_action_provenance( if branch_index > 0 { let source = provenance.get(*position).cloned().unwrap_or_default(); *position += 1; - set_value_span( - storage, - branch.condition, - source.arguments.first().copied().flatten(), - ); + if let Some(provenance) = source.arguments.first() { + apply_value_provenance(storage, branch.condition, provenance); + } } apply_action_provenance(storage, &branch.body, provenance, position)?; } @@ -1335,11 +1458,9 @@ fn apply_action_provenance( apply_action_source(storage, *id, &source); apply_action_provenance(storage, &body, provenance, position)?; *position += 1; - set_value_span( - storage, - condition, - source.arguments.first().copied().flatten(), - ); + if let Some(provenance) = source.arguments.first() { + apply_value_provenance(storage, condition, provenance); + } } wir::Action::ForGlobalVariable { start, @@ -1353,8 +1474,8 @@ fn apply_action_provenance( apply_action_source(storage, *id, &source); apply_action_provenance(storage, &body, provenance, position)?; *position += 1; - for (value, span) in [start, stop, step].into_iter().zip(source.arguments) { - set_value_span(storage, value, span); + for (value, provenance) in [start, stop, step].into_iter().zip(&source.arguments) { + apply_value_provenance(storage, value, provenance); } } wir::Action::ForPlayerVariable { @@ -1370,11 +1491,11 @@ fn apply_action_provenance( apply_action_source(storage, *id, &source); apply_action_provenance(storage, &body, provenance, position)?; *position += 1; - for (value, span) in [player, start, stop, step] + for (value, provenance) in [player, start, stop, step] .into_iter() - .zip(source.arguments) + .zip(&source.arguments) { - set_value_span(storage, value, span); + apply_value_provenance(storage, value, provenance); } } wir::Action::Disabled { action, .. } => { @@ -1396,18 +1517,35 @@ fn apply_action_provenance( } fn apply_action_source(storage: &mut wir::Program, id: wir::ActionId, source: &ActionProvenance) { - let arguments = source.arguments.clone(); if let Some(action) = storage.actions.get_mut(id) { match action { - wir::Action::SetGlobalVariable { span, .. } - | wir::Action::ModifyGlobalVariable { span, .. } - | wir::Action::SetPlayerVariable { span, .. } - | wir::Action::ModifyPlayerVariable { span, .. } - | wir::Action::AssignMember { span, .. } - | wir::Action::CallSubroutine { span, .. } + wir::Action::SetGlobalVariable { + span, target_span, .. + } + | wir::Action::ModifyGlobalVariable { + span, target_span, .. + } + | wir::Action::SetPlayerVariable { + span, target_span, .. + } + | wir::Action::ModifyPlayerVariable { + span, target_span, .. + } + | wir::Action::ForGlobalVariable { + span, target_span, .. + } => { + *span = source.span; + *target_span = source.identifier; + } + wir::Action::CallSubroutine { + span, callee_span, .. + } => { + *span = source.span; + *callee_span = source.identifier; + } + wir::Action::AssignMember { span, .. } | wir::Action::If { span, .. } | wir::Action::While { span, .. } - | wir::Action::ForGlobalVariable { span, .. } | wir::Action::ForPlayerVariable { span, .. } | wir::Action::Disabled { span, .. } | wir::Action::Call { span, .. } => *span = source.span, @@ -1418,8 +1556,8 @@ fn apply_action_source(storage: &mut wir::Program, id: wir::ActionId, source: &A .get(id) .map(action_value_ids) .unwrap_or_default(); - for (value, span) in value_ids.into_iter().zip(arguments) { - set_value_span(storage, value, span); + for (value, provenance) in value_ids.into_iter().zip(&source.arguments) { + apply_value_provenance(storage, value, provenance); } } @@ -1449,13 +1587,62 @@ fn action_value_ids(action: &wir::Action) -> Vec { } } -fn set_value_span( +/// The recorded span of the variable or subroutine a WIR action names, when +/// the parse produced one. +fn action_identifier(action: &wir::Action) -> Option { + match action { + wir::Action::SetGlobalVariable { target_span, .. } + | wir::Action::ModifyGlobalVariable { target_span, .. } + | wir::Action::SetPlayerVariable { target_span, .. } + | wir::Action::ModifyPlayerVariable { target_span, .. } + | wir::Action::ForGlobalVariable { target_span, .. } => *target_span, + wir::Action::CallSubroutine { callee_span, .. } => *callee_span, + _ => None, + } +} + +/// The recorded provenance of a WIR value node and its children, mirroring +/// the public [`Value`] tree. +fn value_provenance(storage: &wir::Program, id: wir::ValueId) -> ValueProvenance { + let Some(node) = storage.values.get(id) else { + return ValueProvenance::default(); + }; + ValueProvenance { + span: node.span, + children: wir_value_children(&node.value) + .into_iter() + .map(|child| value_provenance(storage, child)) + .collect(), + } +} + +/// The child value ids of a WIR value, in the order the public [`Value`] +/// exposes them. +fn wir_value_children(value: &wir::Value) -> Vec { + match value { + wir::Value::Array(values) => values.clone(), + wir::Value::Vector { x, y, z } => vec![*x, *y, *z], + wir::Value::PlayerVariable { player, .. } => vec![*player], + wir::Value::Call { args, .. } => args.clone(), + _ => Vec::new(), + } +} + +/// Write a recorded value-provenance tree back onto a WIR value subtree. +fn apply_value_provenance( storage: &mut wir::Program, value: wir::ValueId, - span: Option, + source: &ValueProvenance, ) { + let Some(node) = storage.values.get(value) else { + return; + }; + let children = wir_value_children(&node.value); if let Some(node) = storage.values.get_mut(value) { - node.span = span; + node.span = source.span; + } + for (child, source) in children.into_iter().zip(&source.children) { + apply_value_provenance(storage, child, source); } } diff --git a/crates/workshop-rs/src/program/source_map.rs b/crates/workshop-rs/src/program/source_map.rs index dd82e5be..42edf095 100644 --- a/crates/workshop-rs/src/program/source_map.rs +++ b/crates/workshop-rs/src/program/source_map.rs @@ -331,7 +331,7 @@ impl SourceMap { .get_mut(*rule) .and_then(|rule| rule.conditions.get_mut(*condition)) .ok_or(SourceMapError::InvalidPosition)?; - if slot.replace(span).is_some() { + if slot.span.replace(span).is_some() { return Err(SourceMapError::DuplicateEntry); } } @@ -370,7 +370,7 @@ impl SourceMap { .ok_or(SourceMapError::InvalidPosition)? .arguments; fit(arguments, count); - if arguments[*argument].replace(span).is_some() { + if arguments[*argument].span.replace(span).is_some() { return Err(SourceMapError::DuplicateEntry); } } diff --git a/crates/workshop-rs/src/wir/action.rs b/crates/workshop-rs/src/wir/action.rs index 83fb65a4..79e7db8c 100644 --- a/crates/workshop-rs/src/wir/action.rs +++ b/crates/workshop-rs/src/wir/action.rs @@ -11,7 +11,6 @@ pub(crate) enum Action { variable: GlobalVarId, value: ValueId, span: Option, - #[allow(dead_code)] target_span: Option, }, ModifyGlobalVariable { @@ -19,7 +18,6 @@ pub(crate) enum Action { op: ModifyOp, value: ValueId, span: Option, - #[allow(dead_code)] target_span: Option, }, SetPlayerVariable { @@ -27,7 +25,6 @@ pub(crate) enum Action { variable: PlayerVarId, value: ValueId, span: Option, - #[allow(dead_code)] target_span: Option, }, ModifyPlayerVariable { @@ -36,7 +33,6 @@ pub(crate) enum Action { op: ModifyOp, value: ValueId, span: Option, - #[allow(dead_code)] target_span: Option, }, /// Assignment to a canonical Workshop member-access target, optionally @@ -51,7 +47,6 @@ pub(crate) enum Action { CallSubroutine { subroutine: SubroutineId, span: Option, - #[allow(dead_code)] callee_span: Option, }, If { @@ -71,7 +66,6 @@ pub(crate) enum Action { step: ValueId, body: Vec, span: Option, - #[allow(dead_code)] target_span: Option, }, /// `For Player Variable(player, name, start, stop, step)`: the diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index b991b569..39d28c4d 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -1,5 +1,5 @@ use workshop_rs::source::{Position, SourceFile, Span}; -use workshop_rs::{Action, Condition, Event, Program, Rule, Value, Variable}; +use workshop_rs::{Action, Condition, Event, Program, Rule, Subroutine, Value, Variable}; fn catalog() -> workshop_rs::catalog::Catalog { workshop_rs::catalog::Catalog::builtin().expect("builtin catalog") @@ -357,6 +357,183 @@ fn disabled_modifier_wrapping_a_terminator_is_rejected() { )); } +const IDENTIFIER_SOURCE: &str = r#"variables { + global: + 0: cakePos + player: + 1: playerScore +} + +subroutines { + 0: tick +} + +rule ("identifiers") { + event { Ongoing - Global; } + conditions { + Add(Event Player.playerScore, Global.cakePos) > 0; + } + actions { + Set Global Variable(cakePos, 1); + Global.cakePos = Vector(1, 2, 3); + Modify Global Variable(cakePos, Add, 1); + Event Player.playerScore = 5; + Set Player Variable(Event Player, playerScore, 7); + Call Subroutine(tick); + Set Global Variable(cakePos, Add(Event Player.playerScore, 1)); + } +} +"#; + +fn span_text(program: &Program, span: Span) -> &str { + let document = program.source(span.file).expect("source file"); + &document.text()[document.byte_range(span).expect("span range")] +} + +#[test] +fn declaration_and_use_identifiers_slice_to_the_recorded_text() { + let catalog = catalog(); + let locale = workshop_rs::catalog::Locale::new("en-US"); + let program = workshop_rs::parser::parse(IDENTIFIER_SOURCE, &catalog, &locale).expect("parses"); + + assert_eq!( + span_text(&program, program.global_variable_name_span(0).unwrap()), + "cakePos" + ); + assert_eq!( + span_text(&program, program.player_variable_name_span(0).unwrap()), + "playerScore" + ); + assert_eq!( + span_text(&program, program.subroutine_name_span(0).unwrap()), + "tick" + ); + + // Infix assignments record a target span: `Event Player.name` records the + // identifier itself, while `Global.name` records the qualified target. + assert_eq!( + span_text(&program, program.action_identifier_span(0, 3).unwrap()), + "playerScore" + ); + assert_eq!( + span_text(&program, program.action_identifier_span(0, 1).unwrap()), + "Global.cakePos" + ); + // Standard-form variable writes and `Call Subroutine` record no + // identifier span. + assert_eq!(program.action_identifier_span(0, 0), None); + assert_eq!(program.action_identifier_span(0, 2), None); + assert_eq!(program.action_identifier_span(0, 5), None); + + // A read nested inside another value is addressed by a path into the + // public value tree: `Add(Event Player.playerScore, 1)` argument 0. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 6, 0, &[0]).unwrap(), + ), + "playerScore" + ); + // The same spelling inside a condition resolves to the identifier. + assert_eq!( + span_text( + &program, + program.condition_value_span(0, 0, &[0, 0]).unwrap(), + ), + "playerScore" + ); + // A `Global.name` read records the `Global` keyword, not the identifier. + assert_eq!( + span_text( + &program, + program.condition_value_span(0, 0, &[0, 1]).unwrap(), + ), + "Global" + ); + + // An empty path addresses the argument or condition value itself. + assert_eq!( + program.action_argument_value_span(0, 6, 0, &[]), + program.action_argument_span(0, 6, 0) + ); + assert_eq!( + program.condition_value_span(0, 0, &[]), + program.condition_span(0, 0) + ); + // Positions outside the value tree carry no span. + assert_eq!( + program.action_argument_value_span(0, 6, 0, &[0, 0, 0]), + None + ); + assert_eq!(program.condition_value_span(0, 0, &[9]), None); +} + +#[test] +fn programs_without_source_carry_no_identifier_provenance() { + let mut program = Program::new(); + program + .global_variable(Variable::new("cakePos")) + .player_variable(Variable::new("playerScore")) + .subroutine(Subroutine::new("tick")) + .rule( + Rule::new("no source", Event::Global) + .condition(Condition::new(Value::call( + "add", + [ + Value::player_variable(Value::EventPlayer, "playerScore"), + Value::number(1.0), + ], + ))) + .action(Action::SetGlobalVariable { + variable: "cakePos".to_string(), + value: Value::number(1.0), + }) + .action(Action::CallSubroutine { + subroutine: "tick".to_string(), + }), + ); + + assert_eq!(program.global_variable_name_span(0), None); + assert_eq!(program.player_variable_name_span(0), None); + assert_eq!(program.subroutine_name_span(0), None); + assert_eq!(program.action_identifier_span(0, 0), None); + assert_eq!(program.action_identifier_span(0, 1), None); + assert_eq!(program.condition_value_span(0, 0, &[]), None); + assert_eq!(program.condition_value_span(0, 0, &[0]), None); + assert_eq!(program.action_argument_value_span(0, 0, 0, &[]), None); + assert_eq!(program.action_argument_value_span(0, 0, 0, &[0]), None); +} + +#[test] +fn identifier_provenance_drops_out_when_the_shape_changes() { + let catalog = catalog(); + let locale = workshop_rs::catalog::Locale::new("en-US"); + let mut program = + workshop_rs::parser::parse(IDENTIFIER_SOURCE, &catalog, &locale).expect("parses"); + + program.global_variable(Variable::new("extra")); + assert_eq!(program.global_variable_name_span(0), None); + + program.rules[0].actions.push(Action::End); + assert_eq!(program.action_identifier_span(0, 3), None); + assert_eq!(program.action_argument_value_span(0, 6, 0, &[0]), None); +} + +#[test] +fn attached_identifier_spans_take_priority_over_declaration_spans() { + let mut program = Program::new(); + program.global_variable(Variable::new("Score")); + let file = program.add_file(SourceFile::with_source("main.opy", "globalvar 0: Score\n")); + let declaration_span = Span::new(file, Position::new(1, 1), Position::new(1, 18)); + let name_span = Span::new(file, Position::new(1, 14), Position::new(1, 19)); + + program + .set_global_variable_spans(0, Some(declaration_span), Some(name_span)) + .expect("declaration spans attach"); + + assert_eq!(program.global_variable_name_span(0), Some(name_span)); +} + #[test] fn round_trip_equivalence_distinguishes_disabled_from_active() { let catalog = catalog(); diff --git a/docs/source-preservation.md b/docs/source-preservation.md index 7a5ad7d4..bf8c4bee 100644 --- a/docs/source-preservation.md +++ b/docs/source-preservation.md @@ -34,8 +34,31 @@ comments, whitespace, and mixed source structure are preserved. Parsed programs expose rule, condition, action, and direct action-argument spans through `Program::rule_span`, `Program::condition_span`, -`Program::action_span`, and `Program::action_argument_span`. Consumers that -construct a program can attach the same metadata with +`Program::action_span`, and `Program::action_argument_span`. Identifier and +nested-value provenance is exposed separately: + +- `Program::global_variable_name_span`, `Program::player_variable_name_span`, + and `Program::subroutine_name_span` return the span of a declared name: the + attached identifier span when a source-language provider supplies one, + otherwise the recorded declaration span, which raw parses record as the + name itself. +- `Program::action_identifier_span` returns the span recorded for the + variable or subroutine an action names — a set/modify/for target or a + `Call Subroutine` callee. +- `Program::condition_value_span` and `Program::action_argument_value_span` + return the span recorded for a value nested inside a condition or action + argument, addressed by a path into the public `Value` tree. + +Only the identifier spans the parser actually records are returned. Raw +Workshop records a target for `Global.name`/`Event Player.name` infix +assignments — `Global.name` covers the qualified name, `Event Player.name` +the identifier alone — while standard-form `Set`/`Modify` writes and +`Call Subroutine` record none. Variable reads record the declared name for +`Event Player.name`, bare-name, and `... At Index` argument spellings; +`Global.name` and `Global/Player Variable(...)` reads record the leading +keyword instead. + +Consumers that construct a program can attach the same metadata with `Program::set_rule_span`, `Program::set_condition_span`, `Program::set_action_span`, and `Program::set_action_argument_span`; declaration spans use the corresponding variable and subroutine methods. `Program::edit_source` @@ -126,7 +149,8 @@ The `node` tags and their keys are `rule` (`rule`), `condition` (`rule`, `{"line", "column"}`. Only nodes with an authored origin have an entry, so `spans` may be empty. Decoders ignore unknown members. The mapping granularity is rule, condition, action, direct action argument, and -declarations; nested value mappings are not part of the format. +declarations; nested-value and action-identifier mappings are not part of the +format. Columns follow `Position`: 1-based Unicode scalar values. Producers convert from other units, and editor presentation converts to UTF-16. From ee50ca2b12eedaf524dfd65e1dc1a90ea7e68d33 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 00:02:18 +0800 Subject: [PATCH 2/5] test: cover value-path traversal and recording boundaries in provenance tests Review findings: assert Vector component and PlayerVariable player child paths, the standard Set Player Variable boundary, disabled-action provenance, and out-of-range argument/path positions; note that the None assertions describe current parser coverage pending #325. --- crates/workshop-rs/tests/program_model.rs | 33 ++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index 39d28c4d..e70e08f1 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -381,6 +381,7 @@ rule ("identifiers") { Set Player Variable(Event Player, playerScore, 7); Call Subroutine(tick); Set Global Variable(cakePos, Add(Event Player.playerScore, 1)); + disabled Modify Global Variable(cakePos, Add, Event Player.playerScore); } } "#; @@ -420,10 +421,13 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { "Global.cakePos" ); // Standard-form variable writes and `Call Subroutine` record no - // identifier span. + // identifier span; these `None`s describe current parser coverage, not + // the eventual contract (#325 records the remaining spans). assert_eq!(program.action_identifier_span(0, 0), None); assert_eq!(program.action_identifier_span(0, 2), None); + assert_eq!(program.action_identifier_span(0, 4), None); assert_eq!(program.action_identifier_span(0, 5), None); + assert_eq!(program.action_identifier_span(0, 7), None); // A read nested inside another value is addressed by a path into the // public value tree: `Add(Event Player.playerScore, 1)` argument 0. @@ -434,6 +438,32 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { ), "playerScore" ); + // `PlayerVariable` exposes its player expression as child 0. + assert_eq!( + span_text( + &program, + program + .action_argument_value_span(0, 6, 0, &[0, 0]) + .unwrap(), + ), + "Event Player" + ); + // `Vector` components are addressed as children 0/1/2. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 1, 0, &[2]).unwrap(), + ), + "3" + ); + // A `disabled` action's provenance is that of the wrapped action. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 7, 0, &[]).unwrap(), + ), + "playerScore" + ); // The same spelling inside a condition resolves to the identifier. assert_eq!( span_text( @@ -465,6 +495,7 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { program.action_argument_value_span(0, 6, 0, &[0, 0, 0]), None ); + assert_eq!(program.action_argument_value_span(0, 6, 1, &[]), None); assert_eq!(program.condition_value_span(0, 0, &[9]), None); } From 657dcda6d75e816ac7a0fe443d489e8efdee9157 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 00:02:18 +0800 Subject: [PATCH 3/5] docs: include for-variable actions in unrecorded identifier coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding: for-variable actions were listed among the identifier-bearing forms but can never return a span — ForGlobalVariable's target_span is never populated and ForPlayerVariable has no field. --- crates/workshop-rs/src/program.rs | 4 ++-- docs/source-preservation.md | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index 6fc7a091..258a4048 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -383,8 +383,8 @@ impl Program { /// records a target span for `Global.name` and `Event Player.name` infix /// assignments — the `Global.name` form's span covers the qualified name /// including the `Global.` qualifier — while standard-form `Set`/`Modify` - /// variable actions and `Call Subroutine` carry no recorded identifier and - /// return `None`. Other action forms always return `None`. + /// variable actions, `For` variable loops, and `Call Subroutine` record + /// none. Other action forms always return `None`. pub fn action_identifier_span(&self, rule: usize, action: usize) -> Option { self.action_provenance(rule, action)?.identifier } diff --git a/docs/source-preservation.md b/docs/source-preservation.md index bf8c4bee..e64e3785 100644 --- a/docs/source-preservation.md +++ b/docs/source-preservation.md @@ -52,11 +52,11 @@ nested-value provenance is exposed separately: Only the identifier spans the parser actually records are returned. Raw Workshop records a target for `Global.name`/`Event Player.name` infix assignments — `Global.name` covers the qualified name, `Event Player.name` -the identifier alone — while standard-form `Set`/`Modify` writes and -`Call Subroutine` record none. Variable reads record the declared name for -`Event Player.name`, bare-name, and `... At Index` argument spellings; -`Global.name` and `Global/Player Variable(...)` reads record the leading -keyword instead. +the identifier alone — while standard-form `Set`/`Modify` writes, `For` +variable loops, and `Call Subroutine` record none. Variable reads record the +declared name for `Event Player.name`, bare-name, and `... At Index` argument +spellings; `Global.name` and `Global/Player Variable(...)` reads record the +leading keyword instead. Consumers that construct a program can attach the same metadata with `Program::set_rule_span`, `Program::set_condition_span`, From 368cb00ac0f9463b7014b9b2a241d5b64c78104d Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 00:28:21 +0800 Subject: [PATCH 4/5] feat(parser): record identifier spans for declarations, references, and calls The raw Workshop parser now records the declared identifier at every position that names a variable or subroutine: variable and subroutine declaration sections, Set/Modify/For write targets, Call Subroutine and Start Rule callees, Subroutine event bindings, indexed-write name arguments, and reads written as Global.name, Global/Player Variable(name), Event Player.name, or a bare declared name. ValueNode carries a dedicated identifier slot so a node's own span keeps its expression meaning; Event::Subroutine becomes a struct variant with name_span; For Player Variable gains target_span. The public provenance accessors return the recorded identifier for references, and the WIR validator now checks every recorded span so the every-span-is-valid invariant covers identifier fields too. Fixes #325 --- crates/workshop-rs/src/actions/parser.rs | 108 ++++++++++------- crates/workshop-rs/src/events/parser.rs | 36 +++--- crates/workshop-rs/src/events/validate.rs | 2 +- crates/workshop-rs/src/frontend/parser.rs | 69 +++++------ crates/workshop-rs/src/output/roundtrip.rs | 5 +- crates/workshop-rs/src/program.rs | 75 ++++++++---- crates/workshop-rs/src/rules/emitter.rs | 2 +- crates/workshop-rs/src/rules/parser.rs | 6 +- crates/workshop-rs/src/values/parser.rs | 134 ++++++++++++++------- crates/workshop-rs/src/wir/action.rs | 1 + crates/workshop-rs/src/wir/dump.rs | 3 +- crates/workshop-rs/src/wir/event.rs | 8 +- crates/workshop-rs/src/wir/rule.rs | 2 +- crates/workshop-rs/src/wir/validate.rs | 28 ++++- crates/workshop-rs/src/wir/value.rs | 15 ++- crates/workshop-rs/tests/parser.rs | 2 +- crates/workshop-rs/tests/program_model.rs | 105 +++++++++++++--- docs/source-preservation.md | 24 ++-- 18 files changed, 420 insertions(+), 205 deletions(-) diff --git a/crates/workshop-rs/src/actions/parser.rs b/crates/workshop-rs/src/actions/parser.rs index 8238858d..3821bd8a 100644 --- a/crates/workshop-rs/src/actions/parser.rs +++ b/crates/workshop-rs/src/actions/parser.rs @@ -257,7 +257,7 @@ impl ParseContext<'_> { if matches!(canonical_keyword(&first), "Global" | "global") { self.pos += 1; self.expect(TokenKind::Dot, "expected '.' after 'Global'")?; - let (name, _, target_end) = self.phrase()?; + let (name, name_start, target_end) = self.phrase()?; if matches!( self.peek().map(|token| token.kind), Some(TokenKind::LBracket) @@ -275,10 +275,17 @@ impl ParseContext<'_> { ) })?; let variable = self.global_by_name(&name)?; - let target = self.target.values.push(ValueNode::new( - Value::GlobalVariable(variable), - Some(Span::new(self.file(), start, target_end)), - )); + let target = self.target.values.push( + ValueNode::new( + Value::GlobalVariable(variable), + Some(Span::new(self.file(), start, target_end)), + ) + .with_identifier(Some(Span::new( + self.file(), + name_start, + target_end, + ))), + ); let value = self.value()?; self.expect(TokenKind::Semi, "expected ';' after indexed assignment")?; return Ok(Some(self.indexed_assignment_action( @@ -292,7 +299,7 @@ impl ParseContext<'_> { let value = self.value()?; self.expect(TokenKind::Semi, "expected ';' after assignment")?; let span = Some(Span::new(self.file(), start, self.previous_span().1)); - let target_span = Some(Span::new(self.file(), start, target_end)); + let target_span = Some(Span::new(self.file(), name_start, target_end)); return Ok(Some(self.target.actions.push(match operator { AssignmentOperator::Set => Action::SetGlobalVariable { variable, @@ -361,13 +368,20 @@ impl ParseContext<'_> { })?; let value = self.value()?; self.expect(TokenKind::Semi, "expected ';' after indexed assignment")?; - let variable_value = self.target.values.push(ValueNode::new( - Value::PlayerVariable { - player: event_player, - variable, - }, - Some(Span::new(self.file(), target_start, target_end)), - )); + let variable_value = self.target.values.push( + ValueNode::new( + Value::PlayerVariable { + player: event_player, + variable, + }, + Some(Span::new(self.file(), target_start, target_end)), + ) + .with_identifier(Some(Span::new( + self.file(), + target_start, + target_end, + ))), + ); return Ok(Some(self.indexed_assignment_action( false, variable_value, @@ -618,7 +632,7 @@ impl ParseContext<'_> { TokenKind::LParen, "expected '(' after 'For Global Variable'", )?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.global_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after loop variable")?; let start_value = self.value()?; @@ -645,7 +659,7 @@ impl ParseContext<'_> { step, body, span: Some(Span::new(self.file(), start, end_span.1)), - target_span: None, + target_span: Some(Span::new(self.file(), name_start, name_end)), }; Ok(self.target.actions.push(action)) } @@ -683,7 +697,7 @@ impl ParseContext<'_> { )?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after loop player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after loop variable")?; let start_value = self.value()?; @@ -711,6 +725,7 @@ impl ParseContext<'_> { step, body, span: Some(Span::new(self.file(), start, end_span.1)), + target_span: Some(Span::new(self.file(), name_start, name_end)), }; Ok(self.target.actions.push(action)) } @@ -731,7 +746,7 @@ impl ParseContext<'_> { TokenKind::LParen, "expected '(' after 'Set Global Variable'", )?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.global_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after variable")?; let value = self.value()?; @@ -741,12 +756,12 @@ impl ParseContext<'_> { variable, value, span: Some(Span::new(self.file(), start, end)), - target_span: None, + target_span: Some(Span::new(self.file(), name_start, name_end)), })) } "modifyGlobalVariable" => { self.expect(TokenKind::LParen, "expected '('")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.global_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after variable")?; let op = self.modify_op()?; @@ -759,14 +774,14 @@ impl ParseContext<'_> { op, value, span: Some(Span::new(self.file(), start, end)), - target_span: None, + target_span: Some(Span::new(self.file(), name_start, name_end)), })) } "setPlayerVariable" => { self.expect(TokenKind::LParen, "expected '('")?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after variable")?; let value = self.value()?; @@ -777,14 +792,14 @@ impl ParseContext<'_> { variable, value, span: Some(Span::new(self.file(), start, end)), - target_span: None, + target_span: Some(Span::new(self.file(), name_start, name_end)), })) } "modifyPlayerVariable" => { self.expect(TokenKind::LParen, "expected '('")?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after variable")?; let op = self.modify_op()?; @@ -798,21 +813,21 @@ impl ParseContext<'_> { op, value, span: Some(Span::new(self.file(), start, end)), - target_span: None, + target_span: Some(Span::new(self.file(), name_start, name_end)), })) } "forGlobalVariable" => self.for_group(), "forPlayerVariable" => self.for_player_group(), "callSubroutine" => { self.expect(TokenKind::LParen, "expected '('")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let subroutine = self.subroutine_by_name(&name)?; self.expect(TokenKind::RParen, "expected ')'")?; self.expect(TokenKind::Semi, "expected ';'")?; Ok(self.target.actions.push(Action::CallSubroutine { subroutine, span: Some(Span::new(self.file(), start, end)), - callee_span: None, + callee_span: Some(Span::new(self.file(), name_start, name_end)), })) } other => Err(WorkshopError::Unsupported { @@ -847,13 +862,19 @@ impl ParseContext<'_> { self.expect(TokenKind::LParen, "expected '('")?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; + let name_span = Some(Span::new(self.file(), name_start, name_end)); let mut args = Vec::with_capacity(4); - args.push(self.target.values.push(wir::ValueNode::new( - wir::Value::PlayerVariable { player, variable }, - None, - ))); + args.push( + self.target.values.push( + wir::ValueNode::new( + wir::Value::PlayerVariable { player, variable }, + None, + ) + .with_identifier(name_span), + ), + ); // The remaining arguments sit at overall argument // indexes 2.. (player and name consumed indexes 0-1), // so the signature context resolves their expected @@ -896,7 +917,7 @@ impl ParseContext<'_> { } "startRule" => { self.expect(TokenKind::LParen, "expected '('")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let subroutine = self.subroutine_by_name(&name)?; self.expect(TokenKind::Comma, "expected ',' after subroutine")?; let saved = self.expected_domain; @@ -905,10 +926,11 @@ impl ParseContext<'_> { self.expected_domain = saved; self.expect(TokenKind::RParen, "expected ')'")?; self.expect(TokenKind::Semi, "expected ';' after action")?; - let subroutine_value = self - .target - .values - .push(ValueNode::new(Value::Subroutine(subroutine), None)); + let name_span = Some(Span::new(self.file(), name_start, name_end)); + let subroutine_value = self.target.values.push( + ValueNode::new(Value::Subroutine(subroutine), name_span) + .with_identifier(name_span), + ); return Ok(self.target.actions.push(Action::Call { name: action.id.clone(), args: vec![subroutine_value, behavior], @@ -919,14 +941,18 @@ impl ParseContext<'_> { self.expect(TokenKind::LParen, "expected '('")?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; self.expect(TokenKind::RParen, "expected ')'")?; self.expect(TokenKind::Semi, "expected ';' after action")?; - let player_variable = self.target.values.push(ValueNode::new( - Value::PlayerVariable { player, variable }, - None, - )); + let player_variable = self.target.values.push( + ValueNode::new(Value::PlayerVariable { player, variable }, None) + .with_identifier(Some(Span::new( + self.file(), + name_start, + name_end, + ))), + ); return Ok(self.target.actions.push(Action::Call { name: action.id.clone(), args: vec![player_variable], diff --git a/crates/workshop-rs/src/events/parser.rs b/crates/workshop-rs/src/events/parser.rs index 9f1d3431..00e272a2 100644 --- a/crates/workshop-rs/src/events/parser.rs +++ b/crates/workshop-rs/src/events/parser.rs @@ -4,7 +4,7 @@ impl ParseContext<'_> { pub(crate) fn event_section(&mut self) -> Result { self.expect_keyword("event")?; self.expect(TokenKind::LBrace, "expected '{' after 'event'")?; - let mut lines: Vec = Vec::new(); + let mut lines: Vec<(String, Option)> = Vec::new(); loop { match self.peek() { Some(Token { @@ -19,16 +19,16 @@ impl ParseContext<'_> { .. }) => { self.pos += 1; - lines.push(String::new()); + lines.push((String::new(), None)); } Some(_) => { - let text = self.line_text()?; - lines.push(text); + let (text, span) = self.line_text()?; + lines.push((text, span)); } None => return Err(self.malformed("unexpected end of input in event", self.eof())), } } - let Some(name_line) = lines.first().cloned() else { + let Some((name_line, _)) = lines.first() else { return Err(self.malformed("event section is empty", self.previous())); }; let name_line = name_line.trim(); @@ -43,13 +43,13 @@ impl ParseContext<'_> { })?; match entry.id.as_str() { "global" => { - if lines[1..].iter().any(|line| !line.trim().is_empty()) { + if lines[1..].iter().any(|(line, _)| !line.trim().is_empty()) { return Err(self.unsupported_event_parameters("global")); } Ok(Event::Global) } "eachPlayer" => { - if lines[1..].iter().all(|line| line.trim().is_empty()) { + if lines[1..].iter().all(|(line, _)| line.trim().is_empty()) { return Ok(Event::EachPlayer); } let (team, target) = self.event_filters(&lines, "eachPlayer", true)?; @@ -75,18 +75,21 @@ impl ParseContext<'_> { .get(2..) .unwrap_or(&[]) .iter() - .any(|line| !line.trim().is_empty()) + .any(|(line, _)| !line.trim().is_empty()) { return Err(self.unsupported_event_parameters("subroutine")); } - let Some(sub_name) = lines.get(1).map(|s| s.trim()) else { + let Some((sub_name, sub_span)) = lines.get(1) else { return Err(self.malformed( "subroutine event requires a subroutine name", self.previous(), )); }; - let id = self.subroutine_by_name(sub_name)?; - Ok(Event::Subroutine(id)) + let id = self.subroutine_by_name(sub_name.trim())?; + Ok(Event::Subroutine { + subroutine: id, + name_span: *sub_span, + }) } other => Err(WorkshopError::Unsupported { message: format!("unsupported event '{other}'"), @@ -95,21 +98,24 @@ impl ParseContext<'_> { } } - pub(crate) fn player_event(&self, lines: &[String], kind: PlayerEventKind) -> Result { + pub(crate) fn player_event( + &self, + lines: &[(String, Option)], + kind: PlayerEventKind, + ) -> Result { let (team, target) = self.event_filters(lines, kind.catalog_id(), false)?; Ok(Event::Player { kind, team, target }) } pub(crate) fn event_filters( &self, - lines: &[String], + lines: &[(String, Option)], event_id: &str, allow_empty: bool, ) -> Result<(EventTeam, EventTarget)> { let parameters: Vec<&str> = lines[1..] .iter() - .map(String::as_str) - .map(str::trim) + .map(|(line, _)| line.trim()) .filter(|line| !line.is_empty()) .collect(); if parameters.is_empty() { diff --git a/crates/workshop-rs/src/events/validate.rs b/crates/workshop-rs/src/events/validate.rs index 91ddfc51..324533d2 100644 --- a/crates/workshop-rs/src/events/validate.rs +++ b/crates/workshop-rs/src/events/validate.rs @@ -12,7 +12,7 @@ pub(crate) fn validate_event( wir::Event::EachPlayer => ("eachPlayer", None), wir::Event::EachPlayerWithFilters { team, target } => ("eachPlayer", Some((*team, target))), wir::Event::Player { kind, team, target } => (kind.catalog_id(), Some((*team, target))), - wir::Event::Subroutine(_) => ("subroutine", None), + wir::Event::Subroutine { .. } => ("subroutine", None), }; if catalog.entry(Kind::Event, id).is_none() { return Err(WorkshopError::Unknown { diff --git a/crates/workshop-rs/src/frontend/parser.rs b/crates/workshop-rs/src/frontend/parser.rs index 39518384..62a33174 100644 --- a/crates/workshop-rs/src/frontend/parser.rs +++ b/crates/workshop-rs/src/frontend/parser.rs @@ -364,9 +364,12 @@ impl<'a> ParseContext<'a> { } /// Read a text line (tokens until `;`), joining words and dashes into - /// the literal text, and consume the terminating `;`. - pub(crate) fn line_text(&mut self) -> Result { + /// the literal text, and consume the terminating `;`. Returns the text + /// and the span covering the line's content tokens. + pub(crate) fn line_text(&mut self) -> Result<(String, Option)> { let mut parts = Vec::new(); + let mut start = None; + let mut end = None; loop { match self.peek() { Some(Token { @@ -376,50 +379,34 @@ impl<'a> ParseContext<'a> { self.pos += 1; break; } - Some(Token { - kind: TokenKind::Word(word), - .. - }) => { - parts.push(word.clone()); - self.pos += 1; - } - Some(Token { - kind: TokenKind::Op(op), - .. - }) if op == "-" => { - parts.push("-".to_string()); - self.pos += 1; - } - Some(Token { - kind: TokenKind::Number { value, .. }, - .. - }) => { - parts.push(value.to_string()); - self.pos += 1; - } - Some(Token { - kind: TokenKind::Dot, - .. - }) => { - parts.push(".".to_string()); + Some(token) => { + start.get_or_insert(token.start); + end = Some(token.end); + match &token.kind { + TokenKind::Word(word) => parts.push(word.clone()), + TokenKind::Op(op) if op == "-" => parts.push("-".to_string()), + TokenKind::Number { value, .. } => parts.push(value.to_string()), + TokenKind::Dot => parts.push(".".to_string()), + TokenKind::Colon => parts.push(":".to_string()), + _ => return Err(self.malformed("expected a text line", &token)), + } self.pos += 1; } - Some(Token { - kind: TokenKind::Colon, - .. - }) => { - parts.push(":".to_string()); - self.pos += 1; + None => { + return Err(self.malformed("unexpected end of input in line", self.eof())); } - Some(token) => return Err(self.malformed("expected a text line", &token)), - None => return Err(self.malformed("unexpected end of input in line", self.eof())), } } - Ok(parts - .join(" ") - .replace(" .", ".") - .replace(". ", ".") - .replace(" : ", ":")) + Ok(( + parts + .join(" ") + .replace(" .", ".") + .replace(". ", ".") + .replace(" : ", ":"), + start + .zip(end) + .map(|(start, end)| Span::new(self.file(), start, end)), + )) } /// Consume a known keyword phrase, verifying its spelling. diff --git a/crates/workshop-rs/src/output/roundtrip.rs b/crates/workshop-rs/src/output/roundtrip.rs index c8ec2785..051af237 100644 --- a/crates/workshop-rs/src/output/roundtrip.rs +++ b/crates/workshop-rs/src/output/roundtrip.rs @@ -358,7 +358,10 @@ fn event_equivalent( target: target_b, }, ) => kind_a == kind_b && team_a == team_b && target_a == target_b, - (wir::Event::Subroutine(sa), wir::Event::Subroutine(sb)) => { + ( + wir::Event::Subroutine { subroutine: sa, .. }, + wir::Event::Subroutine { subroutine: sb, .. }, + ) => { let name_a = a.subroutines.get(*sa).map(|s| s.name.as_str()); let name_b = b.subroutines.get(*sb).map(|s| s.name.as_str()); name_a == name_b diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index 258a4048..6b25132a 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -39,6 +39,8 @@ struct DeclarationProvenance { #[derive(Debug, Clone, Default)] struct RuleProvenance { span: Option, + /// The recorded span of the subroutine name a `Subroutine` event binds. + event_name: Option, conditions: Vec, actions: Vec, } @@ -57,6 +59,9 @@ struct ActionProvenance { #[derive(Debug, Clone, Default)] struct ValueProvenance { span: Option, + /// The recorded span of the variable or subroutine identifier the value + /// names, when the value is such a reference. + identifier: Option, children: Vec, } @@ -379,16 +384,21 @@ impl Program { /// public action names: the target of set/modify and for-variable actions, /// or the callee of a [`Call Subroutine`](Action::CallSubroutine) action. /// - /// Only the spans the parser actually recorded are returned: raw Workshop - /// records a target span for `Global.name` and `Event Player.name` infix - /// assignments — the `Global.name` form's span covers the qualified name - /// including the `Global.` qualifier — while standard-form `Set`/`Modify` - /// variable actions, `For` variable loops, and `Call Subroutine` record - /// none. Other action forms always return `None`. + /// Raw Workshop parses record the variable name for `Set`/`Modify` + /// variable actions, `For` variable loops, `Global.name`/`Event + /// Player.name` infix assignments, and indexed writes, and the callee name + /// for `Call Subroutine`. Other action forms always return `None`. pub fn action_identifier_span(&self, rule: usize, action: usize) -> Option { self.action_provenance(rule, action)?.identifier } + /// Return the span recorded for the subroutine name a rule's `Subroutine` + /// event binding names, or `None` for other event kinds and when no + /// provenance was recorded. + pub fn rule_event_name_span(&self, rule: usize) -> Option { + self.rule_provenance(rule)?.event_name + } + /// Return the authored span of a value nested inside a public rule /// condition. /// @@ -398,11 +408,11 @@ impl Program { /// `Value::PlayerVariable` player at `0`. An empty path returns the /// condition value's own span, matching [`condition_span`](Self::condition_span). /// - /// For a variable or subroutine reference the returned span is whatever - /// the parser recorded for that node: raw Workshop records the declared - /// name for `Event Player.name`, bare-name, and `... At Index` argument - /// spellings, while `Global.name` and `Global/Player Variable(...)` reads - /// span their leading keyword rather than the identifier. + /// For a variable or subroutine reference the identifier span is returned + /// when the parser recorded one: raw Workshop records the variable name + /// for `Global.name`, `Global/Player Variable(name)`, `Event Player.name`, + /// bare-name, and `... At Index` argument spellings. Other nodes return + /// the span recorded for the node itself. pub fn condition_value_span( &self, rule: usize, @@ -413,7 +423,7 @@ impl Program { for &index in path { value = value.children.get(index)?; } - value.span + value.identifier.or(value.span) } /// Return the authored span of a value nested inside a direct value @@ -422,7 +432,9 @@ impl Program { /// `argument` selects the same direct argument as /// [`action_argument_span`](Self::action_argument_span) and `path` walks /// into it the way [`condition_value_span`](Self::condition_value_span) - /// describes; an empty path returns the argument's own span. + /// describes; an empty path returns the argument's own span. Like + /// `condition_value_span`, a variable or subroutine reference returns its + /// recorded identifier span. pub fn action_argument_value_span( &self, rule: usize, @@ -437,7 +449,7 @@ impl Program { for &index in path { value = value.children.get(index)?; } - value.span + value.identifier.or(value.span) } /// Create a checked source edit through the authored source attached to @@ -646,6 +658,10 @@ impl Program { public_actions(&storage, *action, &mut actions)?; public_action_provenance(&storage, *action, &mut action_provenance)?; } + let event_name = match &rule.event { + wir::Event::Subroutine { name_span, .. } => *name_span, + _ => None, + }; program .provenance .as_mut() @@ -653,6 +669,7 @@ impl Program { .rules .push(RuleProvenance { span: rule.span, + event_name, conditions: rule .conditions .iter() @@ -728,7 +745,12 @@ impl Program { } for (rule_index, rule) in self.rules.iter().enumerate() { - let event = wir_event(&rule.event, &subroutines)?; + let mut event = wir_event(&rule.event, &subroutines)?; + if let wir::Event::Subroutine { name_span, .. } = &mut event { + *name_span = self + .rule_provenance(rule_index) + .and_then(|provenance| provenance.event_name); + } let conditions = rule .conditions .iter() @@ -815,11 +837,11 @@ fn public_event(storage: &wir::Program, event: &wir::Event) -> Result { team: public_team(*team), target: public_target(target), }, - wir::Event::Subroutine(id) => Event::Subroutine( + wir::Event::Subroutine { subroutine, .. } => Event::Subroutine( storage .subroutines - .get(*id) - .ok_or_else(|| malformed_id("subroutine", id.index()))? + .get(*subroutine) + .ok_or_else(|| malformed_id("subroutine", subroutine.index()))? .name .clone(), ), @@ -1393,6 +1415,7 @@ fn lower_actions( step, body, span: None, + target_span: None, })); } action => { @@ -1533,6 +1556,9 @@ fn apply_action_source(storage: &mut wir::Program, id: wir::ActionId, source: &A } | wir::Action::ForGlobalVariable { span, target_span, .. + } + | wir::Action::ForPlayerVariable { + span, target_span, .. } => { *span = source.span; *target_span = source.identifier; @@ -1546,7 +1572,6 @@ fn apply_action_source(storage: &mut wir::Program, id: wir::ActionId, source: &A wir::Action::AssignMember { span, .. } | wir::Action::If { span, .. } | wir::Action::While { span, .. } - | wir::Action::ForPlayerVariable { span, .. } | wir::Action::Disabled { span, .. } | wir::Action::Call { span, .. } => *span = source.span, } @@ -1595,7 +1620,8 @@ fn action_identifier(action: &wir::Action) -> Option { | wir::Action::ModifyGlobalVariable { target_span, .. } | wir::Action::SetPlayerVariable { target_span, .. } | wir::Action::ModifyPlayerVariable { target_span, .. } - | wir::Action::ForGlobalVariable { target_span, .. } => *target_span, + | wir::Action::ForGlobalVariable { target_span, .. } + | wir::Action::ForPlayerVariable { target_span, .. } => *target_span, wir::Action::CallSubroutine { callee_span, .. } => *callee_span, _ => None, } @@ -1609,6 +1635,7 @@ fn value_provenance(storage: &wir::Program, id: wir::ValueId) -> ValueProvenance }; ValueProvenance { span: node.span, + identifier: node.identifier, children: wir_value_children(&node.value) .into_iter() .map(|child| value_provenance(storage, child)) @@ -1640,6 +1667,7 @@ fn apply_value_provenance( let children = wir_value_children(&node.value); if let Some(node) = storage.values.get_mut(value) { node.span = source.span; + node.identifier = source.identifier; } for (child, source) in children.into_iter().zip(&source.children) { apply_value_provenance(storage, child, source); @@ -1826,11 +1854,12 @@ fn wir_event( team: wir_team(*team), target: wir_target(target), }, - Event::Subroutine(name) => wir::Event::Subroutine( - *subroutines + Event::Subroutine(name) => wir::Event::Subroutine { + subroutine: *subroutines .get(name) .ok_or_else(|| unknown_name("subroutine", name))?, - ), + name_span: None, + }, }) } diff --git a/crates/workshop-rs/src/rules/emitter.rs b/crates/workshop-rs/src/rules/emitter.rs index 71ee8f7b..196ea23c 100644 --- a/crates/workshop-rs/src/rules/emitter.rs +++ b/crates/workshop-rs/src/rules/emitter.rs @@ -37,7 +37,7 @@ impl<'a> EmitContext<'a> { self.line(2, &format!("{spelling};"))?; self.event_filters(*team, target)?; } - wir::Event::Subroutine(subroutine) => { + wir::Event::Subroutine { subroutine, .. } => { let spelling = self.spelling(Kind::Event, "subroutine")?; self.line(2, &format!("{spelling};"))?; let name = self diff --git a/crates/workshop-rs/src/rules/parser.rs b/crates/workshop-rs/src/rules/parser.rs index d80d427e..e5120053 100644 --- a/crates/workshop-rs/src/rules/parser.rs +++ b/crates/workshop-rs/src/rules/parser.rs @@ -135,9 +135,7 @@ impl ParseContext<'_> { } else { span }), - // Workshop-text sources carry no `.opy` identifier provenance; - // exact rename occurrences are only produced by the native path. - name_span: None, + name_span: Some(name_span), }) } @@ -162,7 +160,7 @@ impl ParseContext<'_> { name, index, span: Some(Span::new(self.file(), start, end)), - name_span: None, + name_span: Some(Span::new(self.file(), start, end)), }); self.subroutines .insert(self.target.subroutines.get(id).unwrap().name.clone(), id); diff --git a/crates/workshop-rs/src/values/parser.rs b/crates/workshop-rs/src/values/parser.rs index 07a4af5d..f9fa540b 100644 --- a/crates/workshop-rs/src/values/parser.rs +++ b/crates/workshop-rs/src/values/parser.rs @@ -237,24 +237,25 @@ impl ParseContext<'_> { { self.pos += 1; self.expect(TokenKind::LParen, "expected '(' after 'Global Variable'")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.global_by_name(&name)?; self.expect(TokenKind::RParen, "expected ')' after Global Variable")?; let span = Some(Span::new(self.file(), start, end)); - return Ok(self - .target - .values - .push(ValueNode::new(Value::GlobalVariable(variable), span))); + return Ok(self.target.values.push( + ValueNode::new(Value::GlobalVariable(variable), span).with_identifier( + Some(Span::new(self.file(), name_start, name_end)), + ), + )); } } self.expect(TokenKind::Dot, "expected '.' after 'Global'")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.global_by_name(&name)?; let span = Some(Span::new(self.file(), start, end)); - Ok(self - .target - .values - .push(ValueNode::new(Value::GlobalVariable(variable), span))) + Ok(self.target.values.push( + ValueNode::new(Value::GlobalVariable(variable), span) + .with_identifier(Some(Span::new(self.file(), name_start, name_end))), + )) } Some(Token { kind: TokenKind::Word(word), @@ -280,13 +281,20 @@ impl ParseContext<'_> { self.expect(TokenKind::LParen, "expected '(' after 'Player Variable'")?; let player = self.value()?; self.expect(TokenKind::Comma, "expected ',' after player")?; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; self.expect(TokenKind::RParen, "expected ')' after Player Variable")?; - Ok(self.target.values.push(ValueNode::new( - Value::PlayerVariable { player, variable }, - Some(Span::new(self.file(), start, end)), - ))) + Ok(self.target.values.push( + ValueNode::new( + Value::PlayerVariable { player, variable }, + Some(Span::new(self.file(), start, end)), + ) + .with_identifier(Some(Span::new( + self.file(), + name_start, + name_end, + ))), + )) })(); self.expected_domain = saved; result @@ -312,10 +320,17 @@ impl ParseContext<'_> { self.pos += 1; let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; - return Ok(self.target.values.push(ValueNode::new( - Value::PlayerVariable { player, variable }, - Some(Span::new(self.file(), name_start, name_end)), - ))); + return Ok(self.target.values.push( + ValueNode::new( + Value::PlayerVariable { player, variable }, + Some(Span::new(self.file(), name_start, name_end)), + ) + .with_identifier(Some(Span::new( + self.file(), + name_start, + name_end, + ))), + )); } return Ok(player); } @@ -337,10 +352,17 @@ impl ParseContext<'_> { self.pos += 1; let (name, name_start, name_end) = self.phrase()?; let variable = self.player_by_name(&name)?; - return Ok(self.target.values.push(ValueNode::new( - Value::PlayerVariable { player, variable }, - Some(Span::new(self.file(), name_start, name_end)), - ))); + return Ok(self.target.values.push( + ValueNode::new( + Value::PlayerVariable { player, variable }, + Some(Span::new(self.file(), name_start, name_end)), + ) + .with_identifier(Some(Span::new( + self.file(), + name_start, + name_end, + ))), + )); } return Ok(player); } @@ -422,7 +444,7 @@ impl ParseContext<'_> { }) = self.peek() { self.pos += 1; - let (name, _, _) = self.phrase()?; + let (name, name_start, name_end) = self.phrase()?; if matches!( self.target.values.get(inner), Some(ValueNode { @@ -436,13 +458,20 @@ impl ParseContext<'_> { .get(&name) .copied() .unwrap_or(self.player_by_name(&name)?); - Ok(self.target.values.push(ValueNode::new( - Value::PlayerVariable { - player: inner, - variable, - }, - None, - ))) + Ok(self.target.values.push( + ValueNode::new( + Value::PlayerVariable { + player: inner, + variable, + }, + None, + ) + .with_identifier(Some(Span::new( + self.file(), + name_start, + name_end, + ))), + )) } else { let member = self .target @@ -729,10 +758,13 @@ impl ParseContext<'_> { _ => {} } if let Some(variable) = self.globals.get(phrase).copied() { - return Ok(self.target.values.push(ValueNode::new( - Value::GlobalVariable(variable), - Some(Span::new(self.file(), start, end)), - ))); + return Ok(self.target.values.push( + ValueNode::new( + Value::GlobalVariable(variable), + Some(Span::new(self.file(), start, end)), + ) + .with_identifier(Some(Span::new(self.file(), start, end))), + )); } let member = self .expected_domain @@ -899,10 +931,19 @@ impl ParseContext<'_> { { let (name, start, end) = self.phrase()?; let variable = self.global_by_name(&name)?; - args.push(self.target.values.push(ValueNode::new( - Value::GlobalVariable(variable), - Some(Span::new(self.file(), start, end)), - ))); + args.push( + self.target.values.push( + ValueNode::new( + Value::GlobalVariable(variable), + Some(Span::new(self.file(), start, end)), + ) + .with_identifier(Some(Span::new( + self.file(), + start, + end, + ))), + ), + ); } else if matches!( call_id, "setPlayerVariableAtIndex" | "modifyPlayerVariableAtIndex" @@ -912,10 +953,17 @@ impl ParseContext<'_> { let (name, start, end) = self.phrase()?; if let Some(variable) = self.players.get(&name).copied() { let player = args[0]; - args[0] = self.target.values.push(ValueNode::new( - Value::PlayerVariable { player, variable }, - Some(Span::new(self.file(), start, end)), - )); + args[0] = self.target.values.push( + ValueNode::new( + Value::PlayerVariable { player, variable }, + Some(Span::new(self.file(), start, end)), + ) + .with_identifier(Some(Span::new( + self.file(), + start, + end, + ))), + ); } else { self.pos = saved; let saved_domain = self.expected_domain; diff --git a/crates/workshop-rs/src/wir/action.rs b/crates/workshop-rs/src/wir/action.rs index 79e7db8c..4bd03fdb 100644 --- a/crates/workshop-rs/src/wir/action.rs +++ b/crates/workshop-rs/src/wir/action.rs @@ -80,6 +80,7 @@ pub(crate) enum Action { step: ValueId, body: Vec, span: Option, + target_span: Option, }, /// An action carrying the `disabled` modifier: the wrapped action stays /// in the program but does not execute. For a control-flow group the diff --git a/crates/workshop-rs/src/wir/dump.rs b/crates/workshop-rs/src/wir/dump.rs index c9ab057e..49b9f046 100644 --- a/crates/workshop-rs/src/wir/dump.rs +++ b/crates/workshop-rs/src/wir/dump.rs @@ -80,7 +80,7 @@ fn render_event(program: &Program, event: &Event, out: &mut String, level: usize event_team_name(*team), event_target_name(target) )), - Event::Subroutine(subroutine) => { + Event::Subroutine { subroutine, .. } => { let name = program .subroutines .get(*subroutine) @@ -299,6 +299,7 @@ fn render_action(program: &Program, id: super::ActionId, out: &mut String, level step, body, span, + .. } => { out.push_str(&format!("{}forPlayerVariable ", indent(level))); render_value(program, *player, out); diff --git a/crates/workshop-rs/src/wir/event.rs b/crates/workshop-rs/src/wir/event.rs index 7fcfe108..94db2aa5 100644 --- a/crates/workshop-rs/src/wir/event.rs +++ b/crates/workshop-rs/src/wir/event.rs @@ -1,5 +1,7 @@ //! Canonical Workshop event forms and filters. +use crate::core::source::Span; + use super::SubroutineId; /// The team filter attached to a player-scoped Workshop event. @@ -72,5 +74,9 @@ pub(crate) enum Event { target: EventTarget, }, /// A subroutine body (`def name():`), referencing the subroutine. - Subroutine(SubroutineId), + Subroutine { + subroutine: SubroutineId, + /// The span of the subroutine name in the event section. + name_span: Option, + }, } diff --git a/crates/workshop-rs/src/wir/rule.rs b/crates/workshop-rs/src/wir/rule.rs index 150bffb1..8f9b4710 100644 --- a/crates/workshop-rs/src/wir/rule.rs +++ b/crates/workshop-rs/src/wir/rule.rs @@ -38,7 +38,7 @@ pub(crate) struct Condition { pub(crate) struct Rule { pub(crate) name: String, pub(crate) span: Option, - #[allow(dead_code)] + /// The exact span of the rule name string, when recorded. pub(crate) name_span: Option, pub(crate) disabled: bool, pub(crate) event: Event, diff --git a/crates/workshop-rs/src/wir/validate.rs b/crates/workshop-rs/src/wir/validate.rs index d36eb504..698b0fe9 100644 --- a/crates/workshop-rs/src/wir/validate.rs +++ b/crates/workshop-rs/src/wir/validate.rs @@ -14,12 +14,15 @@ pub(crate) fn validate(program: &Program) -> Result<(), IrError> { } for variable in program.global_variables.iter() { check_span(variable.span, program)?; + check_span(variable.name_span, program)?; } for variable in program.player_variables.iter() { check_span(variable.span, program)?; + check_span(variable.name_span, program)?; } for subroutine in program.subroutines.iter() { check_span(subroutine.span, program)?; + check_span(subroutine.name_span, program)?; } for rule in program.rules.iter() { check_rule(program, rule)?; @@ -29,7 +32,13 @@ pub(crate) fn validate(program: &Program) -> Result<(), IrError> { fn check_rule(program: &Program, rule: &Rule) -> Result<(), IrError> { check_span(rule.span, program)?; - if let Event::Subroutine(subroutine) = &rule.event { + check_span(rule.name_span, program)?; + if let Event::Subroutine { + subroutine, + name_span, + } = &rule.event + { + check_span(*name_span, program)?; if !program.subroutines.contains(*subroutine) { return Err(dangling("subroutine", subroutine.index())); } @@ -62,6 +71,7 @@ fn check_action(program: &Program, id: super::ActionId) -> Result<(), IrError> { .get(id) .ok_or_else(|| dangling("action", id.index()))?; check_span(action.span(), program)?; + check_span(action_identifier_span(action), program)?; match action { Action::SetGlobalVariable { variable, value, .. @@ -196,6 +206,7 @@ fn check_value(program: &Program, id: super::ValueId) -> Result<(), IrError> { .get(id) .ok_or_else(|| dangling("value", id.index()))?; check_span(node.span, program)?; + check_span(node.identifier, program)?; let value = &node.value; match value { Value::Array(elements) => { @@ -240,6 +251,21 @@ fn check_value(program: &Program, id: super::ValueId) -> Result<(), IrError> { Ok(()) } +/// The recorded identifier span a WIR action carries, when its variant has +/// one. +fn action_identifier_span(action: &Action) -> Option { + match action { + Action::SetGlobalVariable { target_span, .. } + | Action::ModifyGlobalVariable { target_span, .. } + | Action::SetPlayerVariable { target_span, .. } + | Action::ModifyPlayerVariable { target_span, .. } + | Action::ForGlobalVariable { target_span, .. } + | Action::ForPlayerVariable { target_span, .. } => *target_span, + Action::CallSubroutine { callee_span, .. } => *callee_span, + _ => None, + } +} + fn check_span(span: Option, program: &Program) -> Result<(), IrError> { let Some(span) = span else { return Ok(()); diff --git a/crates/workshop-rs/src/wir/value.rs b/crates/workshop-rs/src/wir/value.rs index 5c1405df..edcfc5b9 100644 --- a/crates/workshop-rs/src/wir/value.rs +++ b/crates/workshop-rs/src/wir/value.rs @@ -9,6 +9,9 @@ use super::{GlobalVarId, PlayerVarId, SubroutineId, ValueId}; pub(crate) struct ValueNode { pub(crate) value: Value, pub(crate) span: Option, + /// The span of the identifier a variable or subroutine reference names; + /// `None` for nodes that do not reference a declared identifier. + pub(crate) identifier: Option, } /// A workshop value (expression). @@ -55,6 +58,16 @@ pub(crate) enum Value { impl ValueNode { /// Build a value node with a source span. pub(crate) fn new(value: Value, span: Option) -> Self { - ValueNode { value, span } + ValueNode { + value, + span, + identifier: None, + } + } + + /// Record the identifier span a variable or subroutine reference names. + pub(crate) fn with_identifier(mut self, identifier: Option) -> Self { + self.identifier = identifier; + self } } diff --git a/crates/workshop-rs/tests/parser.rs b/crates/workshop-rs/tests/parser.rs index 642715fe..ce2e951c 100644 --- a/crates/workshop-rs/tests/parser.rs +++ b/crates/workshop-rs/tests/parser.rs @@ -336,7 +336,7 @@ fn parsed_events_are_canonical() { wir::Event::EachPlayer => "eachPlayer".to_string(), wir::Event::EachPlayerWithFilters { .. } => "eachPlayer".to_string(), wir::Event::Player { kind, .. } => kind.catalog_id().to_string(), - wir::Event::Subroutine(subroutine) => format!( + wir::Event::Subroutine { subroutine, .. } => format!( "subroutine:{}", program.subroutines.get(*subroutine).unwrap().name ), diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index e70e08f1..0c7d8918 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -382,6 +382,26 @@ rule ("identifiers") { Call Subroutine(tick); Set Global Variable(cakePos, Add(Event Player.playerScore, 1)); disabled Modify Global Variable(cakePos, Add, Event Player.playerScore); + Start Rule(tick, Restart Rule); + Set Global Variable At Index(cakePos, 0, 1); + Big Message(All Players(All Teams), Custom String("cakePos in a string is not a reference", cakePos)); + // cakePos inside a comment is not a reference either + For Global Variable(cakePos, 0, 10, 1); + Abort; + End; + For Player Variable(Event Player, playerScore, 0, 5, 1); + Abort; + End; + } +} + +rule ("subroutine runner") { + event { + Subroutine; + tick; + } + actions { + Call Subroutine(tick); } } "#; @@ -410,24 +430,32 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { "tick" ); - // Infix assignments record a target span: `Event Player.name` records the - // identifier itself, while `Global.name` records the qualified target. - assert_eq!( - span_text(&program, program.action_identifier_span(0, 3).unwrap()), - "playerScore" - ); + // Every variable write names its target identifier: standard-form + // `Set`/`Modify`, infix `Global.name`/`Event Player.name` assignments, + // `For` loops, and `Call Subroutine` callees. + for action in [0, 1, 2, 6, 7, 11] { + assert_eq!( + span_text(&program, program.action_identifier_span(0, action).unwrap()), + "cakePos", + "action {action}" + ); + } + for action in [3, 4, 14] { + assert_eq!( + span_text(&program, program.action_identifier_span(0, action).unwrap()), + "playerScore", + "action {action}" + ); + } assert_eq!( - span_text(&program, program.action_identifier_span(0, 1).unwrap()), - "Global.cakePos" + span_text(&program, program.action_identifier_span(0, 5).unwrap()), + "tick" ); - // Standard-form variable writes and `Call Subroutine` record no - // identifier span; these `None`s describe current parser coverage, not - // the eventual contract (#325 records the remaining spans). - assert_eq!(program.action_identifier_span(0, 0), None); - assert_eq!(program.action_identifier_span(0, 2), None); - assert_eq!(program.action_identifier_span(0, 4), None); - assert_eq!(program.action_identifier_span(0, 5), None); - assert_eq!(program.action_identifier_span(0, 7), None); + // Generic calls carry no action-level identifier; the subroutine or + // variable they name is provenance of a value argument instead. + assert_eq!(program.action_identifier_span(0, 8), None); + assert_eq!(program.action_identifier_span(0, 9), None); + assert_eq!(program.action_identifier_span(0, 10), None); // A read nested inside another value is addressed by a path into the // public value tree: `Add(Event Player.playerScore, 1)` argument 0. @@ -464,7 +492,7 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { ), "playerScore" ); - // The same spelling inside a condition resolves to the identifier. + // The same spellings inside a condition resolve to their identifiers. assert_eq!( span_text( &program, @@ -472,14 +500,52 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { ), "playerScore" ); - // A `Global.name` read records the `Global` keyword, not the identifier. assert_eq!( span_text( &program, program.condition_value_span(0, 0, &[0, 1]).unwrap(), ), - "Global" + "cakePos" + ); + // `Start Rule`'s subroutine argument and an `... At Index` variable name + // record their identifiers as value provenance. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 8, 0, &[]).unwrap(), + ), + "tick" + ); + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 9, 0, &[]).unwrap(), + ), + "cakePos" + ); + // A string literal carrying the same spelling is string provenance, not + // the identifier; the real argument next to it inside `Custom String` is. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 10, 1, &[0]).unwrap(), + ), + "\"cakePos in a string is not a reference\"" + ); + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 10, 1, &[1]).unwrap(), + ), + "cakePos" + ); + + // A `Subroutine` event binding records the bound name. + assert_eq!( + span_text(&program, program.rule_event_name_span(1).unwrap()), + "tick" ); + assert_eq!(program.rule_event_name_span(0), None); // An empty path addresses the argument or condition value itself. assert_eq!( @@ -527,6 +593,7 @@ fn programs_without_source_carry_no_identifier_provenance() { assert_eq!(program.global_variable_name_span(0), None); assert_eq!(program.player_variable_name_span(0), None); assert_eq!(program.subroutine_name_span(0), None); + assert_eq!(program.rule_event_name_span(0), None); assert_eq!(program.action_identifier_span(0, 0), None); assert_eq!(program.action_identifier_span(0, 1), None); assert_eq!(program.condition_value_span(0, 0, &[]), None); diff --git a/docs/source-preservation.md b/docs/source-preservation.md index e64e3785..da77f521 100644 --- a/docs/source-preservation.md +++ b/docs/source-preservation.md @@ -45,18 +45,22 @@ nested-value provenance is exposed separately: - `Program::action_identifier_span` returns the span recorded for the variable or subroutine an action names — a set/modify/for target or a `Call Subroutine` callee. +- `Program::rule_event_name_span` returns the span recorded for the + subroutine name a `Subroutine` event binding names. - `Program::condition_value_span` and `Program::action_argument_value_span` return the span recorded for a value nested inside a condition or action - argument, addressed by a path into the public `Value` tree. - -Only the identifier spans the parser actually records are returned. Raw -Workshop records a target for `Global.name`/`Event Player.name` infix -assignments — `Global.name` covers the qualified name, `Event Player.name` -the identifier alone — while standard-form `Set`/`Modify` writes, `For` -variable loops, and `Call Subroutine` record none. Variable reads record the -declared name for `Event Player.name`, bare-name, and `... At Index` argument -spellings; `Global.name` and `Global/Player Variable(...)` reads record the -leading keyword instead. + argument, addressed by a path into the public `Value` tree. For a variable + or subroutine reference the recorded identifier span is returned; other + nodes return the span recorded for the node itself. + +Raw Workshop parses record the declared identifier at every position that +names a variable or subroutine: `Set`/`Modify` standard forms and infix +`Global.name`/`Event Player.name` assignments, `For` variable loops, +`Call Subroutine` and `Start Rule` callees, `Subroutine` event bindings, +`... At Index` name arguments, and reads written as `Global.name`, +`Global/Player Variable(name)`, `Event Player.name`, or a bare declared name. +Positions with no recorded provenance return `None` rather than a neighboring +or enclosing span. Consumers that construct a program can attach the same metadata with `Program::set_rule_span`, `Program::set_condition_span`, From bb9bd9330e06644ad80291b3dd70d2aa2f6f5d01 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 01:15:47 +0800 Subject: [PATCH 5/5] fix(parser): correct For-group span starts and complete identifier provenance Address review findings on the identifier-provenance work: - `for_group`/`for_player_group` recorded `Variable` (the phrase's last word) as the action span start; group parsers now take the phrase start so `For`/`While`/`If` spans cover their keyword. - `wir::Rule.name_span` is populated (inside the `rule("name")` string token), carried through `RuleProvenance`, and exposed as `Program::rule_name_span`. - `Call Subroutine`/`Start Rule`/subroutine-event failures to resolve a name now report the recorded name span in the `Unknown` diagnostic. - `action_identifier_span` documentation no longer claims indexed writes; those names live on the lowered call's variable argument, and the source docs note `set_*` does not cover identifier-level provenance. - Drop redundant `While`/`For` value-provenance reapplication and simplify the declaration-line span branch that was vacuous. - Extend the identifier provenance test over indexed writes, `Global/Player Variable(name)` reads, parenthesized `Event Player` member reads, multi-word names, chase forms, and `If`/`Else If` condition reads. --- crates/workshop-rs/src/actions/parser.rs | 40 +++--- crates/workshop-rs/src/events/parser.rs | 2 +- crates/workshop-rs/src/program.rs | 61 ++++----- crates/workshop-rs/src/rules/parser.rs | 38 +++--- crates/workshop-rs/src/values/parser.rs | 6 +- crates/workshop-rs/tests/program_model.rs | 144 ++++++++++++++++++++++ docs/source-preservation.md | 15 ++- 7 files changed, 221 insertions(+), 85 deletions(-) diff --git a/crates/workshop-rs/src/actions/parser.rs b/crates/workshop-rs/src/actions/parser.rs index 3821bd8a..9878de67 100644 --- a/crates/workshop-rs/src/actions/parser.rs +++ b/crates/workshop-rs/src/actions/parser.rs @@ -104,12 +104,12 @@ impl ParseContext<'_> { if let Some(structural) = self.resolve_entry(Kind::Structural, rest) { match structural.id.as_str() { "while" => { - let group = self.while_group()?; + let group = self.while_group(self.previous_span().0)?; actions.push(self.disabled_action(group)); continue; } "if" => { - let group = self.if_group()?; + let group = self.if_group(self.previous_span().0)?; actions.push(self.disabled_action(group)); continue; } @@ -139,7 +139,7 @@ impl ParseContext<'_> { ) { self.pos += 1; - let group = self.while_group()?; + let group = self.while_group(self.previous_span().0)?; actions.push(self.disabled_action(group)); continue; } @@ -156,10 +156,10 @@ impl ParseContext<'_> { self.pos = saved; return Ok((actions, Stop::Else)); } - "if" => actions.push(self.if_group()?), - "forGlobalVariable" => actions.push(self.for_group()?), - "forPlayerVariable" => actions.push(self.for_player_group()?), - "while" => actions.push(self.while_group()?), + "if" => actions.push(self.if_group(start)?), + "forGlobalVariable" => actions.push(self.for_group(start)?), + "forPlayerVariable" => actions.push(self.for_player_group(start)?), + "while" => actions.push(self.while_group(start)?), "Loop" => actions.push(self.action_call_from_phrase(phrase, start, end)?), "Loop If Condition Is True" => { actions.push(self.action_call_from_phrase(phrase, start, end)?) @@ -568,8 +568,7 @@ impl ParseContext<'_> { })) } - pub(crate) fn if_group(&mut self) -> Result { - let start = self.previous_span().0; + pub(crate) fn if_group(&mut self, start: Position) -> Result { self.expect(TokenKind::LParen, "expected '(' after 'If'")?; let condition = self.value()?; self.expect(TokenKind::RParen, "expected ')' after If condition")?; @@ -626,8 +625,7 @@ impl ParseContext<'_> { Ok(self.target.actions.push(action)) } - pub(crate) fn for_group(&mut self) -> Result { - let start = self.previous_span().0; + pub(crate) fn for_group(&mut self, start: Position) -> Result { self.expect( TokenKind::LParen, "expected '(' after 'For Global Variable'", @@ -664,8 +662,7 @@ impl ParseContext<'_> { Ok(self.target.actions.push(action)) } - pub(crate) fn while_group(&mut self) -> Result { - let start = self.previous_span().0; + pub(crate) fn while_group(&mut self, start: Position) -> Result { self.expect(TokenKind::LParen, "expected '(' after 'While'")?; let condition = self.value()?; self.expect(TokenKind::RParen, "expected ')' after While condition")?; @@ -689,8 +686,7 @@ impl ParseContext<'_> { /// reference's per-player loop form (parsed from pinned reference /// evidence; the differential gate normalizes it to the declared global /// form, #119). - pub(crate) fn for_player_group(&mut self) -> Result { - let start = self.previous_span().0; + pub(crate) fn for_player_group(&mut self, start: Position) -> Result { self.expect( TokenKind::LParen, "expected '(' after 'For Player Variable'", @@ -816,12 +812,15 @@ impl ParseContext<'_> { target_span: Some(Span::new(self.file(), name_start, name_end)), })) } - "forGlobalVariable" => self.for_group(), - "forPlayerVariable" => self.for_player_group(), + "forGlobalVariable" => self.for_group(start), + "forPlayerVariable" => self.for_player_group(start), "callSubroutine" => { self.expect(TokenKind::LParen, "expected '('")?; let (name, name_start, name_end) = self.phrase()?; - let subroutine = self.subroutine_by_name(&name)?; + let subroutine = self.subroutine_by_name( + &name, + Some(Span::new(self.file(), name_start, name_end)), + )?; self.expect(TokenKind::RParen, "expected ')'")?; self.expect(TokenKind::Semi, "expected ';'")?; Ok(self.target.actions.push(Action::CallSubroutine { @@ -918,7 +917,10 @@ impl ParseContext<'_> { "startRule" => { self.expect(TokenKind::LParen, "expected '('")?; let (name, name_start, name_end) = self.phrase()?; - let subroutine = self.subroutine_by_name(&name)?; + let subroutine = self.subroutine_by_name( + &name, + Some(Span::new(self.file(), name_start, name_end)), + )?; self.expect(TokenKind::Comma, "expected ',' after subroutine")?; let saved = self.expected_domain; self.expected_domain = self.context.expected_domain(action.id.as_str(), 1); diff --git a/crates/workshop-rs/src/events/parser.rs b/crates/workshop-rs/src/events/parser.rs index 00e272a2..d9d261de 100644 --- a/crates/workshop-rs/src/events/parser.rs +++ b/crates/workshop-rs/src/events/parser.rs @@ -85,7 +85,7 @@ impl ParseContext<'_> { self.previous(), )); }; - let id = self.subroutine_by_name(sub_name.trim())?; + let id = self.subroutine_by_name(sub_name.trim(), *sub_span)?; Ok(Event::Subroutine { subroutine: id, name_span: *sub_span, diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index 6b25132a..ef3d3696 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -39,6 +39,8 @@ struct DeclarationProvenance { #[derive(Debug, Clone, Default)] struct RuleProvenance { span: Option, + /// The recorded span of the rule's quoted name in `rule("name")`. + name: Option, /// The recorded span of the subroutine name a `Subroutine` event binds. event_name: Option, conditions: Vec, @@ -385,13 +387,22 @@ impl Program { /// or the callee of a [`Call Subroutine`](Action::CallSubroutine) action. /// /// Raw Workshop parses record the variable name for `Set`/`Modify` - /// variable actions, `For` variable loops, `Global.name`/`Event - /// Player.name` infix assignments, and indexed writes, and the callee name - /// for `Call Subroutine`. Other action forms always return `None`. + /// variable actions, `For` variable loops, and `Global.name`/`Event + /// Player.name` infix assignments, and the callee name for `Call + /// Subroutine`. Indexed writes lower to `... Variable At Index` calls and + /// record the name on their variable argument — see + /// [`action_argument_value_span`](Self::action_argument_value_span). + /// Other action forms always return `None`. pub fn action_identifier_span(&self, rule: usize, action: usize) -> Option { self.action_provenance(rule, action)?.identifier } + /// Return the span recorded for a rule's name inside its `rule("name")` + /// string, or `None` when no provenance was recorded. + pub fn rule_name_span(&self, rule: usize) -> Option { + self.rule_provenance(rule)?.name + } + /// Return the span recorded for the subroutine name a rule's `Subroutine` /// event binding names, or `None` for other event kinds and when no /// provenance was recorded. @@ -669,6 +680,7 @@ impl Program { .rules .push(RuleProvenance { span: rule.span, + name: rule.name_span, event_name, conditions: rule .conditions @@ -808,7 +820,9 @@ impl Program { storage.rules.push(wir::Rule { name: rule.name.clone(), span: self.rule_span(rule_index), - name_span: None, + name_span: self + .rule_provenance(rule_index) + .and_then(|provenance| provenance.name), disabled: rule.disabled, event, conditions, @@ -1473,53 +1487,20 @@ fn apply_action_provenance( } *position += 1; } - wir::Action::While { - condition, body, .. - } => { - let source = provenance.get(*position).cloned().unwrap_or_default(); - *position += 1; - apply_action_source(storage, *id, &source); - apply_action_provenance(storage, &body, provenance, position)?; - *position += 1; - if let Some(provenance) = source.arguments.first() { - apply_value_provenance(storage, condition, provenance); - } - } - wir::Action::ForGlobalVariable { - start, - stop, - step, - body, - .. - } => { + wir::Action::While { body, .. } => { let source = provenance.get(*position).cloned().unwrap_or_default(); *position += 1; apply_action_source(storage, *id, &source); apply_action_provenance(storage, &body, provenance, position)?; *position += 1; - for (value, provenance) in [start, stop, step].into_iter().zip(&source.arguments) { - apply_value_provenance(storage, value, provenance); - } } - wir::Action::ForPlayerVariable { - player, - start, - stop, - step, - body, - .. - } => { + wir::Action::ForGlobalVariable { body, .. } + | wir::Action::ForPlayerVariable { body, .. } => { let source = provenance.get(*position).cloned().unwrap_or_default(); *position += 1; apply_action_source(storage, *id, &source); apply_action_provenance(storage, &body, provenance, position)?; *position += 1; - for (value, provenance) in [player, start, stop, step] - .into_iter() - .zip(&source.arguments) - { - apply_value_provenance(storage, value, provenance); - } } wir::Action::Disabled { action, .. } => { let start = *position; diff --git a/crates/workshop-rs/src/rules/parser.rs b/crates/workshop-rs/src/rules/parser.rs index e5120053..1fcac4e0 100644 --- a/crates/workshop-rs/src/rules/parser.rs +++ b/crates/workshop-rs/src/rules/parser.rs @@ -112,15 +112,11 @@ impl ParseContext<'_> { } pub(crate) fn variable_line(&mut self) -> Result { - let (index, span) = match self.next() { + let index = match self.next() { Some(Token { kind: TokenKind::Number { value, .. }, - start, - end, - }) => ( - value as u32, - Span::new(synthetic_span(start).file, start, end), - ), + .. + }) => value as u32, Some(token) => return Err(self.malformed("expected a variable index", &token)), None => return Err(self.malformed("expected a variable index", self.eof())), }; @@ -130,11 +126,7 @@ impl ParseContext<'_> { Ok(wir::WorkshopVariable { name, index, - span: Some(if span.file.index() == 0 { - name_span - } else { - span - }), + span: Some(name_span), name_span: Some(name_span), }) } @@ -173,13 +165,25 @@ impl ParseContext<'_> { self.expect_keyword("rule")?; self.expect(TokenKind::LParen, "expected '(' after 'rule'")?; let name = self.expect_string("expected a rule name string")?; + let (name_start, name_end) = self.previous_span(); + // The name sits inside the quotes of its string token; a quoted name + // spanning lines keeps the whole token extent instead. + let name_span = if name_start.line == name_end.line && name_end.col > name_start.col + 1 { + Span::new( + self.file(), + Position::new(name_start.line, name_start.col + 1), + Position::new(name_end.line, name_end.col - 1), + ) + } else { + Span::new(self.file(), name_start, name_end) + }; self.expect(TokenKind::RParen, "expected ')' after rule name")?; self.expect(TokenKind::LBrace, "expected '{' after rule header")?; let mut rule = wir::Rule { name, span: None, - name_span: None, + name_span: Some(name_span), disabled, event: Event::Global, conditions: Vec::new(), @@ -282,7 +286,11 @@ impl ParseContext<'_> { } } - pub(crate) fn subroutine_by_name(&self, name: &str) -> Result { + pub(crate) fn subroutine_by_name( + &self, + name: &str, + span: Option, + ) -> Result { self.subroutines .get(name) .copied() @@ -290,7 +298,7 @@ impl ParseContext<'_> { kind: "subroutine", spelling: name.to_string(), locale: self.locale.clone(), - span: None, + span, }) } } diff --git a/crates/workshop-rs/src/values/parser.rs b/crates/workshop-rs/src/values/parser.rs index f9fa540b..21f4460a 100644 --- a/crates/workshop-rs/src/values/parser.rs +++ b/crates/workshop-rs/src/values/parser.rs @@ -453,11 +453,7 @@ impl ParseContext<'_> { }) ) || self.players.contains_key(&name) { - let variable = self - .players - .get(&name) - .copied() - .unwrap_or(self.player_by_name(&name)?); + let variable = self.player_by_name(&name)?; Ok(self.target.values.push( ValueNode::new( Value::PlayerVariable { diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index 0c7d8918..51baab24 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -360,6 +360,7 @@ fn disabled_modifier_wrapping_a_terminator_is_rejected() { const IDENTIFIER_SOURCE: &str = r#"variables { global: 0: cakePos + 2: my var player: 1: playerScore } @@ -392,6 +393,24 @@ rule ("identifiers") { For Player Variable(Event Player, playerScore, 0, 5, 1); Abort; End; + Global.cakePos[0] = 4; + Event Player.playerScore[0] = 9; + Set Global Variable At Index(my var, 1, 2); + Set Player Variable At Index(Event Player, playerScore, 0, 3); + Modify Player Variable At Index(Event Player, playerScore, 0, Add, 5); + Set Global Variable(my var, Add(Global Variable(cakePos), Player Variable(Event Player, playerScore))); + Set Global Variable(cakePos, Add((Event Player).playerScore, 0)); + Stop Chasing Player Variable(Event Player, playerScore); + Chase Player Variable Over Time(Event Player, playerScore, 0, 30, None); + Chase Global Variable Over Time(my var, 0, 30, None); + Stop Chasing Global Variable(cakePos); + If(cakePos > 0); + Abort; + Else If(playerScore > 1); + Abort; + Else; + Abort; + End; } } @@ -421,6 +440,10 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { span_text(&program, program.global_variable_name_span(0).unwrap()), "cakePos" ); + assert_eq!( + span_text(&program, program.global_variable_name_span(1).unwrap()), + "my var" + ); assert_eq!( span_text(&program, program.player_variable_name_span(0).unwrap()), "playerScore" @@ -429,6 +452,15 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { span_text(&program, program.subroutine_name_span(0).unwrap()), "tick" ); + // Rule names sit inside their `rule("name")` string token. + assert_eq!( + span_text(&program, program.rule_name_span(0).unwrap()), + "identifiers" + ); + assert_eq!( + span_text(&program, program.rule_name_span(1).unwrap()), + "subroutine runner" + ); // Every variable write names its target identifier: standard-form // `Set`/`Modify`, infix `Global.name`/`Event Player.name` assignments, @@ -540,6 +572,118 @@ fn declaration_and_use_identifiers_slice_to_the_recorded_text() { "cakePos" ); + // `For` action spans cover the full `For ... End;` block, starting at + // the `For` keyword rather than the last word of its phrase. + assert!( + span_text(&program, program.action_span(0, 11).unwrap()) + .starts_with("For Global Variable(cakePos, 0, 10, 1);") + ); + assert!(span_text(&program, program.action_span(0, 11).unwrap()).ends_with("End;")); + assert!( + span_text(&program, program.action_span(0, 14).unwrap()) + .starts_with("For Player Variable(Event Player, playerScore, 0, 5, 1);") + ); + + // Indexed writes are `... Variable At Index` calls: the action itself + // carries no identifier, the variable argument records the name. + for action in [17, 18, 19, 20, 21] { + assert_eq!( + program.action_identifier_span(0, action), + None, + "action {action}" + ); + } + for (action, expected) in [ + (17, "cakePos"), + (18, "playerScore"), + (19, "my var"), + (20, "playerScore"), + (21, "playerScore"), + ] { + assert_eq!( + span_text( + &program, + program + .action_argument_value_span(0, action, 0, &[]) + .unwrap(), + ), + expected, + "action {action}" + ); + } + + // `Global Variable(name)` and `Player Variable(player, name)` read forms + // inside a `Set` value argument. + assert_eq!( + span_text(&program, program.action_identifier_span(0, 22).unwrap(),), + "my var" + ); + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 22, 0, &[0]).unwrap(), + ), + "cakePos" + ); + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 22, 0, &[1]).unwrap(), + ), + "playerScore" + ); + assert_eq!( + span_text( + &program, + program + .action_argument_value_span(0, 22, 0, &[1, 0]) + .unwrap(), + ), + "Event Player" + ); + // A parenthesized `(Event Player).name` read. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 23, 0, &[0]).unwrap(), + ), + "playerScore" + ); + // `Stop Chasing`/`Chase` variable forms record the name on the folded + // variable argument. + for (action, expected) in [ + (24, "playerScore"), + (25, "playerScore"), + (26, "my var"), + (27, "cakePos"), + ] { + assert_eq!( + span_text( + &program, + program + .action_argument_value_span(0, action, 0, &[]) + .unwrap(), + ), + expected, + "action {action}" + ); + } + // `If` and `Else If` conditions carry their reads as value provenance. + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 28, 0, &[0]).unwrap(), + ), + "cakePos" + ); + assert_eq!( + span_text( + &program, + program.action_argument_value_span(0, 30, 0, &[0]).unwrap(), + ), + "playerScore" + ); + // A `Subroutine` event binding records the bound name. assert_eq!( span_text(&program, program.rule_event_name_span(1).unwrap()), diff --git a/docs/source-preservation.md b/docs/source-preservation.md index da77f521..d7fc3409 100644 --- a/docs/source-preservation.md +++ b/docs/source-preservation.md @@ -44,7 +44,10 @@ nested-value provenance is exposed separately: name itself. - `Program::action_identifier_span` returns the span recorded for the variable or subroutine an action names — a set/modify/for target or a - `Call Subroutine` callee. + `Call Subroutine` callee. Indexed `... Variable At Index` writes are calls + and record the name on their variable argument instead. +- `Program::rule_name_span` returns the span of the name inside a rule's + `rule("name")` string. - `Program::rule_event_name_span` returns the span recorded for the subroutine name a `Subroutine` event binding names. - `Program::condition_value_span` and `Program::action_argument_value_span` @@ -62,10 +65,12 @@ names a variable or subroutine: `Set`/`Modify` standard forms and infix Positions with no recorded provenance return `None` rather than a neighboring or enclosing span. -Consumers that construct a program can attach the same metadata with -`Program::set_rule_span`, `Program::set_condition_span`, -`Program::set_action_span`, and `Program::set_action_argument_span`; declaration -spans use the corresponding variable and subroutine methods. `Program::edit_source` +Consumers that construct a program can attach rule, condition, action, and +direct-argument spans with `Program::set_rule_span`, +`Program::set_condition_span`, `Program::set_action_span`, and +`Program::set_action_argument_span`; declaration spans use the corresponding +variable and subroutine methods. Identifier-level provenance is recorded by +raw parsing only — no `set_*` method exists for it. `Program::edit_source` creates a checked edit from those public spans without exposing normalized WIR storage. Programmatic construction remains source-free when no files or spans are attached.