Skip to content

Put an injected snapshot literal on its own line #757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SimonCropp merged 1 commit into from
Aug 14, 2026
Merged

Put an injected snapshot literal on its own line #757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 32 additions & 26 deletions src/DiffEngine.Tests/InlinePatcherTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ public async Task ReplaceRegularLiteral()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _);
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
"""
await Snapshot(""
""" + "\"");
await Assert.That(newSource).Contains(" new");
" await Snapshot(\n" +
" \"\"\"\n" +
" new\n" +
" \"\"\");");
}

[Test]
Expand Down Expand Up @@ -225,7 +225,7 @@ public async Task InsertIntoEmptyArgumentList()
var source = Method(" await Snapshot();");
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await Snapshot(\"\"\"\n new\n \"\"\");");
await Assert.That(newSource).Contains("await Snapshot(\n \"\"\"\n new\n \"\"\");");
}

[Test]
Expand All @@ -234,7 +234,7 @@ public async Task InsertReplacesNullArgument()
var source = Method(" await Snapshot(null, file, line);");
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await Snapshot(\"\"\"\n new\n \"\"\", file, line);");
await Assert.That(newSource).Contains("await Snapshot(\n \"\"\"\n new\n \"\"\", file, line);");
}

[Test]
Expand Down Expand Up @@ -290,7 +290,8 @@ public async Task AppendToABareVerify()
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
" await Verify(value)\n" +
" .Snapshot(\"\"\"\n" +
" .Snapshot(\n" +
" \"\"\"\n" +
" new\n" +
" \"\"\");");
}
Expand All @@ -309,7 +310,8 @@ public async Task AppendGoesAfterAnExistingChain()
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
" .ScrubLinesContaining(\"x\")\n" +
" .Snapshot(\"\"\"\n" +
" .Snapshot(\n" +
" \"\"\"\n" +
" new\n" +
" \"\"\");");
}
Expand All @@ -322,7 +324,7 @@ public async Task AppendToAnEntryPointOverload()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await VerifyXml(value)\n .Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await VerifyXml(value)\n .Snapshot(\n \"\"\"");
}

// The verify call spans lines, so the closing paren is nowhere near the hint
Expand All @@ -339,7 +341,7 @@ public async Task AppendToAMultiLineVerifyCall()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(" })\n .Snapshot(\"\"\"");
await Assert.That(newSource).Contains(" })\n .Snapshot(\n \"\"\"");
}

[Test]
Expand Down Expand Up @@ -409,7 +411,8 @@ public async Task AppendPrefersTheEntryPointOverANestedHelper()
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
" await Verify(ContentValidation.Verify(value))\n" +
" .Snapshot(\"\"\"");
" .Snapshot(\n" +
" \"\"\"");
}

[Test]
Expand All @@ -420,7 +423,7 @@ public async Task AppendToAVerifierQualifiedCall()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await Verifier.Verify(value)\n .Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await Verifier.Verify(value)\n .Snapshot(\n \"\"\"");
}

[Test]
Expand All @@ -431,7 +434,7 @@ public async Task AppendToAThisQualifiedCall()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await this.Verify(value)\n .Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await this.Verify(value)\n .Snapshot(\n \"\"\"");
}

[Test]
Expand Down Expand Up @@ -529,7 +532,7 @@ public async Task TabIndentedFileUsesTabUnit()
var source = "class Tests\n{\n\tasync Task Test()\n\t{\n\t\tawait Snapshot();\n\t}\n}";
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("Snapshot(\"\"\"\n\t\t\tnew\n\t\t\t\"\"\");");
await Assert.That(newSource).Contains("Snapshot(\n\t\t\t\"\"\"\n\t\t\tnew\n\t\t\t\"\"\");");
}

[Test]
Expand All @@ -538,7 +541,8 @@ public async Task HintBeyondEndOfFile()
var source = "await Snapshot();";
var status = InlinePatcher.TryApply(source, 500, InlinePatchMode.Set, null, "new", out var newSource, out _);
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("Snapshot(\"\"\"");
// No newline in the file, so the splice falls back to the environment's
await Assert.That(newSource).Contains($"Snapshot({Environment.NewLine} \"\"\"");
}

[Test]
Expand Down Expand Up @@ -721,7 +725,7 @@ public async Task SameLiteralInTheVerifyArgumentIsNotPatched()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"same\"", "changed", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await Verify(\"same\").Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await Verify(\"same\").Snapshot(\n \"\"\"");
await Assert.That(newSource).Contains("changed");
}

Expand Down Expand Up @@ -764,7 +768,7 @@ public async Task CommentedOutCallIsSkipped()

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(" // await Verify(x).Snapshot(\"doc example\");\n");
await Assert.That(newSource).Contains(" await Verify(x).Snapshot(\"\"\"");
await Assert.That(newSource).Contains(" await Verify(x).Snapshot(\n \"\"\"");
}

[Test]
Expand All @@ -778,7 +782,7 @@ public async Task CallInsideAStringIsSkipped()

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("var text = \"await Snapshot(\\\"x\\\")\";\n");
await Assert.That(newSource).Contains("await Verify(x).Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await Verify(x).Snapshot(\n \"\"\"");
}

// A declaration is a name followed by parens too, so it has to be told apart by what precedes it
Expand Down Expand Up @@ -814,7 +818,8 @@ public async Task AppendSkipsAVerifyPrefixedDeclaration()
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
" Task VerifyThing(string value) => Verify(value)\n" +
" .Snapshot(\"\"\"");
" .Snapshot(\n" +
" \"\"\"");
}

// Snapshot terminates the chain, so a comment in the middle of one must not end the walk
Expand All @@ -830,7 +835,8 @@ public async Task AppendGoesAfterACommentInTheChain()
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(
" .UseDirectory(\"snapshots\")\n" +
" .Snapshot(\"\"\"");
" .Snapshot(\n" +
" \"\"\"");
}

[Test]
Expand All @@ -844,7 +850,7 @@ public async Task LiteralInACommentIsNotPatched()

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("// was \"old\"\n");
await Assert.That(newSource).Contains("Snapshot(\"\"\"");
await Assert.That(newSource).Contains("Snapshot(\n \"\"\"");
}

[Test]
Expand All @@ -864,7 +870,7 @@ public async Task LiteralInAnotherMethodIsNotPatched()

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("void Helper() => Log(\"old\");");
await Assert.That(newSource).Contains("Snapshot(\"\"\"");
await Assert.That(newSource).Contains("Snapshot(\n \"\"\"");
}

// The empty literal is two characters, and the opening of every raw literal holds a pair
Expand All @@ -888,7 +894,7 @@ public async Task EmptyOriginalIsNotMatchedInsideARawDelimiter()

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(" content\n \"\"\");");
await Assert.That(newSource).Contains("Verify(b).Snapshot(\"\"\"\n new\n \"\"\");");
await Assert.That(newSource).Contains("Verify(b).Snapshot(\n \"\"\"\n new\n \"\"\");");
}

[Test]
Expand All @@ -899,7 +905,7 @@ public async Task GenericSnapshotCall()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains(".Snapshot<Thing>(\"\"\"");
await Assert.That(newSource).Contains(".Snapshot<Thing>(\n \"\"\"");
}

[Test]
Expand All @@ -910,7 +916,7 @@ public async Task AppendToAGenericVerify()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("await Verify<Thing>(value)\n .Snapshot(\"\"\"");
await Assert.That(newSource).Contains("await Verify<Thing>(value)\n .Snapshot(\n \"\"\"");
}

[Test]
Expand All @@ -921,7 +927,7 @@ public async Task CommentInTheArgumentListIsNotTheArgument()
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);

await Assert.That(status).IsEqualTo(PatchStatus.Applied);
await Assert.That(newSource).Contains("Snapshot(/* keep */\"\"\"");
await Assert.That(newSource).Contains("Snapshot(/* keep */\n \"\"\"");
}

[Test]
Expand Down
56 changes: 48 additions & 8 deletions src/DiffEngine/Inline/InlinePatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ public static PatchStatus TryApply(
return PatchStatus.AlreadyApplied;
}

var indent = IndentForSpan(source, lineStarts, expected.Start);
var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol);
var rendered = RenderArgument(source, lineStarts, expected.Start, newContent, eol);
newSource = Splice(source, expected.Start, expected.End, rendered);
return PatchStatus.Applied;
}
Expand Down Expand Up @@ -128,8 +127,7 @@ static PatchStatus InsertOrCheck(
return PatchStatus.NotFound;
}

var emptyIndent = IndentForSpan(source, lineStarts, expected.Start);
var emptyRendered = CsStringLiteral.RenderRaw(newContent, emptyIndent, eol);
var emptyRendered = RenderArgument(source, lineStarts, expected.Start, newContent, eol);
newSource = Splice(source, expected.Start, expected.Start, emptyRendered);
return PatchStatus.Applied;
}
Expand Down Expand Up @@ -159,8 +157,7 @@ static PatchStatus InsertOrCheck(
return PatchStatus.NotFound;
}

var indent = IndentForSpan(source, lineStarts, expected.Start);
var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol);
var rendered = RenderArgument(source, lineStarts, expected.Start, newContent, eol);
newSource = Splice(source, expected.Start, expected.End, rendered);
return PatchStatus.Applied;
}
Expand Down Expand Up @@ -286,8 +283,10 @@ static PatchStatus TryAppend(
var callIndent = LineOf(lineStarts, insertAt - 1) == LineOf(lineStarts, nameStart)
? statementIndent + unit
: LeadingWhitespace(source, lineStarts, insertAt - 1);
var rendered = CsStringLiteral.RenderRaw(newContent, callIndent + unit, eol);
newSource = Splice(source, insertAt, insertAt, $"{eol}{callIndent}.{methodName}({rendered})");
var contentIndent = callIndent + unit;
var rendered = CsStringLiteral.RenderRaw(newContent, contentIndent, eol);
var argument = OnOwnLine(rendered, contentIndent, eol);
newSource = Splice(source, insertAt, insertAt, $"{eol}{callIndent}.{methodName}({argument})");
return PatchStatus.Applied;
}

Expand Down Expand Up @@ -728,6 +727,47 @@ static void TrimSpan(string source, CsScan scan, ref int start, ref int end)
}
}

/// <summary>
/// Renders the literal for a splice at <paramref name="spanStart"/>, indented to suit where it
/// lands.
/// </summary>
static string RenderArgument(string source, List<int> lineStarts, int spanStart, string newContent, string eol)
{
var indent = IndentForSpan(source, lineStarts, spanStart);
var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol);
if (StartsLine(source, lineStarts, spanStart))
{
return rendered;
}

return OnOwnLine(rendered, indent, eol);
}

/// <summary>
/// Puts a multi-line literal on its own line, so the opening delimiter sits with the content
/// and the closing one rather than trailing the open paren. A single line literal (only the
/// empty snapshot renders as one) stays where it is, since there is nothing to line up with.
/// </summary>
static string OnOwnLine(string rendered, string indent, string eol) =>
rendered.IndexOf('\n') == -1 ? rendered : $"{eol}{indent}{rendered}";

/// <summary>
/// True when only whitespace precedes the offset on its line.
/// </summary>
static bool StartsLine(string source, List<int> lineStarts, int offset)
{
for (var index = lineStarts[LineOf(lineStarts, offset) - 1]; index < offset; index++)
{
if (source[index] != ' ' &&
source[index] != '\t')
{
return false;
}
}

return true;
}

static string Splice(string source, int start, int end, string replacement) =>
new StringBuilder(source.Length - (end - start) + replacement.Length)
.Append(source, 0, start)
Expand Down
Loading