Repository navigation
fix(program): audit only the attached file's spans when retaining source - #418
Conversation
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.
e54-bot
left a comment
There was a problem hiding this comment.
Reviewed at 773e685 (worktree wt-410-prune-fix). Verdict: approve-equivalent — the fix is correct and minimal.
Fix verification (crates/workshop-rs/src/program.rs:126-169): prune_slot now prunes only when span.file == file && document.byte_range(*span).is_none(). The gate is sound: SourceFile::set_source binds document.file = self.file (core/source.rs:45), and add_file always calls bind_file (program.rs:255), so document.file == Some(file) holds for every document set_file_source produces — span.file == file is exactly the complement of byte_range's file-mismatch rejection (core/source.rs:115-117). Foreign-file spans survive; same-file unresolvable spans still prune; None slots are skipped via if let Some.
No validation hazard from retained foreign spans: wir::validate::check_span (wir/validate.rs:274-282) only checks a span when its own file has an attached source, so a span in a not-yet-attached file cannot fail validation while it waits for its own attach.
Test (tests/program_model.rs:118-159) reproduces the reported scenario faithfully: two files, sequential set_file_source attaches per the documented consumer pattern. On the pre-fix code, attaching file 0 clears the file-1 rule span (byte_range → None on file mismatch), so assert_eq!(program.rule_span(0).map(|s| s.file), Some(other)) fails — the test is a genuine regression guard. The third block also confirms a file-1 span past its own attached text still prunes on its own attach.
Verification run: cargo test -p workshop-rs --test integration file_source → 3/3 pass; cargo clippy -p workshop-rs --all-targets clean.
Summary
Post-merge review of #410 found that
set_file_sourceaudited every provenance span against the newly attached document — andSourceDocument::byte_rangereturnsNonefor any span whosefilediffers, not just out-of-bounds extents. The documented consumer pattern (attach each mapped file's text in sequence, as wright#602 does) therefore cleared every earlier file's spans when the next source arrived: after the loop, zero provenance remained for any multi-file provider-mapped program (imports,#!include, script-macro files).prune_spans_outsidenow takes the attachedFileIdand clears a slot only whenspan.file == fileand the span fails to resolve — a span in another file is not stale, its own source simply has not arrived yet.Test plan
file_source_retention_prunes_only_the_attached_file: two-file program, sequential attaches — foreign-file spans survive both attaches; a file-1 span past its own attached text is still pruned on its own attachcargo test --workspace --all-targets,cargo fmt,cargo clippy -D warningsclean