From 062a4ae67fc0ff1648426da9e6f03dc4e68c8b6f Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 04:04:30 +0800 Subject: [PATCH] feat: carry identifier and nested-value provenance in mapped text Providers can now attach rule name/event-name spans, action identifier spans, and recursive value provenance (identifier spans plus positional children) through additive optional members on the existing workshop-rs/mapped-text-v1 entries. Consumers built for the extended contract recover exact identifier locations that previously reported as unmapped; older decoders ignore the new members and keep the previous behavior. Closes #328 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- crates/workshop-rs/src/program.rs | 40 ++ crates/workshop-rs/src/program/source_map.rs | 273 +++++++++-- crates/workshop-rs/tests/source_map.rs | 484 ++++++++++++++++++- docs/source-preservation.md | 33 +- 4 files changed, 771 insertions(+), 59 deletions(-) diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index ef3d369..f940314 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -2033,6 +2033,46 @@ impl From for Condition { } } +/// The direct value arguments of a public action, in their mapped order. +fn action_argument_values(action: &Action) -> Vec<&Value> { + match action { + Action::SetGlobalVariable { value, .. } | Action::ModifyGlobalVariable { value, .. } => { + vec![value] + } + Action::SetPlayerVariable { player, value, .. } + | Action::ModifyPlayerVariable { player, value, .. } => vec![player, value], + Action::AssignMember { target, value, .. } => vec![target, value], + Action::If { condition } | Action::ElseIf { condition } | Action::While { condition } => { + vec![condition] + } + Action::ForGlobalVariable { + start, stop, step, .. + } => vec![start, stop, step], + Action::ForPlayerVariable { + player, + start, + stop, + step, + .. + } => vec![player, start, stop, step], + Action::Call { args, .. } => args.iter().collect(), + Action::CallSubroutine { .. } | Action::Else | Action::End => Vec::new(), + Action::Disabled { action } => action_argument_values(action), + } +} + +/// The children a public value exposes to provenance addressing, in the order +/// [`Program::condition_value_span`] documents. +fn value_children(value: &Value) -> Vec<&Value> { + match value { + Value::Array(values) => values.iter().collect(), + Value::Vector { x, y, z } => vec![x.as_ref(), y.as_ref(), z.as_ref()], + Value::PlayerVariable { player, .. } => vec![player.as_ref()], + Value::Call { args, .. } => args.iter().collect(), + _ => Vec::new(), + } +} + fn action_argument_count(action: &Action) -> usize { match action { Action::SetGlobalVariable { .. } diff --git a/crates/workshop-rs/src/program/source_map.rs b/crates/workshop-rs/src/program/source_map.rs index 42edf09..8b78339 100644 --- a/crates/workshop-rs/src/program/source_map.rs +++ b/crates/workshop-rs/src/program/source_map.rs @@ -2,7 +2,10 @@ use serde::{Deserialize, Serialize}; -use super::{DeclarationProvenance, Program, ProgramProvenance, action_argument_count, fit}; +use super::{ + DeclarationProvenance, Program, ProgramProvenance, Value, ValueProvenance, + action_argument_count, action_argument_values, fit, value_children, +}; use crate::source::{FileId, Position, SourceFile, Span}; /// Identifier of the canonical Workshop text artifact: the Workshop text alone. @@ -22,8 +25,15 @@ pub const MAPPED_TEXT_V1: &str = "workshop-rs/mapped-text-v1"; /// values. /// /// The mapping granularity is rule, condition, action, direct action argument, -/// and variable and subroutine declarations. Nodes without an authored origin -/// have no entry, so consumers report evidence on them as unmapped. +/// and variable and subroutine declarations. Entries may additionally carry the +/// identifier span the node names — a rule's name, a rule's subroutine event +/// binding, an action's target or callee, a value's variable or subroutine +/// identifier — and condition and action-argument entries carry the provenance +/// of the value's children, keyed by position in the public [`Value`] tree. +/// Every field of an entry is optional: a node with only finer-grained +/// provenance, such as a member read that records just its identifier, +/// appears as an entry without `span`. Nodes without an authored origin have +/// no entry, so consumers report evidence on them as unmapped. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SourceMap { files: Vec, @@ -79,7 +89,7 @@ pub enum SourceMapError { InvalidPosition, /// Two entries map the same node. DuplicateEntry, - /// A declaration entry carries neither a span nor a name span. + /// An entry carries no position at all. EmptyEntry, /// A span references a file outside the file table. UnknownFile(usize), @@ -134,7 +144,7 @@ impl std::fmt::Display for SourceMapError { write!(formatter, "source map entry is outside the program shape") } Self::DuplicateEntry => write!(formatter, "source map maps a node twice"), - Self::EmptyEntry => write!(formatter, "source map declaration entry has no span"), + Self::EmptyEntry => write!(formatter, "source map entry has no position"), Self::UnknownFile(file) => { write!(formatter, "source map span references unknown file {file}") } @@ -190,36 +200,65 @@ impl SourceMap { &mut spans, ); for (rule, public) in program.rules.iter().enumerate() { - if let Some(span) = program.rule_span(rule) { - spans.push(MappedNode::Rule { - rule, - span: span.into(), - }); + if let Some(provenance) = program.rule_provenance(rule) { + if provenance.span.is_some() + || provenance.name.is_some() + || provenance.event_name.is_some() + { + spans.push(MappedNode::Rule { + rule, + span: provenance.span.map(WireSpan::from), + name_span: provenance.name.map(WireSpan::from), + event_name_span: provenance.event_name.map(WireSpan::from), + }); + } } for condition in 0..public.conditions.len() { - if let Some(span) = program.condition_span(rule, condition) { + let Some(provenance) = program.condition_provenance(rule, condition) else { + continue; + }; + let children = wire_children(&provenance.children); + if provenance.span.is_some() + || provenance.identifier.is_some() + || !children.is_empty() + { spans.push(MappedNode::Condition { rule, condition, - span: span.into(), + span: provenance.span.map(WireSpan::from), + identifier_span: provenance.identifier.map(WireSpan::from), + children, }); } } for (action, public_action) in public.actions.iter().enumerate() { - if let Some(span) = program.action_span(rule, action) { + let Some(provenance) = program.action_provenance(rule, action) else { + continue; + }; + if provenance.span.is_some() || provenance.identifier.is_some() { spans.push(MappedNode::Action { rule, action, - span: span.into(), + span: provenance.span.map(WireSpan::from), + identifier_span: provenance.identifier.map(WireSpan::from), }); } for argument in 0..action_argument_count(public_action) { - if let Some(span) = program.action_argument_span(rule, action, argument) { + let Some(argument_provenance) = provenance.arguments.get(argument) else { + continue; + }; + let children = wire_children(&argument_provenance.children); + if argument_provenance.span.is_some() + || argument_provenance.identifier.is_some() + || !children.is_empty() + { spans.push(MappedNode::ActionArgument { rule, action, argument, - span: span.into(), + span: argument_provenance.span.map(WireSpan::from), + identifier_span: argument_provenance.identifier.map(WireSpan::from), + children, }); } } @@ -309,60 +348,108 @@ impl SourceMap { } *declaration = mapped; } - MappedNode::Rule { rule, span } => { - let span = self.span(*span)?; - let slot = &mut provenance + MappedNode::Rule { + rule, + span, + name_span, + event_name_span, + } => { + let span = span.map(|span| self.span(span)).transpose()?; + let name = name_span.map(|span| self.span(span)).transpose()?; + let event_name = event_name_span.map(|span| self.span(span)).transpose()?; + let slot = provenance .rules .get_mut(*rule) - .ok_or(SourceMapError::InvalidPosition)? - .span; - if slot.replace(span).is_some() { + .ok_or(SourceMapError::InvalidPosition)?; + if slot.span.is_some() || slot.name.is_some() || slot.event_name.is_some() { return Err(SourceMapError::DuplicateEntry); } + if span.is_none() && name.is_none() && event_name.is_none() { + return Err(SourceMapError::EmptyEntry); + } + slot.span = span; + slot.name = name; + slot.event_name = event_name; } MappedNode::Condition { rule, condition, span, + identifier_span, + children, } => { - let span = self.span(*span)?; + let span = span.map(|span| self.span(span)).transpose()?; + let identifier = identifier_span.map(|span| self.span(span)).transpose()?; + let has_children = children.iter().any(|child| !wire_value_unmapped(child)); + let children = self.mapped_children( + children, + &program + .rules + .get(*rule) + .and_then(|rule| rule.conditions.get(*condition)) + .ok_or(SourceMapError::InvalidPosition)? + .value, + )?; let slot = provenance .rules .get_mut(*rule) .and_then(|rule| rule.conditions.get_mut(*condition)) .ok_or(SourceMapError::InvalidPosition)?; - if slot.span.replace(span).is_some() { + if slot.span.is_some() || slot.identifier.is_some() || !slot.children.is_empty() + { return Err(SourceMapError::DuplicateEntry); } + if span.is_none() && identifier.is_none() && !has_children { + return Err(SourceMapError::EmptyEntry); + } + slot.span = span; + slot.identifier = identifier; + slot.children = children; } - MappedNode::Action { rule, action, span } => { - let span = self.span(*span)?; - let slot = &mut provenance + MappedNode::Action { + rule, + action, + span, + identifier_span, + } => { + let span = span.map(|span| self.span(span)).transpose()?; + let identifier = identifier_span.map(|span| self.span(span)).transpose()?; + let slot = provenance .rules .get_mut(*rule) .and_then(|rule| rule.actions.get_mut(*action)) - .ok_or(SourceMapError::InvalidPosition)? - .span; - if slot.replace(span).is_some() { + .ok_or(SourceMapError::InvalidPosition)?; + if slot.span.is_some() || slot.identifier.is_some() { return Err(SourceMapError::DuplicateEntry); } + if span.is_none() && identifier.is_none() { + return Err(SourceMapError::EmptyEntry); + } + slot.span = span; + slot.identifier = identifier; } MappedNode::ActionArgument { rule, action, argument, span, + identifier_span, + children, } => { - let span = self.span(*span)?; - let count = program + let span = span.map(|span| self.span(span)).transpose()?; + let identifier = identifier_span.map(|span| self.span(span)).transpose()?; + let argument_values = program .rules .get(*rule) .and_then(|rule| rule.actions.get(*action)) - .map(action_argument_count) + .map(action_argument_values) .ok_or(SourceMapError::InvalidPosition)?; - if *argument >= count { + let Some(&value) = argument_values.get(*argument) else { return Err(SourceMapError::InvalidPosition); - } + }; + let count = argument_values.len(); + let has_children = children.iter().any(|child| !wire_value_unmapped(child)); + let children = self.mapped_children(children, value)?; let arguments = &mut provenance .rules .get_mut(*rule) @@ -370,9 +457,17 @@ impl SourceMap { .ok_or(SourceMapError::InvalidPosition)? .arguments; fit(arguments, count); - if arguments[*argument].span.replace(span).is_some() { + let slot = &mut arguments[*argument]; + if slot.span.is_some() || slot.identifier.is_some() || !slot.children.is_empty() + { return Err(SourceMapError::DuplicateEntry); } + if span.is_none() && identifier.is_none() && !has_children { + return Err(SourceMapError::EmptyEntry); + } + slot.span = span; + slot.identifier = identifier; + slot.children = children; } } } @@ -409,6 +504,43 @@ impl SourceMap { name_span: name_span.map(|span| self.span(span)).transpose()?, }) } + + /// Map serialized children onto a public value's children by position. + /// + /// Emission can canonicalize a value into a form with a different public + /// arity — `(expr).member` reparse as a variable read drops the member-name + /// child, for example — so children are matched positionally: entries + /// beyond the value's children are dropped and positions with no entry are + /// left unmapped. + fn mapped_children( + &self, + wire: &[WireValue], + value: &Value, + ) -> Result, SourceMapError> { + let children = value_children(value); + let mut mapped = wire + .iter() + .zip(children.iter().copied()) + .map(|(wire, value)| self.mapped_value(wire, value)) + .collect::, _>>()?; + mapped.resize_with(children.len(), ValueProvenance::default); + Ok(mapped) + } + + fn mapped_value( + &self, + wire: &WireValue, + value: &Value, + ) -> Result { + Ok(ValueProvenance { + span: wire.span.map(|span| self.span(span)).transpose()?, + identifier: wire + .identifier_span + .map(|span| self.span(span)) + .transpose()?, + children: self.mapped_children(&wire.children, value)?, + }) + } } impl MappedText { @@ -461,6 +593,37 @@ impl MappedText { } } +/// The wire form of recorded value-provenance children: children keep their +/// position up to the last mapped one, and a level with no recorded provenance +/// at all is omitted. +fn wire_children(children: &[ValueProvenance]) -> Vec { + let Some(last) = children.iter().rposition(|child| !value_unmapped(child)) else { + return Vec::new(); + }; + children[..=last].iter().map(wire_value).collect() +} + +fn wire_value(provenance: &ValueProvenance) -> WireValue { + WireValue { + span: provenance.span.map(WireSpan::from), + identifier_span: provenance.identifier.map(WireSpan::from), + children: wire_children(&provenance.children), + } +} + +fn value_unmapped(provenance: &ValueProvenance) -> bool { + provenance.span.is_none() + && provenance.identifier.is_none() + && provenance.children.iter().all(value_unmapped) +} + +/// Whether a serialized value entry contributes any position. +fn wire_value_unmapped(value: &WireValue) -> bool { + value.span.is_none() + && value.identifier_span.is_none() + && value.children.iter().all(wire_value_unmapped) +} + fn push_declarations( program: &Program, count: usize, @@ -576,23 +739,41 @@ impl Shape { enum MappedNode { Rule { rule: usize, - span: WireSpan, + #[serde(default, skip_serializing_if = "Option::is_none")] + span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + name_span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + event_name_span: Option, }, Condition { rule: usize, condition: usize, - span: WireSpan, + #[serde(default, skip_serializing_if = "Option::is_none")] + span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + identifier_span: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + children: Vec, }, Action { rule: usize, action: usize, - span: WireSpan, + #[serde(default, skip_serializing_if = "Option::is_none")] + span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + identifier_span: Option, }, ActionArgument { rule: usize, action: usize, argument: usize, - span: WireSpan, + #[serde(default, skip_serializing_if = "Option::is_none")] + span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + identifier_span: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + children: Vec, }, GlobalVariable { index: usize, @@ -617,6 +798,18 @@ enum MappedNode { }, } +/// The mapped provenance of one value node, mirroring the structure of the +/// public [`Value`] tree: `children` addresses child values by position. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct WireValue { + #[serde(default, skip_serializing_if = "Option::is_none")] + span: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + identifier_span: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + children: Vec, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] struct WireSpan { file: usize, diff --git a/crates/workshop-rs/tests/source_map.rs b/crates/workshop-rs/tests/source_map.rs index a02a30b..f3c053c 100644 --- a/crates/workshop-rs/tests/source_map.rs +++ b/crates/workshop-rs/tests/source_map.rs @@ -19,37 +19,481 @@ fn span(file: FileId, line: u32, start: u32, end: u32) -> Span { Span::new(file, Position::new(line, start), Position::new(line, end)) } -/// Every mapped position of a program in a stable order. -fn mapped_positions(program: &Program) -> Vec<(String, Option)> { +/// Every mapped position of `program` in a stable order. +/// +/// Value paths are enumerated on `public` so a reparsed program is compared +/// against the tree the map was extracted from: emission can canonicalize +/// value forms (a boolean condition becomes a comparison), and positions only +/// addressable in the reparsed tree hold no provenance by construction. +fn mapped_positions(program: &Program, public: &Program) -> Vec<(String, Option)> { let mut positions = Vec::new(); - for (rule, public) in program.rules.iter().enumerate() { + for (rule, public) in public.rules.iter().enumerate() { positions.push((format!("rule {rule}"), program.rule_span(rule))); - for condition in 0..public.conditions.len() { + positions.push((format!("rule {rule} name"), program.rule_name_span(rule))); + positions.push(( + format!("rule {rule} event name"), + program.rule_event_name_span(rule), + )); + for (condition, public_condition) in public.conditions.iter().enumerate() { positions.push(( format!("rule {rule} condition {condition}"), program.condition_span(rule, condition), )); + for path in value_paths(&public_condition.value) { + positions.push(( + format!("rule {rule} condition {condition} value {path:?}"), + program.condition_value_span(rule, condition, &path), + )); + } } - for action in 0..public.actions.len() { + for (action, public_action) in public.actions.iter().enumerate() { positions.push(( format!("rule {rule} action {action}"), program.action_span(rule, action), )); + positions.push(( + format!("rule {rule} action {action} identifier"), + program.action_identifier_span(rule, action), + )); for argument in 0..6 { positions.push(( format!("rule {rule} action {action} argument {argument}"), program.action_argument_span(rule, action, argument), )); } + for (argument, argument_value) in action_arguments(public_action).iter().enumerate() { + for path in value_paths(argument_value) { + positions.push(( + format!("rule {rule} action {action} argument {argument} value {path:?}"), + program.action_argument_value_span(rule, action, argument, &path), + )); + } + } } } positions } +fn value_paths(value: &Value) -> Vec> { + fn walk(value: &Value, path: &mut Vec, paths: &mut Vec>) { + paths.push(path.clone()); + for (index, child) in value_children(value).into_iter().enumerate() { + path.push(index); + walk(child, path, paths); + path.pop(); + } + } + let mut paths = Vec::new(); + walk(value, &mut Vec::new(), &mut paths); + paths +} + +fn value_children(value: &Value) -> Vec<&Value> { + match value { + Value::Array(values) => values.iter().collect(), + Value::Vector { x, y, z } => vec![x.as_ref(), y.as_ref(), z.as_ref()], + Value::PlayerVariable { player, .. } => vec![player.as_ref()], + Value::Call { args, .. } => args.iter().collect(), + _ => Vec::new(), + } +} + +fn action_arguments(action: &Action) -> Vec<&Value> { + match action { + Action::SetGlobalVariable { value, .. } | Action::ModifyGlobalVariable { value, .. } => { + vec![value] + } + Action::SetPlayerVariable { player, value, .. } + | Action::ModifyPlayerVariable { player, value, .. } => vec![player, value], + Action::AssignMember { target, value, .. } => vec![target, value], + Action::If { condition } | Action::ElseIf { condition } | Action::While { condition } => { + vec![condition] + } + Action::ForGlobalVariable { + start, stop, step, .. + } => vec![start, stop, step], + Action::ForPlayerVariable { + player, + start, + stop, + step, + .. + } => vec![player, start, stop, step], + Action::Call { args, .. } => args.iter().collect(), + Action::CallSubroutine { .. } | Action::Else | Action::End => Vec::new(), + Action::Disabled { action } => action_arguments(action), + } +} + fn parsed(source: &str) -> Program { parser::parse(source, &catalog(), &en()).expect("parses") } +const PROVENANCE_SOURCE: &str = r#"variables { + global: + 0: counter + player: + 1: score +} + +subroutines { + 0: tick +} + +rule ("writes") { + event { Ongoing - Global; } + conditions { + Global.counter > 0; + } + actions { + Set Global Variable(counter, Add(Global.counter, Event Player.score)); + Call Subroutine(tick); + Global.counter = 2; + } +} + +rule ("on tick") { + event { + Subroutine; + tick; + } + actions { + Wait(1); + } +} +"#; + +#[test] +fn extraction_and_application_preserve_identifier_and_nested_value_provenance() { + let catalog = catalog(); + let program = parsed(PROVENANCE_SOURCE); + let artifact = MappedText { + text: emitter::emit(&program, &catalog, &en()).expect("emits"), + map: SourceMap::extract(&program), + }; + let json: serde_json::Value = serde_json::from_str(&artifact.to_json()).unwrap(); + + let entries = json["spans"].as_array().unwrap(); + let entries_of = |node: &str| { + entries + .iter() + .filter(|entry| entry["node"] == node) + .collect::>() + }; + for rule in entries_of("rule") { + assert!(rule["name_span"].is_object(), "{rule}"); + } + let subroutine_rule = entries_of("rule") + .into_iter() + .find(|entry| entry["rule"] == 1) + .expect("second rule entry"); + assert!(subroutine_rule["event_name_span"].is_object()); + assert!( + entries_of("action") + .iter() + .any(|entry| entry["identifier_span"].is_object()), + "no action entry carries an identifier span" + ); + assert!( + entries_of("action_argument") + .iter() + .any(|entry| entry["children"] + .as_array() + .is_some_and(|children| !children.is_empty())), + "no action argument entry carries children" + ); + assert!( + entries_of("condition").iter().any(|entry| entry["children"] + .as_array() + .is_some_and(|children| !children.is_empty())), + "no condition entry carries children" + ); + + let decoded = MappedText::from_json(&artifact.to_json()).expect("decodes"); + let mut reparsed = parsed(&decoded.text); + decoded.map.apply(&mut reparsed).expect("applies"); + assert_eq!( + mapped_positions(&reparsed, &program), + mapped_positions(&program, &program) + ); + assert_eq!(SourceMap::extract(&reparsed), decoded.map); +} + +fn wire_span(file: usize, line: u32, start: u32, end: u32) -> serde_json::Value { + serde_json::json!({ + "file": file, + "start": {"line": line, "column": start}, + "end": {"line": line, "column": end}, + }) +} + +#[test] +fn authored_entries_attach_identifier_and_nested_value_provenance() { + // A provider authors the entries directly: `name_span`, `event_name_span`, + // `identifier_span`, and `children` are optional members of the existing + // entries, and spans may address any file in the table. + let artifact = serde_json::json!({ + "format": "workshop-rs/mapped-text-v1", + "text": PROVENANCE_SOURCE, + "files": [{"path": "src/main.opy"}, {"path": "src/generated.opy"}], + "shape": { + "global_variables": 1, + "player_variables": 1, + "subroutines": 1, + "rules": [ + {"conditions": 1, "actions": 3}, + {"conditions": 0, "actions": 1}, + ], + }, + "spans": [ + { + "node": "rule", "rule": 0, + "span": wire_span(0, 1, 1, 30), + "name_span": wire_span(0, 1, 7, 13), + }, + { + "node": "condition", "rule": 0, "condition": 0, + "span": wire_span(0, 2, 1, 20), + "children": [ + {"identifier_span": wire_span(0, 2, 8, 15)}, + {"span": wire_span(0, 2, 19, 20)}, + ], + }, + { + "node": "action", "rule": 0, "action": 0, + "span": wire_span(0, 3, 1, 50), + "identifier_span": wire_span(0, 3, 22, 29), + }, + { + "node": "action_argument", "rule": 0, "action": 0, "argument": 0, + "span": wire_span(0, 3, 31, 49), + "children": [ + {"identifier_span": wire_span(0, 3, 35, 48)}, + {"span": wire_span(0, 3, 50, 68)}, + ], + }, + { + "node": "action", "rule": 0, "action": 1, + "span": wire_span(0, 4, 1, 20), + "identifier_span": wire_span(0, 4, 16, 20), + }, + {"node": "action", "rule": 0, "action": 2, "span": wire_span(0, 5, 1, 15)}, + { + "node": "rule", "rule": 1, + "span": wire_span(1, 1, 1, 25), + "name_span": wire_span(1, 1, 7, 14), + "event_name_span": wire_span(1, 3, 9, 13), + }, + { + "node": "action", "rule": 1, "action": 0, + "span": wire_span(1, 5, 5, 12), + "identifier_span": wire_span(1, 5, 9, 11), + }, + ], + }); + let decoded = MappedText::from_json(&artifact.to_string()).expect("decodes"); + let mut program = parsed(&decoded.text); + decoded.map.apply(&mut program).expect("applies"); + + let authored = FileId::from_index(0); + let generated = FileId::from_index(1); + assert_eq!(program.rule_span(0), Some(span(authored, 1, 1, 30))); + assert_eq!(program.rule_name_span(0), Some(span(authored, 1, 7, 13))); + assert_eq!(program.rule_name_span(1), Some(span(generated, 1, 7, 14))); + assert_eq!(program.rule_event_name_span(0), None); + assert_eq!( + program.rule_event_name_span(1), + Some(span(generated, 3, 9, 13)) + ); + assert_eq!( + program.action_identifier_span(0, 0), + Some(span(authored, 3, 22, 29)) + ); + assert_eq!( + program.action_identifier_span(0, 1), + Some(span(authored, 4, 16, 20)) + ); + assert_eq!(program.action_identifier_span(0, 2), None); + assert_eq!( + program.action_identifier_span(1, 0), + Some(span(generated, 5, 9, 11)) + ); + assert_eq!( + program.condition_value_span(0, 0, &[]), + Some(span(authored, 2, 1, 20)) + ); + assert_eq!( + program.condition_value_span(0, 0, &[0]), + Some(span(authored, 2, 8, 15)) + ); + assert_eq!( + program.condition_value_span(0, 0, &[1]), + Some(span(authored, 2, 19, 20)) + ); + assert_eq!(program.condition_value_span(0, 0, &[2]), None); + assert_eq!( + program.action_argument_value_span(0, 0, 0, &[]), + Some(span(authored, 3, 31, 49)) + ); + assert_eq!( + program.action_argument_value_span(0, 0, 0, &[0]), + Some(span(authored, 3, 35, 48)) + ); + assert_eq!( + program.action_argument_value_span(0, 0, 0, &[1]), + Some(span(authored, 3, 50, 68)) + ); + assert_eq!(SourceMap::extract(&program), decoded.map); +} + +#[test] +fn nested_value_entries_are_validated_against_the_value_tree() { + let map = SourceMap::extract(&parsed(PROVENANCE_SOURCE)); + let base: serde_json::Value = + serde_json::from_str(&MappedText::new("", map).to_json()).unwrap(); + let argument_with_children = base["spans"] + .as_array() + .unwrap() + .iter() + .position(|entry| { + entry["node"] == "action_argument" + && entry["children"] + .as_array() + .is_some_and(|children| !children.is_empty()) + }) + .expect("a child-bearing action argument entry"); + let mut target = parsed(PROVENANCE_SOURCE); + let before = mapped_positions(&target, &target); + + let mut unknown_child_file = base.clone(); + unknown_child_file["spans"][argument_with_children]["children"][0]["identifier_span"]["file"] = + serde_json::json!(9); + let mut invalid_child_span = base.clone(); + invalid_child_span["spans"][argument_with_children]["children"][0]["identifier_span"]["end"] = + serde_json::json!({"line": 0, "column": 0}); + let mut invalid_name_span = base.clone(); + invalid_name_span["spans"] + .as_array_mut() + .unwrap() + .iter_mut() + .find(|entry| entry["node"] == "rule") + .expect("a rule entry")["name_span"]["file"] = serde_json::json!(9); + let mut empty_entry = base.clone(); + let fieldless = serde_json::json!({"node": "action", "rule": 1, "action": 0}); + empty_entry["spans"].as_array_mut().unwrap().push(fieldless); + + for (artifact, matches) in [ + ( + unknown_child_file, + (|error: &SourceMapError| matches!(error, SourceMapError::UnknownFile(9))) + as fn(&SourceMapError) -> bool, + ), + (invalid_child_span, |error| { + matches!(error, SourceMapError::InvalidSpan(_)) + }), + (invalid_name_span, |error| { + matches!(error, SourceMapError::UnknownFile(9)) + }), + (empty_entry, |error| { + matches!(error, SourceMapError::DuplicateEntry) + }), + ] { + let decoded = MappedText::from_json(&artifact.to_string()).expect("structure decodes"); + let error = decoded + .map + .apply(&mut target) + .expect_err("entry is invalid"); + assert!(matches(&error), "{error:?}"); + assert_eq!(mapped_positions(&target, &target), before); + } + + // Children beyond the applied value's tree are dropped rather than + // rejected: emission can canonicalize a value into a form with fewer + // children (a member access reparses as a variable read). + let mut extra_child = base.clone(); + extra_child["spans"][argument_with_children]["children"] + .as_array_mut() + .unwrap() + .push(serde_json::json!({"span": wire_span(0, 1, 1, 2)})); + let mut grandchild_of_a_leaf = base.clone(); + grandchild_of_a_leaf["spans"][argument_with_children]["children"][0]["children"] = + serde_json::json!([{"span": wire_span(0, 1, 1, 2)}]); + for artifact in [extra_child, grandchild_of_a_leaf] { + let decoded = MappedText::from_json(&artifact.to_string()).expect("structure decodes"); + let mut reparsed = parsed(PROVENANCE_SOURCE); + decoded + .map + .apply(&mut reparsed) + .expect("excess children attach positionally"); + assert_eq!(reparsed.action_argument_value_span(0, 0, 0, &[2]), None); + assert!(reparsed.action_argument_value_span(0, 0, 0, &[0]).is_some()); + } + + // An entry carrying no position at all is rejected. + let mut no_position = base.clone(); + let action = no_position["spans"] + .as_array_mut() + .unwrap() + .iter_mut() + .find(|entry| entry["node"] == "action") + .expect("an action entry"); + action.as_object_mut().unwrap().remove("span"); + action.as_object_mut().unwrap().remove("identifier_span"); + let decoded = MappedText::from_json(&no_position.to_string()).expect("structure decodes"); + assert_eq!( + decoded.map.apply(&mut target), + Err(SourceMapError::EmptyEntry) + ); + assert_eq!(mapped_positions(&target, &target), before); +} + +#[test] +fn artifacts_without_identifier_or_children_members_apply_as_before() { + // An artifact produced before these members existed decodes and applies + // unchanged: coarse spans map and the finer positions report unmapped. + fn strip_new_members(value: &mut serde_json::Value) { + if let Some(object) = value.as_object_mut() { + for member in [ + "name_span", + "event_name_span", + "identifier_span", + "children", + ] { + object.remove(member); + } + } + if let Some(array) = value.as_array_mut() { + for item in array { + strip_new_members(item); + } + } else if let Some(object) = value.as_object_mut() { + for item in object.values_mut() { + strip_new_members(item); + } + } + } + + let map = SourceMap::extract(&parsed(PROVENANCE_SOURCE)); + let mut json: serde_json::Value = + serde_json::from_str(&MappedText::new("", map).to_json()).unwrap(); + strip_new_members(&mut json); + json["spans"][0]["future_member"] = serde_json::json!(true); + let decoded = MappedText::from_json(&json.to_string()).expect("decodes"); + let mut program = parsed(PROVENANCE_SOURCE); + decoded.map.apply(&mut program).expect("applies"); + + assert_eq!(program.rule_name_span(0), None); + assert_eq!(program.rule_event_name_span(1), None); + assert_eq!(program.action_identifier_span(0, 0), None); + let coarse_condition = program.condition_span(0, 0); + assert!(coarse_condition.is_some()); + assert_eq!(program.condition_value_span(0, 0, &[]), coarse_condition); + assert_eq!(program.condition_value_span(0, 0, &[0]), None); + assert!(program.action_argument_span(0, 0, 0).is_some()); + assert_eq!(program.action_argument_value_span(0, 0, 0, &[0]), None); + assert_eq!(SourceMap::extract(&program), decoded.map); +} + const TWO_RULES: &str = r#"rule ("first") { event { Ongoing - Global; } conditions { 1 == 1; } @@ -118,8 +562,13 @@ fn extract_emit_parse_apply_round_trips_every_mapped_position() { .unwrap_or_else(|error| panic!("{} apply failed: {error}", case.id)); assert!(reparsed.source(FileId::from_index(0)).is_none()); - let expected = mapped_positions(&program); - assert_eq!(mapped_positions(&reparsed), expected, "{}", case.id); + let expected = mapped_positions(&program, &program); + assert_eq!( + mapped_positions(&reparsed, &program), + expected, + "{}", + case.id + ); compared += expected.iter().filter(|(_, span)| span.is_some()).count(); } assert!(compared > 0, "no real-project positions were compared"); @@ -140,7 +589,10 @@ fn programmatic_mapping_round_trips_through_text_and_declarations() { let mut reparsed = parser::parse(&decoded.text, &catalog, &en()).expect("reparses"); decoded.map.apply(&mut reparsed).expect("applies"); - assert_eq!(mapped_positions(&reparsed), mapped_positions(&program)); + assert_eq!( + mapped_positions(&reparsed, &program), + mapped_positions(&program, &program) + ); assert!(reparsed.rule_span(0).is_some()); assert!(reparsed.source(FileId::from_index(0)).is_none()); assert_eq!(SourceMap::extract(&reparsed), decoded.map); @@ -242,9 +694,13 @@ fn shape_mismatch_rejects_the_whole_mapping() { }, ), ] { - let before = mapped_positions(&mutated); + let before = mapped_positions(&mutated, &mutated); assert_eq!(map.apply(&mut mutated), Err(expected)); - assert_eq!(mapped_positions(&mutated), before, "no partial application"); + assert_eq!( + mapped_positions(&mutated, &mutated), + before, + "no partial application" + ); } } @@ -257,7 +713,7 @@ fn invalid_entries_reject_the_whole_mapping_without_changing_the_program() { } .to_json(); let mut target = parsed(TWO_RULES); - let before = mapped_positions(&target); + let before = mapped_positions(&target, &target); let mut unknown_file: serde_json::Value = serde_json::from_str(&json).unwrap(); unknown_file["spans"] @@ -311,7 +767,7 @@ fn invalid_entries_reject_the_whole_mapping_without_changing_the_program() { .apply(&mut target) .expect_err("entry is invalid"); assert!(matches(&error), "{error:?}"); - assert_eq!(mapped_positions(&target), before); + assert_eq!(mapped_positions(&target, &target), before); } } @@ -333,9 +789,13 @@ fn inserting_or_removing_nodes_hides_displaced_spans() { for program in [&inserted_rule, &removed_rule] { assert_eq!(program.rule_span(0), None); assert_eq!(program.rule_span(1), None); + assert_eq!(program.rule_name_span(0), None); assert_eq!(program.condition_span(0, 0), None); + assert_eq!(program.condition_value_span(0, 0, &[]), None); assert_eq!(program.action_span(0, 0), None); + assert_eq!(program.action_identifier_span(0, 0), None); assert_eq!(program.action_argument_span(0, 0, 0), None); + assert_eq!(program.action_argument_value_span(0, 0, 0, &[]), None); } let mut inserted_condition = base.clone(); diff --git a/docs/source-preservation.md b/docs/source-preservation.md index d7fc340..f2c21e6 100644 --- a/docs/source-preservation.md +++ b/docs/source-preservation.md @@ -138,7 +138,7 @@ the program's retained source documents, including the Workshop text parsed from the artifact, are dropped and `Program::source` returns `None` for them. The whole mapping is rejected with a typed `SourceMapError`, leaving the program unchanged, when the program shape differs from the recorded shape or -when any entry is invalid, duplicated, or an empty declaration entry. +when any entry is invalid, duplicated, or carries no position at all. A `mapped-text-v1` document has these members: @@ -153,13 +153,32 @@ A `mapped-text-v1` document has these members: The `node` tags and their keys are `rule` (`rule`), `condition` (`rule`, `condition`), `action` (`rule`, `action`), `action_argument` (`rule`, `action`, `argument`), and `global_variable`, `player_variable`, and `subroutine` -(`index`). The first four carry a `span`; declarations carry `span`, -`name_span`, or both. A span is `{"file", "start", "end"}` with positions +(`index`). A span is `{"file", "start", "end"}` with positions `{"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 and action-identifier mappings are not part of the -format. +`spans` may be empty. Decoders ignore unknown members. + +Every entry carries `span` optionally; an entry may instead carry only the +finer-grained members below, which is how identifier-only provenance such as +a `(expr).member` variable read maps. Additional optional members: + +- `rule`: `name_span` for the `rule("name")` string and `event_name_span` + for the subroutine name a `Subroutine` event binding names. +- `action`: `identifier_span` for the target or callee the action names. +- `condition` and `action_argument`: `identifier_span` for the value's own + variable or subroutine identifier, and `children`, a list of + `{"span", "identifier_span", "children"}` members addressing the value's + children by position in the public `Value` tree — `Array` elements and + `Call` arguments by index, `Vector` components as `0`/`1`/`2` for x/y/z, + and a `PlayerVariable` player at `0`. All three members are optional at + every level. Children attach positionally when the mapped text reparses to + a differently shaped value — emission can canonicalize a member access into + a variable read — so children beyond the applied value's tree are dropped + and positions without an entry stay unmapped. +- declarations: `name_span` for the declared name in addition to `span`. + +Artifacts produced before these members existed decode and apply unchanged; +an artifact whose entries omit `span` requires a consumer built on this +contract. Columns follow `Position`: 1-based Unicode scalar values. Producers convert from other units, and editor presentation converts to UTF-16.