From 773e6859e6e296c3d99a24593736c174494b77ec Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 10 Oct 2026 07:28:56 +0800 Subject: [PATCH] fix(program): audit only the attached file's spans when retaining source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set_file_source audited every recorded span against the newly attached document, but byte_range rejects spans in other files too — so the documented per-file attach loop cleared each earlier file's provenance as the next source arrived. Only spans belonging to the attached file can be stale; spans in other files now keep their slots until their own source is attached. --- crates/workshop-rs/src/program.rs | 63 +++++++++++++---------- crates/workshop-rs/tests/program_model.rs | 43 ++++++++++++++++ 2 files changed, 78 insertions(+), 28 deletions(-) diff --git a/crates/workshop-rs/src/program.rs b/crates/workshop-rs/src/program.rs index d831451..11c8b90 100644 --- a/crates/workshop-rs/src/program.rs +++ b/crates/workshop-rs/src/program.rs @@ -117,24 +117,29 @@ 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) { +/// Clear every recorded span in `provenance` that belongs to `file` and does +/// not resolve inside `document`. A span in another file is not stale: its +/// own source has not been attached yet, so it cannot be judged against this +/// document — `SourceDocument::byte_range` already rejects both a file +/// mismatch and an out-of-bounds extent, so the span's own file gate is what +/// keeps foreign-file spans alive until their source arrives. +fn prune_spans_outside( + provenance: &mut ProgramProvenance, + file: FileId, + document: &SourceDocument, +) { + fn prune_slot(slot: &mut Option, file: FileId, document: &SourceDocument) { if let Some(span) = slot { - if document.byte_range(*span).is_none() { + if span.file == file && 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); + fn prune_value(record: &mut ValueProvenance, file: FileId, document: &SourceDocument) { + prune_slot(&mut record.span, file, document); + prune_slot(&mut record.identifier, file, document); for child in &mut record.children { - prune_value(child, document); + prune_value(child, file, document); } } for declaration in provenance @@ -143,21 +148,21 @@ fn prune_spans_outside(provenance: &mut ProgramProvenance, document: &SourceDocu .chain(&mut provenance.player_variables) .chain(&mut provenance.subroutines) { - prune_slot(&mut declaration.span, document); - prune_slot(&mut declaration.name_span, document); + prune_slot(&mut declaration.span, file, document); + prune_slot(&mut declaration.name_span, file, 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); + prune_slot(&mut rule.span, file, document); + prune_slot(&mut rule.name, file, document); + prune_slot(&mut rule.event_name, file, document); for condition in &mut rule.conditions { - prune_value(condition, document); + prune_value(condition, file, document); } for action in &mut rule.actions { - prune_slot(&mut action.span, document); - prune_slot(&mut action.identifier, document); + prune_slot(&mut action.span, file, document); + prune_slot(&mut action.identifier, file, document); for argument in &mut action.arguments { - prune_value(argument, document); + prune_value(argument, file, document); } } } @@ -280,12 +285,14 @@ 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). + /// Attaching text audits the provenance that belongs to this file: a + /// recorded span in `file` 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). Spans recorded against other files are left + /// untouched; they are audited when their own source is attached. 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; @@ -295,7 +302,7 @@ impl Program { self.provenance.as_mut(), self.files.get(file.index()).and_then(SourceFile::source), ) { - prune_spans_outside(provenance, document); + prune_spans_outside(provenance, file, document); } true } diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index abc8d65..29de7e6 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -115,6 +115,49 @@ fn file_source_retention_prunes_spans_outside_the_text() { full.validate().expect("in-bounds spans validate"); } +#[test] +fn file_source_retention_prunes_only_the_attached_file() { + // Provider-mapped programs attach each file's text in sequence (the + // documented consumer pattern): a span in a file whose source has not + // arrived yet cannot be judged and must survive every other file's + // attach (wrightkit/workshop-rs#410 regression). + let workshop = "rule (\"tick\") {\n event { Ongoing - Each Player; All; All; }\n actions { Wait(1); }\n}\n"; + let mut program = parser::parse(workshop, &catalog(), &en()).expect("parses"); + SourceMap::extract(&program) + .apply(&mut program) + .expect("map applies"); + let other = program.add_file(SourceFile::new("included.opy")); + // Re-point the rule span into the second file, as a multi-file map would. + program + .set_rule_span( + 0, + Some(Span::new(other, Position::new(1, 1), Position::new(2, 1))), + ) + .expect("file 1 span attaches"); + let name = program.rule_name_span(0).expect("file 0 name span"); + + // Attaching file 0 audits file-0 spans only. + assert!(program.set_file_source(FileId::from_index(0), workshop)); + assert_eq!(program.rule_span(0).map(|s| s.file), Some(other)); + assert_eq!(program.rule_name_span(0), Some(name)); + + // Attaching file 1 audits file-1 spans only. + assert!(program.set_file_source(other, "rule (\"tick\") {\n}\n")); + assert!(program.rule_span(0).is_some()); + assert_eq!(program.rule_name_span(0), Some(name)); + + // A file-1 span past its attached text is still pruned on its own attach. + program + .set_rule_span( + 0, + Some(Span::new(other, Position::new(1, 1), Position::new(9, 1))), + ) + .expect("file 1 span reattaches"); + assert!(program.set_file_source(other, "x")); + assert_eq!(program.rule_span(0), None); + assert_eq!(program.rule_name_span(0), Some(name)); +} + #[test] fn values_and_conditions_are_composable() { let value = Value::call(