Fix chunk boundary scrubber bugs and test adapter defects (#1779)

SimonCropp · actions-user · web-flow · commit c5532a8266dc · 2026-07-09T07:23:02.000+10:00
* Fix chunk-boundary scrubber bugs and test-adapter defects

Scrubbers (Verify core):
- UserMachineScrubber: fix IndexOutOfRangeException when a match ends exactly
  on a chunk boundary with a following chunk (read the trailing char via the
  builder indexer instead of chunkSpan[neededFromCurrent])
- UserMachineScrubber, GuidScrubber, DirectoryReplacements: roll the carryover
  forward across chunks so a token spanning three or more chunks (with a middle
  chunk shorter than the token) is no longer silently left unscrubbed
- DateFormatLengthCalculator: 'K' contributes 0 to the minimum length (it can
  render as "", "Z" or "+11:00"), so round-trip/"o" formats now scrub the Z and
  offset-less forms instead of only the offset form
- LinesScrubber.ReplaceLines: guard the trailing-newline trim on the rebuilt
  builder length, not the original string length, fixing an
  ArgumentOutOfRangeException when every line is replaced with null

Test-framework adapters:
- MSTest Verifier: only apply TestData parameters when their count matches the
  method parameter count (matches XunitV3; params-array DataRows no longer break
  parameterized snapshot naming)
- MSTest Verifier_Archive: forward the dropped archiveExtension in the
  VerifyZip(Stream) overload
- MSTest TestExecutionContext: resolve Method lazily so the method scan runs
  only when a verifier is built
- MSTest source generator: require an attribute list in the syntax predicate so
  the uncached CreateSyntaxProvider transform no longer runs semantic work for
  every class on every keystroke
- NUnit VerifyBase_Tuple: pass sourceFile so snapshots resolve to the test's
  path rather than the package build path
- NUnit VerifyBase_Stream: use settings ?? this.settings in the Task&lt;T&gt; and
  ValueTask&lt;T&gt; stream overloads so instance settings are honored

Add repro tests for the scrubber fixes.

* Docs changes

---------

Co-authored-by: GitHub Action &lt;action@github.com&gt;
diff --git a/docs/guids.md b/docs/guids.md
@@ -182,7 +182,7 @@ public Task NamedGuidFluent()
         .AddNamedGuid(guid, "instanceNamed");
 }
 ```
-<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L104-L118' title='Snippet source file'>snippet source</a> | <a href='#snippet-NamedGuidFluent' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L121-L135' title='Snippet source file'>snippet source</a> | <a href='#snippet-NamedGuidFluent' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -217,7 +217,7 @@ public Task InferredNamedGuidFluent()
         .AddNamedGuid(namedGuid);
 }
 ```
-<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L120-L134' title='Snippet source file'>snippet source</a> | <a href='#snippet-InferredNamedGuidFluent' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L137-L151' title='Snippet source file'>snippet source</a> | <a href='#snippet-InferredNamedGuidFluent' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Result: 
diff --git a/src/Verify.MSTest.SourceGenerator/UsesVerifyGenerator.cs b/src/Verify.MSTest.SourceGenerator/UsesVerifyGenerator.cs
@@ -118,8 +118,12 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
     static bool HasTestClassAttribute(INamedTypeSymbol symbol, INamedTypeSymbol testClassType) =>
         !symbol.HasAttributeOfType(testClassType, includeDerived: true);
 
+    // Require at least one attribute list: both paths only care about classes
+    // carrying [UsesVerify] or [TestClass]. Without this filter the uncached
+    // CreateSyntaxProvider transform runs semantic work for every class in the
+    // consuming project on every keystroke.
     static bool IsSyntaxEligibleForGeneration(SyntaxNode node, Cancel _) =>
-        node is ClassDeclarationSyntax;
+        node is ClassDeclarationSyntax { AttributeLists.Count: > 0 };
 
     static bool IsAssemblyEligibleForGeneration(IAssemblySymbol assembly, INamedTypeSymbol markerType) =>
         assembly.HasAttributeOfType(markerType, includeDerived: false);
diff --git a/src/Verify.MSTest/TestExecutionContext.cs b/src/Verify.MSTest/TestExecutionContext.cs
@@ -3,7 +3,12 @@ namespace VerifyMSTest;
 public record TestExecutionContext(TestContext TestContext, Type TestClass)
 {
     public Assembly Assembly { get; } = TestClass.Assembly;
-    public MethodInfo Method { get; } = FindMethod(TestClass, TestContext);
+
+    MethodInfo? method;
+
+    // Resolved lazily: the method scan is only needed when a Verify call actually
+    // builds a verifier, not for every test that constructs a context.
+    public MethodInfo Method => method ??= FindMethod(TestClass, TestContext);
 
     static MethodInfo FindMethod(Type type, TestContext context)
     {
diff --git a/src/Verify.MSTest/Verifier.cs b/src/Verify.MSTest/Verifier.cs
@@ -33,7 +33,12 @@ public static InnerVerifier BuildVerifier(VerifySettings settings, string source
         if (!settings.HasParameters)
         {
             var data = context.TestContext.TestData;
-            if (data != null)
+            // Only apply when the data length matches the method parameter count.
+            // A params-array DataRow exposes raw pre-binding data whose length does
+            // not match the parameter count, which would break parameterized
+            // snapshot file naming. XunitV3 applies the same guard.
+            if (data != null &&
+                data.Length == method.ParameterNames()?.Count)
             {
                 settings.SetParameters(data);
             }
diff --git a/src/Verify.MSTest/Verifier_Archive.cs b/src/Verify.MSTest/Verifier_Archive.cs
@@ -47,7 +47,7 @@ public static SettingsTask VerifyZip(
         bool persistArchive = false,
         string? archiveExtension = null,
         [CallerFilePath] string sourceFile = "") =>
-        Verify(settings, sourceFile, _ => _.VerifyZip(stream, include, info, fileScrubber, includeStructure, persistArchive), true);
+        Verify(settings, sourceFile, _ => _.VerifyZip(stream, include, info, fileScrubber, includeStructure, persistArchive, archiveExtension), true);
 
     /// <summary>
     /// Verifies the contents of a <see cref="ZipArchive" />
diff --git a/src/Verify.NUnit/VerifyBase_Stream.cs b/src/Verify.NUnit/VerifyBase_Stream.cs
@@ -40,7 +40,7 @@ public SettingsTask Verify<T>(
         VerifySettings? settings = null,
         object? info = null)
         where T : Stream =>
-        Verifier.Verify(target, extension, settings, info, sourceFile);
+        Verifier.Verify(target, extension, settings ?? this.settings, info, sourceFile);
 
     [Pure]
     public SettingsTask Verify<T>(
@@ -49,7 +49,7 @@ public SettingsTask Verify<T>(
         VerifySettings? settings = null,
         object? info = null)
         where T : Stream =>
-        Verifier.Verify(target, extension, settings, info, sourceFile);
+        Verifier.Verify(target, extension, settings ?? this.settings, info, sourceFile);
 
     [Pure]
     public SettingsTask Verify(
diff --git a/src/Verify.NUnit/VerifyBase_Tuple.cs b/src/Verify.NUnit/VerifyBase_Tuple.cs
@@ -7,6 +7,6 @@ public partial class VerifyBase
     public SettingsTask VerifyTuple(
         Expression<Func<ITuple>> target,
         VerifySettings? settings = null) =>
-        Verifier.VerifyTuple(target, settings ?? this.settings);
+        Verifier.VerifyTuple(target, settings ?? this.settings, sourceFile);
 }
 #endif
diff --git a/src/Verify.Tests/DateFormatLengthCalculatorTests.cs b/src/Verify.Tests/DateFormatLengthCalculatorTests.cs
@@ -43,8 +43,11 @@
     [InlineData("zz", 3, 3)]
     [InlineData("zzz", 6, 6)]
     [InlineData("zzzz", 6, 6)]
-    [InlineData("K", 6, 6)]
-    [InlineData("KK", 12, 12)]
+    // K renders as "" (Unspecified), "Z" (Utc, 1 char) or "+11:00" (offset, 6 chars),
+    // so its minimum contribution is 0 (not 6) — otherwise round-trip/"o" formats
+    // scrub only the offset form and leak the Z / offset-less forms.
+    [InlineData("K", 6, 0)]
+    [InlineData("KK", 12, 0)]
     [InlineData(":", 1, 1)]
     [InlineData("':'", 1, 1)]
     [InlineData("/", 1, 1)]
diff --git a/src/Verify.Tests/GuidScrubberTests.cs b/src/Verify.Tests/GuidScrubberTests.cs
@@ -101,6 +101,23 @@ public void MultipleChunks()
         Assert.Equal("[Guid_1][Guid_2]", builder.ToString());
     }
 
+    [Fact]
+    public void GuidSpanningThreeChunks()
+    {
+        // Capacity 4 forces small chunks [4][4][rest]; the 36-char guid spans all
+        // three, and the 4-char middle chunk is shorter than the 35-char carryover.
+        // The carryover must accumulate across chunks or the prefix is dropped and
+        // the guid is never found (silent leak).
+        var guid = "173535ae-995b-4cc6-a74e-8cd4be57039c";
+        var builder = new StringBuilder(capacity: 4);
+        builder.Append(guid[..4]);  // chunk0
+        builder.Append(guid[4..8]); // chunk1 (short middle chunk)
+        builder.Append(guid[8..]);  // chunk2
+        using var counter = Counter.Start();
+        GuidScrubber.ReplaceGuids(builder, counter);
+        Assert.Equal("Guid_1", builder.ToString());
+    }
+
     #region NamedGuidFluent
 
     [Fact]
diff --git a/src/Verify.Tests/LinesScrubberTests.cs b/src/Verify.Tests/LinesScrubberTests.cs
@@ -78,6 +78,29 @@ public Task ScrubLinesContaining_case_sensitive()
                     """);
     }
 
+    [Fact]
+    public void ReplaceLines_AllRemovedNoTrailingNewline()
+    {
+        // Single line, no trailing newline, and every line replaced with null.
+        // The trailing-newline trim must guard on the rebuilt builder length, not
+        // the original string length, otherwise Length -= 1 underflows.
+        var builder = new StringBuilder("single line");
+
+        builder.ReplaceLines(_ => null);
+
+        Assert.Equal(string.Empty, builder.ToString());
+    }
+
+    [Fact]
+    public void ReplaceLines_ReplacesLines()
+    {
+        var builder = new StringBuilder("a\nb\nc");
+
+        builder.ReplaceLines(line => line == "b" ? "B" : line);
+
+        Assert.Equal("a\nB\nc", builder.ToString());
+    }
+
     [Fact]
     public void FilterLines_RemovesSingleLine()
     {
diff --git a/src/Verify.Tests/Serialization/DirectoryReplacementTests.cs b/src/Verify.Tests/Serialization/DirectoryReplacementTests.cs
@@ -45,6 +45,22 @@ public void MultipleChunks()
         Assert.Equal("{Child} {Parent} ", builder.ToString());
     }
 
+    [Fact]
+    public void PathSpanningThreeChunks()
+    {
+        // Capacity 4 forces small chunks [4][4][rest]; the 16-char path spans all
+        // three, and the 4-char middle chunk is shorter than the carryover. The
+        // carryover must accumulate across chunks or the prefix is dropped and the
+        // path leaks unscrubbed.
+        List<DirectoryReplacements.Pair> pairs = [new("C:/Parent/ChildX", "{replace}")];
+        var builder = new StringBuilder(capacity: 4);
+        builder.Append("C:/P"); // chunk0
+        builder.Append("aren"); // chunk1 (short middle chunk)
+        builder.Append("t/ChildX.");
+        DirectoryReplacements.Replace(builder, pairs);
+        Assert.Equal("{replace}.", builder.ToString());
+    }
+
     [Fact]
     public void ProcessLongerDirectoryFirst()
     {
diff --git a/src/Verify.Tests/UserMachineScrubberChunkTests.cs b/src/Verify.Tests/UserMachineScrubberChunkTests.cs
@@ -0,0 +1,38 @@
+public class UserMachineScrubberChunkTests
+{
+    [Fact]
+    public void CrossChunkMatchEndingExactlyAtChunkBoundary()
+    {
+        // "ABCD" fills the capacity-4 chunk; the next append lands in a fresh
+        // 16-char chunk holding the remaining 16 chars of the 20-char match, so
+        // the match ends exactly at that chunk's boundary with a chunk after it.
+        // The trailing-char check must not read chunkSpan[chunkSpan.Length].
+        var builder = new StringBuilder(capacity: 4);
+        builder.Append("ABCD");
+        builder.Append("EFGHIJKLMNOPQRST");
+        builder.Append('.');
+
+        UserMachineScrubber.PerformReplacements(builder, "ABCDEFGHIJKLMNOPQRST", "TheUserName");
+
+        Assert.Equal("TheUserName.", builder.ToString());
+    }
+
+    [Fact]
+    public void TokenSpanningThreeChunks()
+    {
+        // Capacity 4 forces small chunks [4][4][…]. The 10-char match spans all
+        // three, and the 4-char middle chunk is shorter than the match. The
+        // carryover must accumulate across chunks; otherwise the first chunk's
+        // prefix is dropped when the short middle chunk overwrites it, and the
+        // match is never found (silent leak).
+        var find = "ABCDEFGHIJ"; // 10 chars
+        var builder = new StringBuilder(capacity: 4);
+        builder.Append("ABCD"); // chunk0 = find[0..4]
+        builder.Append("EFGH"); // chunk1 = find[4..8] (short middle chunk)
+        builder.Append("IJ.");  // chunk2 = find[8..10] + wrapper
+
+        UserMachineScrubber.PerformReplacements(builder, find, "TheUserName");
+
+        Assert.Equal("TheUserName.", builder.ToString());
+    }
+}
diff --git a/src/Verify/Serialization/Scrubbers/DateFormatLengthCalculator.cs b/src/Verify/Serialization/Scrubbers/DateFormatLengthCalculator.cs
@@ -179,8 +179,10 @@ public static (int max, int min) InnerGetLength(scoped CharSpan format, Culture
 
                     break;
                 case 'K':
+                    // K renders as "" (Unspecified), "Z" (Utc, 1 char) or
+                    // "+11:00" (offset, 6 chars), so it can contribute as few as
+                    // 0 chars. Only the maximum is 6; the minimum stays 0.
                     tokenLen = 1;
-                    minLength += 6;
                     maxLength += 6;
                     break;
                 case ':':
diff --git a/src/Verify/Serialization/Scrubbers/DirectoryReplacements_StringBuilder.cs b/src/Verify/Serialization/Scrubbers/DirectoryReplacements_StringBuilder.cs
@@ -169,9 +169,22 @@ static List<Match> FindMatches(StringBuilder builder, List<Pair> pairs)
                 }
             }
 
-            // Save last N chars for next iteration
-            carryoverLength = Math.Min(carryoverSize, chunk.Length);
-            chunkSpan.Slice(chunk.Length - carryoverLength, carryoverLength).CopyTo(carryoverBuffer);
+            // Roll the carryover forward: keep the last carryoverSize chars of
+            // everything seen so far. Rebuilding it from the current chunk alone
+            // drops the prefix when a chunk is shorter than carryoverSize, so a
+            // path spanning three or more chunks would never be found.
+            if (chunk.Length >= carryoverSize)
+            {
+                chunkSpan.Slice(chunk.Length - carryoverSize, carryoverSize).CopyTo(carryoverBuffer);
+                carryoverLength = carryoverSize;
+            }
+            else
+            {
+                var keep = Math.Min(carryoverLength, carryoverSize - chunk.Length);
+                carryoverBuffer.Slice(carryoverLength - keep, keep).CopyTo(carryoverBuffer);
+                chunkSpan.CopyTo(carryoverBuffer[keep..]);
+                carryoverLength = keep + chunk.Length;
+            }
 
             previousChunkAbsoluteEnd = absolutePosition + chunk.Length;
             absolutePosition += chunk.Length;
diff --git a/src/Verify/Serialization/Scrubbers/GuidScrubber.cs b/src/Verify/Serialization/Scrubbers/GuidScrubber.cs
@@ -117,9 +117,22 @@ static List<Match> FindMatches(StringBuilder builder, Counter counter)
                 }
             }
 
-            // Save last 35 chars for next iteration
-            carryoverLength = Math.Min(35, chunk.Length);
-            chunkSpan.Slice(chunk.Length - carryoverLength, carryoverLength).CopyTo(carryoverBuffer);
+            // Roll the carryover forward: keep the last 35 chars of everything seen
+            // so far. Rebuilding it from the current chunk alone drops the prefix
+            // when a chunk is shorter than 35, so a guid spanning three or more
+            // chunks would never be found.
+            if (chunk.Length >= 35)
+            {
+                chunkSpan.Slice(chunk.Length - 35, 35).CopyTo(carryoverBuffer);
+                carryoverLength = 35;
+            }
+            else
+            {
+                var keep = Math.Min(carryoverLength, 35 - chunk.Length);
+                carryoverBuffer.Slice(carryoverLength - keep, keep).CopyTo(carryoverBuffer);
+                chunkSpan.CopyTo(carryoverBuffer[keep..]);
+                carryoverLength = keep + chunk.Length;
+            }
 
             previousChunkAbsoluteEnd = absolutePosition + chunk.Length;
             absolutePosition += chunk.Length;
diff --git a/src/Verify/Serialization/Scrubbers/LinesScrubber.cs b/src/Verify/Serialization/Scrubbers/LinesScrubber.cs
@@ -31,7 +31,7 @@ public static void ReplaceLines(this StringBuilder input, Func<string, string?>
             }
         }
 
-        if (theString.Length > 0 &&
+        if (input.Length > 0 &&
             !theString.EndsWith('\n'))
         {
             input.Length -= 1;
diff --git a/src/Verify/Serialization/Scrubbers/UserMachineScrubber_PerformReplacements.cs b/src/Verify/Serialization/Scrubbers/UserMachineScrubber_PerformReplacements.cs
@@ -72,10 +72,13 @@ static List<int> FindMatches(StringBuilder builder, string find)
                         continue;
                     }
 
-                    // Check trailing character
+                    // Check trailing character. Use the builder indexer rather
+                    // than chunkSpan[neededFromCurrent], which is out of range when
+                    // the match ends exactly at this chunk's boundary, so the check
+                    // still works when there is a following chunk.
                     var endPosition = startPosition + find.Length;
                     var validEnd = endPosition >= builder.Length ||
-                                   IsValidWrapper(chunkSpan[neededFromCurrent]);
+                                   IsValidWrapper(builder[endPosition]);
 
                     if (!validEnd)
                     {
@@ -115,9 +118,22 @@ static List<int> FindMatches(StringBuilder builder, string find)
                 }
             }
 
-            // Save last N chars for next iteration
-            carryoverLength = Math.Min(carryoverSize, chunk.Length);
-            chunkSpan.Slice(chunk.Length - carryoverLength, carryoverLength).CopyTo(carryoverBuffer);
+            // Roll the carryover forward: keep the last carryoverSize chars of
+            // everything seen so far. Rebuilding it from the current chunk alone
+            // drops the prefix when a chunk is shorter than the search string, so
+            // a token spanning three or more chunks would never be found.
+            if (chunk.Length >= carryoverSize)
+            {
+                chunkSpan.Slice(chunk.Length - carryoverSize, carryoverSize).CopyTo(carryoverBuffer);
+                carryoverLength = carryoverSize;
+            }
+            else
+            {
+                var keep = Math.Min(carryoverLength, carryoverSize - chunk.Length);
+                carryoverBuffer.Slice(carryoverLength - keep, keep).CopyTo(carryoverBuffer);
+                chunkSpan.CopyTo(carryoverBuffer[keep..]);
+                carryoverLength = keep + chunk.Length;
+            }
 
             previousChunkAbsoluteEnd = absolutePosition + chunk.Length;
             absolutePosition += chunk.Length;

Original file line number	Diff line number	Diff line change
`@@ -182,7 +182,7 @@ public Task NamedGuidFluent()`
`182`	`182`	`.AddNamedGuid(guid, "instanceNamed");`
`183`	`183`	`}`
`184`	`184`	```
`185`		`-<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L104-L118' title='Snippet source file'>snippet source</a> \| <a href='#snippet-NamedGuidFluent' title='Start of snippet'>anchor</a></sup>`
	`185`	`+<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L121-L135' title='Snippet source file'>snippet source</a> \| <a href='#snippet-NamedGuidFluent' title='Start of snippet'>anchor</a></sup>`
`186`	`186`	`<!-- endSnippet -->`
`187`	`187`
`188`	`188`
`@@ -217,7 +217,7 @@ public Task InferredNamedGuidFluent()`
`217`	`217`	`.AddNamedGuid(namedGuid);`
`218`	`218`	`}`
`219`	`219`	```
`220`		`-<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L120-L134' title='Snippet source file'>snippet source</a> \| <a href='#snippet-InferredNamedGuidFluent' title='Start of snippet'>anchor</a></sup>`
	`220`	`+<sup><a href='/src/Verify.Tests/GuidScrubberTests.cs#L137-L151' title='Snippet source file'>snippet source</a> \| <a href='#snippet-InferredNamedGuidFluent' title='Start of snippet'>anchor</a></sup>`
`221`	`221`	`<!-- endSnippet -->`
`222`	`222`
`223`	`223`	`Result:`
Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,12 @@ namespace VerifyMSTest;`
`3`	`3`	`public record TestExecutionContext(TestContext TestContext, Type TestClass)`
`4`	`4`	`{`
`5`	`5`	`public Assembly Assembly { get; } = TestClass.Assembly;`
`6`		`- public MethodInfo Method { get; } = FindMethod(TestClass, TestContext);`
	`6`	`+`
	`7`	`+ MethodInfo? method;`
	`8`	`+`
	`9`	`+ // Resolved lazily: the method scan is only needed when a Verify call actually`
	`10`	`+ // builds a verifier, not for every test that constructs a context.`
	`11`	`+ public MethodInfo Method => method ??= FindMethod(TestClass, TestContext);`
`7`	`12`
`8`	`13`	`static MethodInfo FindMethod(Type type, TestContext context)`
`9`	`14`	`{`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,12 @@ public static InnerVerifier BuildVerifier(VerifySettings settings, string source`
`33`	`33`	`if (!settings.HasParameters)`
`34`	`34`	`{`
`35`	`35`	`var data = context.TestContext.TestData;`
`36`		`- if (data != null)`
	`36`	`+ // Only apply when the data length matches the method parameter count.`
	`37`	`+ // A params-array DataRow exposes raw pre-binding data whose length does`
	`38`	`+ // not match the parameter count, which would break parameterized`
	`39`	`+ // snapshot file naming. XunitV3 applies the same guard.`
	`40`	`+ if (data != null &&`
	`41`	`+ data.Length == method.ParameterNames()?.Count)`
`37`	`42`	`{`
`38`	`43`	`settings.SetParameters(data);`
`39`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,6 @@ public partial class VerifyBase`
`7`	`7`	`public SettingsTask VerifyTuple(`
`8`	`8`	`Expression<Func<ITuple>> target,`
`9`	`9`	`VerifySettings? settings = null) =>`
`10`		`- Verifier.VerifyTuple(target, settings ?? this.settings);`
	`10`	`+ Verifier.VerifyTuple(target, settings ?? this.settings, sourceFile);`
`11`	`11`	`}`
`12`	`12`	`#endif`