Inline append already applied

docs/inline.md

@@ -211,6 +211,8 @@ For the staging fallback, where no viewer could be resolved and the patch is a f
 
 `Apply` returns `Applied`, `AlreadyApplied` (the literal already matches), `NotFound` (the source changed since the test run — tell the user to re-run rather than retrying), or a failure with a message (locked file, unreadable source), which is retryable.
 
+`AlreadyApplied` covers an `Append` onto a call that already has a `Snapshot` call holding this same content, which is what a multi-targeted project transitioning to inline meets: every framework fails the call site and queues an append, and whichever is accepted first writes the literal the rest are carrying. Only a chained call holding *different* content is `NotFound` — that one genuinely cannot say what it wants until it has been re-run against the literal now in the source. Accepting one framework's append before the others have run does mean the queue never sees them together, so a real disagreement between frameworks is reported as that `NotFound` rather than as a conflict to pick from.
+
 `Remove` mode patches are configuration changes with nothing to review: apply them directly; `AddInlineAsync` refuses them.
 
 

docs/mdsource/inline.source.md

@@ -204,6 +204,8 @@ For the staging fallback, where no viewer could be resolved and the patch is a f
 
 `Apply` returns `Applied`, `AlreadyApplied` (the literal already matches), `NotFound` (the source changed since the test run — tell the user to re-run rather than retrying), or a failure with a message (locked file, unreadable source), which is retryable.
 
+`AlreadyApplied` covers an `Append` onto a call that already has a `Snapshot` call holding this same content, which is what a multi-targeted project transitioning to inline meets: every framework fails the call site and queues an append, and whichever is accepted first writes the literal the rest are carrying. Only a chained call holding *different* content is `NotFound` — that one genuinely cannot say what it wants until it has been re-run against the literal now in the source. Accepting one framework's append before the others have run does mean the queue never sees them together, so a real disagreement between frameworks is reported as that `NotFound` rather than as a conflict to pick from.
+
 `Remove` mode patches are configuration changes with nothing to review: apply them directly; `AddInlineAsync` refuses them.
 
 

src/DiffEngine.Tests/InlinePatcherFsTests.cs

@@ -480,6 +480,30 @@ public async Task AppendIsRefusedWhenOneIsAlreadyChained()
         await Assert.That(reason).Contains("already has a Snapshot call");
     }
 
+    // The second framework of a multi-targeted transition, appending onto the literal the first
+    // one's accept just wrote. Read by value, so F# gets the same answer C# does even though the
+    // layout of its triple quoted literal is nothing like the content it stands for
+    [Test]
+    public async Task AppendOntoTheSameContentIsAlreadyApplied()
+    {
+        var source = Test("    Verifier.Verify(15).Snapshot(\"same\").ToTask()");
+
+        var status = TryApply(source, 5, InlinePatchMode.Append, null, "same", out _, out _);
+
+        await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied);
+    }
+
+    [Test]
+    public async Task AppendingTheSameContentTwiceIsAlreadyApplied()
+    {
+        var source = Test("    Verifier.Verify(15).ToTask()");
+
+        TryApply(source, 5, InlinePatchMode.Append, null, "a\nb", out var applied, out _);
+        var status = TryApply(applied, 5, InlinePatchMode.Append, null, "a\nb", out _, out _);
+
+        await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied);
+    }
+
     [Test]
     public async Task AppendSkipsAVerifyOnAnotherReceiver()
     {

src/DiffEngine.Tests/InlinePatcherTests.cs

@@ -821,6 +821,36 @@ public async Task AppendIsRefusedWhenOneIsAlreadyChained()
         await Assert.That(reason).Contains("already has a Snapshot call");
     }
 
+    /// <summary>
+    /// The multi-targeted transition, which is where an append lands on a chained call that is
+    /// not in its way at all. Two frameworks fail the same snapshot, each queues an append, and
+    /// accepting the first writes the literal the second is still carrying. Refused, that reads
+    /// as a failure over a source file that is already right, and sends the reader off to re-run
+    /// a test with nothing left to say.
+    /// </summary>
+    [Test]
+    public async Task AppendOntoTheSameContentIsAlreadyApplied()
+    {
+        var source = Method("        await Verify(value)\n            .Snapshot(\"new\");");
+
+        var status = TryApply(source, 5, InlinePatchMode.Append, null, "new", out _, out _);
+
+        await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied);
+    }
+
+    // The two halves of that have to agree, or the second framework's accept is refused over a
+    // literal the first one wrote from the very content being compared against it
+    [Test]
+    public async Task AppendingTheSameContentTwiceIsAlreadyApplied()
+    {
+        var source = Method("        await Verify(value);");
+
+        TryApply(source, 5, InlinePatchMode.Append, null, "a\nb", out var applied, out _);
+        var status = TryApply(applied, 5, InlinePatchMode.Append, null, "a\nb", out _, out _);
+
+        await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied);
+    }
+
     [Test]
     public async Task AppendWithNoVerifyCall()
     {

src/DiffEngine/Inline/InlineApplier.cs

@@ -40,10 +40,12 @@ public static InlineApplyResult CanApply(InlinePatch patch) =>
     /// over. Applied for yes, NotFound for no, and Failed where the source could not be read.
     /// <para>
     /// Apart from CanApply in one way, and only for Append: a call that already has a Snapshot call
-    /// chained onto it answers yes here and is refused there. Both are right. An accept has nowhere
-    /// to put the literal it is carrying and says to re-run; a producer asking whether this call
-    /// site can host an inline snapshot has its answer, and taking the verification off inline
-    /// because another process got there first would be the wrong lesson to draw.
+    /// chained onto it holding other content answers yes here and is refused there. Both are right.
+    /// An accept has nowhere to put the literal it is carrying and says to re-run; a producer
+    /// asking whether this call site can host an inline snapshot has its answer, and taking the
+    /// verification off inline because another process got there first would be the wrong lesson
+    /// to draw. Where the chained call holds this same content there is nothing to tell apart and
+    /// both say yes, CanApply as AlreadyApplied.
     /// </para>
     /// </summary>
     public static InlineApplyResult CanAnchor(InlinePatch patch) =>

src/DiffEngine/Inline/InlinePatcher.cs

@@ -433,10 +433,22 @@ static PatchStatus TryAppend(
             return PatchStatus.Applied;
         }
 
-        var insertAt = WalkChain(source, scan, closeParen + 1, methodName, out var alreadyChained);
-        // Another process may have appended one between the run and the accept
-        if (alreadyChained)
+        var insertAt = WalkChain(source, scan, closeParen + 1, methodName, out var chained);
+        // Another process may have appended one between the run and the accept, and two
+        // frameworks failing the same call site is the ordinary way that happens: each queues an
+        // append, and accepting the first leaves the second with nowhere to put a literal that is
+        // already there. Only the content tells the two apart. The same snapshot is done, and
+        // saying so matters - a refusal reads as a failure, and the reader who sent two identical
+        // snapshots and got one applied and one rejected has no way to see that their source is
+        // already right. A different one is a call site that cannot say what it wants until it has
+        // been re-run against the literal it now has.
+        if (chained >= 0)
         {
+            if (HoldsContent(source, scan, chained, newContent))
+            {
+                return PatchStatus.AlreadyApplied;
+            }
+
             failReason = $"The call near line {lineHint} already has a {methodName} call. Re-run the test.";
             return PatchStatus.NotFound;
         }
@@ -454,6 +466,31 @@ static PatchStatus TryAppend(
         return PatchStatus.Applied;
     }
 
+    /// <summary>
+    /// Whether the call at <paramref name="openParen"/> already carries
+    /// <paramref name="content"/> as its expected argument.
+    /// <para>
+    /// What the argument means rather than what it says, so a literal the append would have
+    /// written in another shape - a different delimiter, a different indent - still counts as the
+    /// same snapshot. Anything that is not a literal at all, or is hidden behind another named
+    /// argument, is not this content: no answer can be read out of it, and the caller's other
+    /// branch says to re-run, which is where a call site nobody can make sense of belongs.
+    /// </para>
+    /// </summary>
+    static bool HoldsContent(string source, SourceScan scan, int openParen, string content)
+    {
+        if (!TryReadArguments(source, scan, openParen, out var expected) ||
+            expected.IsAbsent ||
+            expected.BlockedByName)
+        {
+            return false;
+        }
+
+        var argument = source.Substring(expected.Start, expected.End - expected.Start);
+        return scan.Language.TryParse(argument, out var value) &&
+               value == content;
+    }
+
     /// <summary>
     /// Removes the Snapshot call, along with the whitespace and line break that preceded it so no
     /// blank line is left behind.
@@ -533,11 +570,13 @@ static PatchStatus TryRemove(
     /// Walks the calls chained onto an invocation and returns where a call should be appended:
     /// the end of the chain, or the point in front of the language's
     /// <see cref="SourceLanguage.ChainTerminator"/> when the chain ends in one.
-    /// <paramref name="found"/> is set when one of them is a call to <paramref name="name"/>.
+    /// <paramref name="found"/> is the open paren of the first call to <paramref name="name"/>
+    /// among them, or -1 where there is none. The position rather than the fact of it, because a
+    /// caller deciding what to do about one has to read its argument.
     /// </summary>
-    static int WalkChain(string source, SourceScan scan, int index, string name, out bool found)
+    static int WalkChain(string source, SourceScan scan, int index, string name, out int found)
     {
-        found = false;
+        found = -1;
         var terminator = scan.Language.ChainTerminator;
         // Where the chain was before the terminating call, which is where an appended one goes:
         // in front of the terminator, and behind the whitespace and line break that introduced it
@@ -569,9 +608,10 @@ static int WalkChain(string source, SourceScan scan, int index, string name, out
                 break;
             }
 
-            if (IsCall(source, nameStart, cursor, name))
+            if (found < 0 &&
+                IsCall(source, nameStart, cursor, name))
             {
-                found = true;
+                found = paren;
             }
 
             if (terminator != null &&

src/Directory.Build.props

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Project>
   <PropertyGroup>
-    <Version>20.1.1</Version>
+    <Version>20.1.2</Version>
     <AssemblyVersion>1.0.0</AssemblyVersion>
     <PackageTags>Testing, Snapshot, Diff, Compare</PackageTags>
     <Description>Launches diff tools based on file extensions. Designed to be consumed by snapshot testing libraries.</Description>