From b4944b7ff1607dc9cfe6f5da8a83792c3d7f4c99 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 09:56:05 +1000 Subject: [PATCH] Anchor a retire to the snapshot it was sent for Set and Append locate the call by content, and say why: the same literal is just as likely to sit in another test, in a comment, or in the verify call on the same line, so rewriting whichever call the hint lands on leaves the wrong snapshot changed. Remove had exactly that problem with none of the protection - it took the nearest call to the recorded line and deleted it, ignoring the anchor entirely. The recorded line stops being true as soon as anything above it is edited, which is the ordinary state of a source file between a test run and a retire. So a stale hint retired the snapshot in the test next door and reported Applied, and the snapshot that should have gone was still there. TryFindAnchoredCall matches on the expected argument the way the Set path does, by expression where there is one and by parsed value otherwise, and falls back to nearest-to-the-hint only when no anchor was sent at all - which is what a producer whose language withholds CallerArgumentExpression and sends no value either can offer. An anchor that matches nothing is now reported rather than resolved to something else. --- src/DiffEngine.Tests/InlinePatcherTests.cs | 34 ++++++++ src/DiffEngine/Inline/InlinePatcher.cs | 90 +++++++++++++++++++++- 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/src/DiffEngine.Tests/InlinePatcherTests.cs b/src/DiffEngine.Tests/InlinePatcherTests.cs index 6c7db593..601f2987 100644 --- a/src/DiffEngine.Tests/InlinePatcherTests.cs +++ b/src/DiffEngine.Tests/InlinePatcherTests.cs @@ -123,6 +123,40 @@ public async Task ASnapshotBeforeAVerbatimStringOpeningOnAnEscapedQuoteIsStillFo const string q3 = "\"\"\""; const string q4 = "\"\"\"\""; + /// + /// A retire is anchored the same way a set is. It used to delete whichever call sat nearest + /// the recorded line, and that line stops being true the moment anything above it is edited - + /// so a stale hint retired the snapshot in the test next door and reported Applied. + /// + [Test] + public async Task RemoveTakesTheCallTheAnchorNamesRatherThanTheNearest() + { + var source = Method( + " await A().Snapshot(\"one\");\n" + + " await B().Snapshot(\"two\");"); + + // Hint on the second call, anchor on the first + var status = TryApply(source, 6, InlinePatchMode.Remove, "\"one\"", "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).DoesNotContain("\"one\""); + // The other test's snapshot is left alone + await Assert.That(newSource).Contains("Snapshot(\"two\")"); + } + + /// + /// And an anchor that matches nothing is reported rather than resolved to the nearest call. + /// + [Test] + public async Task RemoveReportsWhenTheAnchorIsGone() + { + var source = Method(" await A().Snapshot(\"two\");"); + + var status = TryApply(source, 5, InlinePatchMode.Remove, "\"one\"", "", out _, out var reason); + + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("still the one the test run saw"); + } [Test] public async Task ReplaceRegularLiteral() { diff --git a/src/DiffEngine/Inline/InlinePatcher.cs b/src/DiffEngine/Inline/InlinePatcher.cs index e44cace5..d1cbabc4 100644 --- a/src/DiffEngine/Inline/InlinePatcher.cs +++ b/src/DiffEngine/Inline/InlinePatcher.cs @@ -80,7 +80,7 @@ public static PatchStatus TryApply( if (mode == InlinePatchMode.Remove) { - return TryRemove(source, scan, lineStarts, lineHint, memberLine, ref newSource, ref failReason); + return TryRemove(language, source, scan, lineStarts, lineHint, memberLine, originalExpression, originalValue, eol, ref newSource, ref failReason); } var fileUnit = DetectIndentUnit(source, scan, lineStarts); @@ -385,17 +385,24 @@ static PatchStatus TryAppend( /// blank line is left behind. /// static PatchStatus TryRemove( + SourceLanguage language, string source, SourceScan scan, List lineStarts, int lineHint, int? memberLine, + string? originalExpression, + string? originalValue, + string eol, ref string newSource, ref string failReason) { - if (!TryFindCall(source, scan, lineStarts, lineHint, memberLine, snapshotName, false, out var nameStart, out var openParen)) + var anchored = !string.IsNullOrEmpty(originalExpression) || originalValue != null; + if (!TryFindAnchoredCall(language, source, scan, lineStarts, lineHint, memberLine, originalExpression, originalValue, eol, out var nameStart, out var openParen)) { - failReason = $"Could not find a {methodName} call near line {lineHint}. The source may have changed since the test run. Re-run the test."; + failReason = anchored + ? $"Could not find a {methodName} call near line {lineHint} whose expected argument is still the one the test run saw. The source may have changed since the test run. Re-run the test." + : $"Could not find a {methodName} call near line {lineHint}. The source may have changed since the test run. Re-run the test."; return PatchStatus.NotFound; } @@ -525,6 +532,83 @@ static string LeadingWhitespace(string source, List lineStarts, int offset) static readonly string[] snapshotName = [methodName]; + /// + /// The call the anchor names, rather than whichever one sits nearest the hint. + /// + /// Set and Append locate by content for a reason - the same literal is just as likely to be in + /// the test next door - and a Remove has exactly the same problem with none of the protection. + /// It deleted the nearest call to a line number that stops being true as soon as anything + /// above it is edited, so a stale hint retired somebody else's snapshot and reported Applied. + /// + /// + /// With no anchor there is nothing to match on and nearest-to-the-hint is all there is, which + /// is the case for a producer whose language withholds CallerArgumentExpression and sends no + /// value either. + /// + /// + static bool TryFindAnchoredCall( + SourceLanguage language, + string source, + SourceScan scan, + List lineStarts, + int lineHint, + int? memberLine, + string? originalExpression, + string? originalValue, + string eol, + out int nameStart, + out int openParen) + { + if (string.IsNullOrEmpty(originalExpression) && + originalValue == null) + { + return TryFindCall(source, scan, lineStarts, lineHint, memberLine, snapshotName, false, out nameStart, out openParen); + } + + // ReSharper disable once RedundantSuppressNullableWarningExpression + var needle = string.IsNullOrEmpty(originalExpression) ? null : NormalizeTo(originalExpression!, eol); + var previous = originalValue == null ? null : SourceLanguage.NormalizeNewlines(originalValue); + + foreach (var (candidateName, candidateParen) in FindCalls(source, scan, lineStarts, lineHint, memberLine, snapshotName, false)) + { + if (!TryReadArguments(source, scan, candidateParen, out var expected)) + { + continue; + } + + if (needle != null) + { + if (!expected.Matches(source, needle)) + { + continue; + } + } + else + { + if (expected.IsAbsent || + expected.BlockedByName) + { + continue; + } + + var argument = source.Substring(expected.Start, expected.End - expected.Start); + if (!language.TryParse(argument, out var value) || + value != previous) + { + continue; + } + } + + nameStart = candidateName; + openParen = candidateParen; + return true; + } + + nameStart = -1; + openParen = -1; + return false; + } + static bool TryFindCall(string source, SourceScan scan, List lineStarts, int lineHint, int? memberLine, out int openParen) => TryFindCall(source, scan, lineStarts, lineHint, memberLine, snapshotName, false, out _, out openParen);