From fe4ab774f6ff19a57e7a29fd1389c720aaf5a37b Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 10 Oct 2026 01:59:26 +0800 Subject: [PATCH] fix(program): prune provenance spans that do not resolve in attached file source A provider map can record a span whose extent is not a position in the authored member: OPY macro expansion attributes expanded tokens by shifting them onto the invocation site, so a recorded end column runs past the authored line (wrightkit/wright#583, e.g. bastion's composition/bootstrap.opy rule spans). Once set_file_source retains that member's text, Program::validate bounds-checks every provenance span against the document and the stale position fails the whole load with invalid-span. Attaching text now audits provenance the same way record_identities retires displaced records: a span that does not resolve inside the retained document is cleared so the node survives with no span rather than a wrong one. Consumers keep every span that is still addressable, and lint fix extents derived from surviving spans stay trustworthy. --- crates/workshop-rs/src/program.rs | 57 +++++++++++++++++++++++ crates/workshop-rs/tests/program_model.rs | 27 +++++++++++ 2 files changed, 84 insertions(+) diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index 158c779..d831451 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -119,6 +119,50 @@ impl SpanSlot for ActionProvenance { /// The recorded span of one value node and its children, mirroring the /// structure of the public [`Value`] tree. +/// Clear every recorded span in `provenance` that does not resolve inside +/// `document` — `SourceDocument::byte_range` already rejects spans whose file +/// or extent does not fit. +fn prune_spans_outside(provenance: &mut ProgramProvenance, document: &SourceDocument) { + fn prune_slot(slot: &mut Option, document: &SourceDocument) { + if let Some(span) = slot { + if document.byte_range(*span).is_none() { + *slot = None; + } + } + } + fn prune_value(record: &mut ValueProvenance, document: &SourceDocument) { + prune_slot(&mut record.span, document); + prune_slot(&mut record.identifier, document); + for child in &mut record.children { + prune_value(child, document); + } + } + for declaration in provenance + .global_variables + .iter_mut() + .chain(&mut provenance.player_variables) + .chain(&mut provenance.subroutines) + { + prune_slot(&mut declaration.span, document); + prune_slot(&mut declaration.name_span, document); + } + for rule in &mut provenance.rules { + prune_slot(&mut rule.span, document); + prune_slot(&mut rule.name, document); + prune_slot(&mut rule.event_name, document); + for condition in &mut rule.conditions { + prune_value(condition, document); + } + for action in &mut rule.actions { + prune_slot(&mut action.span, document); + prune_slot(&mut action.identifier, document); + for argument in &mut action.arguments { + prune_value(argument, document); + } + } + } +} + #[derive(Debug, Clone, Default)] struct ValueProvenance { span: Option, @@ -235,11 +279,24 @@ impl Program { /// Attach authored source text to a registered file. Returns `false` when /// `file` is not a known file entry. + /// + /// Attaching text audits provenance: a recorded span that does not + /// resolve inside the retained document is stale data — for a + /// provider-mapped program it can describe the pre-expansion token stream + /// or a coordinate space the map no longer owns — and is cleared so + /// consumers see no span rather than a wrong one, matching how + /// `record_identities` retires displaced records (wrightkit/wright#583). pub fn set_file_source(&mut self, file: FileId, source: impl Into) -> bool { let Some(entry) = self.files.get_mut(file.index()) else { return false; }; entry.set_source(source); + if let (Some(provenance), Some(document)) = ( + self.provenance.as_mut(), + self.files.get(file.index()).and_then(SourceFile::source), + ) { + prune_spans_outside(provenance, document); + } true } diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index c5c1094..abc8d65 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -88,6 +88,33 @@ fn applying_a_source_map_drops_stale_settings_spans() { reparsed.validate().expect("mapped program validates"); } +#[test] +fn file_source_retention_prunes_spans_outside_the_text() { + // A provider map can record a span whose extent is not a position in the + // authored member — macro expansion attributes expanded text by shifting + // it onto the invocation site, so a recorded end column can run past the + // line (wrightkit/wright#583). Retaining the member's text clears such + // spans instead of letting them fail validation. + let workshop = "rule (\"tick\") {\n event { Ongoing - Each Player; All; All; }\n actions { Wait(1); }\n}\n"; + let mut reparsed = parser::parse(workshop, &catalog(), &en()).expect("parses"); + SourceMap::extract(&reparsed) + .apply(&mut reparsed) + .expect("map applies"); + assert!(reparsed.rule_span(0).is_some()); + + // One line covers the `rule` header start but not the name span end. + assert!(reparsed.set_file_source(FileId::from_index(0), "rule (\"t")); + assert_eq!(reparsed.rule_span(0), None); + reparsed.validate().expect("mapped program validates"); + + // A span that fits the retained text survives. + let mut full = parser::parse(workshop, &catalog(), &en()).expect("parses"); + SourceMap::extract(&full).apply(&mut full).expect("applies"); + assert!(full.set_file_source(FileId::from_index(0), workshop)); + assert!(full.rule_span(0).is_some()); + full.validate().expect("in-bounds spans validate"); +} + #[test] fn values_and_conditions_are_composable() { let value = Value::call(