Skip to content

fix(eliminate): preserve inline comments and blank lines via span-based text patching#59

Merged
NikolaRHristov merged 3 commits into
Currentfrom
fix/eliminate-preserve-comments
May 25, 2026
Merged

fix(eliminate): preserve inline comments and blank lines via span-based text patching#59
NikolaRHristov merged 3 commits into
Currentfrom
fix/eliminate-preserve-comments

Conversation

@NikolaRHristov
Copy link
Copy Markdown
Member

Fixes the root cause of the formatting regression: inline comments and blank lines being dropped whenever any elimination occurred.

Problem

Transform::Run called prettyplease::unparse(&Ast) on the entire syn::File whenever any binding was eliminated. prettyplease only sees the AST; it has no view of // inline comments, blank lines, or original spacing. Every touched file was reprinted in prettyplease canonical style, altering regions of the file that had nothing to do with the eliminated bindings.

Fix

New: Transform/Patch.rs

Provides three primitives:

  • LineColToByte(source, line, col) - converts 1-based line / 0-based col (proc_macro2 convention) to a byte offset in the source string.
  • SpanBytes(span, source) - extracts (start, end) byte offsets from a proc_macro2::Span. Returns None when span-location data is absent (triggers fallback).
  • StmtLineRange(stmt, source) - returns the full line range of a statement including leading indent and trailing newline, so deletion leaves no blank lines.
  • ApplyPatches(source, patches) - applies a sorted non-overlapping slice of Patch structs (byte-range replacements) against the source string. Returns None on overlap or out-of-bounds.

Changed: Transform/mod.rs

Run now calls TryPatchSource(source, mutated_ast) after elimination:

  1. Re-parses Source into OriginalAst whose Spans are anchored to Source bytes.
  2. Walks OriginalAst and MutatedAst in parallel over function items.
  3. CollectBlockPatches aligns statements between original and mutated blocks by token-stream equality. Statements present in the original but absent in the mutated block are emitted as delete patches. Statements that are present in both but have different token text (substituted identifier) are emitted as replace patches.
  4. Patches are sorted by start offset and passed to ApplyPatches.
  5. If any span resolution returns None, TryPatchSource returns None and Run falls back to prettyplease::unparse - behaviour is never worse than before.

Tests added

In Patch::Tests:

  • ApplyNoPatches - identity case
  • ApplySingleDelete - range delete
  • ApplySingleReplace - range replace
  • ApplyTwoPatches - two disjoint patches
  • OverlappingPatchesReturnNone - overlap guard
  • OutOfBoundsPatchReturnsNone - bounds guard
  • LineColToByteFirstLine / LineColToByteSecondLine / LineColToByteOutOfRange

…ed text patching

Fixes the root cause of the formatting regression reported in the issue.

Before this change, Transform::Run called prettyplease::unparse on the
entire syn::File whenever any elimination occurred. prettyplease only
sees the AST; it has no view of inline // comments, blank lines, or
original spacing trivia. Every touched file was fully reprinted in
prettyplease canonical style, which removed comments and restructured
whitespace across regions of the file that had nothing to do with the
eliminated bindings.

This change replaces the full reprint with a span-based text-patch
approach:

1. Run::Run now takes the original source string and the post-elimination
   AST side-by-side.
2. A new Patch module collects the byte-span of each removed let
   statement and the byte-span of the use site where the initialiser
   was substituted, sourced from proc_macro2 span start/end offsets on
   the original parsed tokens.
3. PatchSource applies those edits as a sorted, non-overlapping set of
   byte-range replacements against the original source string, leaving
   every other byte - including comments, blank lines, and all
   formatting trivia - untouched.
4. If span information is unavailable (proc_macro2 built without
   span-locations feature, or spans are call_site), the pipeline falls
   back to prettyplease so behaviour is never worse than before.

The fallback path keeps all existing tests passing without modification.
New tests in Patch.rs verify the splice logic directly.
Signed-off-by: Nikola Hristov <Nikola@PlayForm.Cloud>
@NikolaRHristov NikolaRHristov merged commit 8f1b83b into Current May 25, 2026
1 check passed
@NikolaRHristov NikolaRHristov deleted the fix/eliminate-preserve-comments branch May 25, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant