diff --git a/src/DiffEngine.Tests/InlineApplierTests.cs b/src/DiffEngine.Tests/InlineApplierTests.cs index d3a6cd92..8464e855 100644 --- a/src/DiffEngine.Tests/InlineApplierTests.cs +++ b/src/DiffEngine.Tests/InlineApplierTests.cs @@ -23,7 +23,7 @@ static byte[] Utf8(string text, bool bom) return result; } - const string source = "class C\n{\n void M() => VerifyInline(value, \"old\");\n}"; + const string source = "class C\n{\n void M() => Verify(value).Snapshot(\"old\");\n}"; [Test] public async Task Utf8BomPreserved() @@ -193,7 +193,7 @@ public async Task AlreadyAppliedDoesNotWrite() [Test] public async Task ParallelAppliesToSameFile() { - var multi = "class C\n{\n void A() => VerifyInline(a, \"oldA\");\n void B() => VerifyInline(b, \"oldB\");\n}"; + var multi = "class C\n{\n void A() => Verify(a).Snapshot(\"oldA\");\n void B() => Verify(b).Snapshot(\"oldB\");\n}"; var path = WriteTemp(Utf8(multi, bom: false)); try { @@ -234,7 +234,7 @@ static int Count(string text, string value) [Test] public async Task ParallelAppliesWithIdenticalLiterals() { - var multi = "class C\n{\n void A() => VerifyInline(a, \"old\");\n void B() => VerifyInline(b, \"old\");\n}"; + var multi = "class C\n{\n void A() => Verify(a).Snapshot(\"old\");\n void B() => Verify(b).Snapshot(\"old\");\n}"; var path = WriteTemp(Utf8(multi, bom: false)); try { @@ -258,7 +258,7 @@ public async Task ParallelAppliesWithIdenticalLiterals() [Test] public async Task SequentialAppliesWithIdenticalLiterals() { - var multi = "class C\n{\n void A() => VerifyInline(a, \"old\");\n void B() => VerifyInline(b, \"old\");\n}"; + var multi = "class C\n{\n void A() => Verify(a).Snapshot(\"old\");\n void B() => Verify(b).Snapshot(\"old\");\n}"; var path = WriteTemp(Utf8(multi, bom: false)); try { @@ -270,8 +270,8 @@ public async Task SequentialAppliesWithIdenticalLiterals() var text = await File.ReadAllTextAsync(path); await Assert.That(text).DoesNotContain("old"); - var indexA = text.IndexOf("VerifyInline(a", StringComparison.Ordinal); - var indexB = text.IndexOf("VerifyInline(b", StringComparison.Ordinal); + var indexA = text.IndexOf("Verify(a)", StringComparison.Ordinal); + var indexB = text.IndexOf("Verify(b)", StringComparison.Ordinal); var segmentA = text.Substring(indexA, indexB - indexA); await Assert.That(segmentA).Contains("newA"); await Assert.That(segmentA).DoesNotContain("newB"); @@ -341,6 +341,44 @@ public async Task RoundTripNullExpression() } } + [Test] + [Arguments(InlinePatchMode.Set)] + [Arguments(InlinePatchMode.Append)] + [Arguments(InlinePatchMode.Remove)] + public async Task RoundTripMode(InlinePatchMode mode) + { + var patch = new InlinePatch("Tests.cs", 1, null, "content", mode); + + var read = InlinePatchFile.TryParse(InlinePatchFile.Build(patch), out var result); + + await Assert.That(read).IsTrue(); + await Assert.That(result!.Mode).IsEqualTo(mode); + } + + [Test] + public async Task DefaultModeIsSet() + { + var read = InlinePatchFile.TryParse(InlinePatchFile.Build(new("Tests.cs", 1, null, "content")), out var result); + + await Assert.That(read).IsTrue(); + await Assert.That(result!.Mode).IsEqualTo(InlinePatchMode.Set); + } + + // The version 1 shape, which had no mode line + [Test] + public async Task PreviousVersionFails() + { + var read = InlinePatchFile.TryParse("version: 1\nsourceFile: x\nlineHint: 1\noriginalExpression:\nnewContent: YQ==\n", out _); + await Assert.That(read).IsFalse(); + } + + [Test] + public async Task UnknownModeFails() + { + var read = InlinePatchFile.TryParse("version: 2\nsourceFile: x\nlineHint: 1\nmode: Sideways\noriginalExpression:\nnewContent: YQ==\n", out _); + await Assert.That(read).IsFalse(); + } + [Test] public async Task MissingFileFails() { @@ -358,7 +396,7 @@ public async Task GarbageFails() [Test] public async Task WrongVersionFails() { - var read = InlinePatchFile.TryParse("version: 2\nsourceFile: x\nlineHint: 1\noriginalExpression:\nnewContent: YQ==\n", out _); + var read = InlinePatchFile.TryParse("version: 3\nsourceFile: x\nlineHint: 1\nmode: Set\noriginalExpression:\nnewContent: YQ==\n", out _); await Assert.That(read).IsFalse(); } } diff --git a/src/DiffEngine.Tests/InlinePatcherTests.cs b/src/DiffEngine.Tests/InlinePatcherTests.cs index 0bd3c6c5..71fc6bdf 100644 --- a/src/DiffEngine.Tests/InlinePatcherTests.cs +++ b/src/DiffEngine.Tests/InlinePatcherTests.cs @@ -1,4 +1,4 @@ -public class InlinePatcherTests +public class InlinePatcherTests { const string rawOld = "\"\"\"\n old\n \"\"\""; @@ -8,26 +8,26 @@ static string Method(string body) => [Test] public async Task ReplaceRawLiteral() { - var source = Method($" await VerifyInline(value, {rawOld.Replace("\n", "\n ")});"); - var status = InlinePatcher.TryApply(source, 5, rawOld.Replace("\n", "\n "), "new", out var newSource, out _); + var source = Method($" await Snapshot({rawOld.Replace("\n", "\n ")});"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, rawOld.Replace("\n", "\n "), "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).Contains("new"); await Assert.That(newSource).DoesNotContain("old"); // Everything outside the span is untouched await Assert.That(newSource).Contains("class Tests"); - await Assert.That(newSource).Contains("await VerifyInline(value, "); + await Assert.That(newSource).Contains("await Snapshot("); await Assert.That(newSource.EndsWith(");\n }\n}")).IsTrue(); } [Test] public async Task ReplaceRegularLiteral() { - var source = Method(" await VerifyInline(value, \"old\");"); - var status = InlinePatcher.TryApply(source, 5, "\"old\"", "new", out var newSource, out _); + var source = Method(" await Snapshot(\"old\");"); + 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 VerifyInline(value, "" + await Snapshot("" """ + "\""); await Assert.That(newSource).Contains(" new"); } @@ -35,8 +35,8 @@ await VerifyInline(value, "" [Test] public async Task ReplacementUsesFileEol() { - var source = Method(" await VerifyInline(value, \"old\");").Replace("\n", "\r\n"); - var status = InlinePatcher.TryApply(source, 5, "\"old\"", "a\nb", out var newSource, out _); + var source = Method(" await Snapshot(\"old\");").Replace("\n", "\r\n"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "a\nb", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).DoesNotContain("a\nb"); await Assert.That(newSource).Contains("a\r\n b"); @@ -45,8 +45,8 @@ public async Task ReplacementUsesFileEol() [Test] public async Task AlreadyAppliedWhenLiteralMatches() { - var source = Method(" await VerifyInline(value, \"same\");"); - var status = InlinePatcher.TryApply(source, 5, "\"same\"", "same", out _, out _); + var source = Method(" await Snapshot(\"same\");"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"same\"", "same", out _, out _); await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied); } @@ -54,8 +54,8 @@ public async Task AlreadyAppliedWhenLiteralMatches() public async Task ShiftedLinesStillFound() { var padding = string.Concat(Enumerable.Repeat(" // padding\n", 30)); - var source = Method(padding + " await VerifyInline(value, \"old\");"); - var status = InlinePatcher.TryApply(source, 5, "\"old\"", "new", out var newSource, out _); + var source = Method(padding + " await Snapshot(\"old\");"); + 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).DoesNotContain("\"old\""); } @@ -64,14 +64,14 @@ public async Task ShiftedLinesStillFound() public async Task DuplicateLiteralsPicksNearestToHint() { var source = - "await VerifyInline(a, \"dup\");\n" + + "await A().Snapshot(\"dup\");\n" + string.Concat(Enumerable.Repeat("// filler\n", 10)) + - "await VerifyInline(b, \"dup\");\n"; - var status = InlinePatcher.TryApply(source, 12, "\"dup\"", "new", out var newSource, out _); + "await B().Snapshot(\"dup\");\n"; + var status = InlinePatcher.TryApply(source, 12, InlinePatchMode.Set, "\"dup\"", "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); // First occurrence untouched, second replaced - await Assert.That(newSource).Contains("VerifyInline(a, \"dup\")"); - await Assert.That(newSource).DoesNotContain("VerifyInline(b, \"dup\")"); + await Assert.That(newSource).Contains("A().Snapshot(\"dup\")"); + await Assert.That(newSource).DoesNotContain("B().Snapshot(\"dup\")"); } // Two call sites, A on line 4 and B on line 7 @@ -80,17 +80,17 @@ static string TwoCallSites(string literalA, string literalB) => "\n", "class Tests", "{", - " void A() =>", - $" VerifyInline(a, {literalA});", + " Task A() =>", + $" Verify(a).Snapshot({literalA});", "", - " void B() =>", - $" VerifyInline(b, {literalB});", + " Task B() =>", + $" Verify(b).Snapshot({literalB});", "}"); static (string a, string b) Segments(string text) { - var indexA = text.IndexOf("VerifyInline(a", StringComparison.Ordinal); - var indexB = text.IndexOf("VerifyInline(b", StringComparison.Ordinal); + var indexA = text.IndexOf("Verify(a)", StringComparison.Ordinal); + var indexB = text.IndexOf("Verify(b)", StringComparison.Ordinal); return (text.Substring(indexA, indexB - indexA), text.Substring(indexB)); } @@ -99,7 +99,7 @@ public async Task DuplicateLiteralsPicksNearestToHintFirst() { var source = TwoCallSites("\"dup\"", "\"dup\""); - var status = InlinePatcher.TryApply(source, 4, "\"dup\"", "new", out var newSource, out _); + var status = InlinePatcher.TryApply(source, 4, InlinePatchMode.Set, "\"dup\"", "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); var (a, b) = Segments(newSource); @@ -117,14 +117,14 @@ public async Task EquidistantDuplicatesPreferAtOrAfterHint() "\n", "class Tests", "{", - " void A() =>", - " VerifyInline(a, \"dup\");", - " void B() =>", - " VerifyInline(b, \"dup\");", + " Task A() =>", + " Verify(a).Snapshot(\"dup\");", + " Task B() =>", + " Verify(b).Snapshot(\"dup\");", "}"); // Line 5 is equidistant from the sites on lines 4 and 6 - var status = InlinePatcher.TryApply(source, 5, "\"dup\"", "new", out var newSource, out _); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"dup\"", "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); var (a, b) = Segments(newSource); @@ -139,8 +139,8 @@ public async Task SequentialPatchesOfIdenticalLiteralsSameContent() { var source = TwoCallSites("\"old\"", "\"old\""); - var first = InlinePatcher.TryApply(source, 4, "\"old\"", "new", out var afterFirst, out _); - var second = InlinePatcher.TryApply(afterFirst, 7, "\"old\"", "new", out var afterSecond, out var reason); + var first = InlinePatcher.TryApply(source, 4, InlinePatchMode.Set, "\"old\"", "new", out var afterFirst, out _); + var second = InlinePatcher.TryApply(afterFirst, 7, InlinePatchMode.Set, "\"old\"", "new", out var afterSecond, out var reason); await Assert.That(first).IsEqualTo(PatchStatus.Applied); await Assert.That(second).IsEqualTo(PatchStatus.Applied); @@ -156,8 +156,8 @@ public async Task SequentialPatchesOfIdenticalLiteralsDifferentContent() { var source = TwoCallSites("\"old\"", "\"old\""); - var first = InlinePatcher.TryApply(source, 4, "\"old\"", "newA", out var afterFirst, out _); - var second = InlinePatcher.TryApply(afterFirst, 7, "\"old\"", "newB", out var afterSecond, out _); + var first = InlinePatcher.TryApply(source, 4, InlinePatchMode.Set, "\"old\"", "newA", out var afterFirst, out _); + var second = InlinePatcher.TryApply(afterFirst, 7, InlinePatchMode.Set, "\"old\"", "newB", out var afterSecond, out _); await Assert.That(first).IsEqualTo(PatchStatus.Applied); await Assert.That(second).IsEqualTo(PatchStatus.Applied); @@ -176,9 +176,9 @@ public async Task SecondPatchSurvivesLineShiftFromTheFirst() { var source = TwoCallSites("\"old\"", "\"old\""); - InlinePatcher.TryApply(source, 4, "\"old\"", "line1\nline2\nline3", out var afterFirst, out _); + InlinePatcher.TryApply(source, 4, InlinePatchMode.Set, "\"old\"", "line1\nline2\nline3", out var afterFirst, out _); var lineShift = afterFirst.Split('\n').Length - source.Split('\n').Length; - var second = InlinePatcher.TryApply(afterFirst, 7, "\"old\"", "newB", out var afterSecond, out _); + var second = InlinePatcher.TryApply(afterFirst, 7, InlinePatchMode.Set, "\"old\"", "newB", out var afterSecond, out _); await Assert.That(lineShift).IsGreaterThan(0); await Assert.That(second).IsEqualTo(PatchStatus.Applied); @@ -195,7 +195,7 @@ public async Task ReapplyingWithIdenticalLiteralsIsAlreadyApplied() { var source = TwoCallSites("\"new\"", "\"old\""); - var status = InlinePatcher.TryApply(source, 4, "\"gone\"", "new", out _, out _); + var status = InlinePatcher.TryApply(source, 4, InlinePatchMode.Set, "\"gone\"", "new", out _, out _); await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied); } @@ -205,124 +205,289 @@ public async Task ExpressionGoneAndLiteralMatchesIsAlreadyApplied() { // The other TFM already applied: the old expression is gone, // and the current argument renders to the new content. - var source = Method(" await VerifyInline(value, \"new\");"); - var status = InlinePatcher.TryApply(source, 5, "\"old-gone\"", "new", out _, out _); + var source = Method(" await Snapshot(\"new\");"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old-gone\"", "new", out _, out _); await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied); } [Test] public async Task ExpressionGoneAndLiteralDiffersIsNotFound() { - var source = Method(" await VerifyInline(value, \"different\");"); - var status = InlinePatcher.TryApply(source, 5, "\"old-gone\"", "new", out _, out var reason); + var source = Method(" await Snapshot(\"different\");"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old-gone\"", "new", out _, out var reason); await Assert.That(status).IsEqualTo(PatchStatus.NotFound); await Assert.That(reason).Contains("Re-run the test"); } [Test] - public async Task InsertIntoSingleArgumentCall() + public async Task InsertIntoEmptyArgumentList() { - var source = Method(" await VerifyInline(value);"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + 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 VerifyInline(value, \"\"\"\n new\n \"\"\");"); + await Assert.That(newSource).Contains("await Snapshot(\"\"\"\n new\n \"\"\");"); } [Test] - public async Task InsertWithComplexTargetExpression() + public async Task InsertReplacesNullArgument() { - var source = Method(" await VerifyInline(new { a = 1, b = Call(\"x, y\") });"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + 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("Call(\"x, y\") }, \"\"\""); + await Assert.That(newSource).Contains("await Snapshot(\"\"\"\n new\n \"\"\", file, line);"); } [Test] - public async Task InsertReplacesNullArgument() + public async Task InsertBeforeAnotherNamedArgument() { - var source = Method(" await VerifyInline(value, null, settings);"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + var source = Method(" await Snapshot(file: myFile);"); + 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 VerifyInline(value, \"\"\"\n new\n \"\"\", settings);"); + await Assert.That(newSource).Contains("await Snapshot(expected: \"\"\"\n new\n \"\"\", file: myFile);"); + } + + [Test] + public async Task NullOriginalWithDifferingLiteralIsNotFound() + { + var source = Method(" await Snapshot(\"different\");"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out _, out var reason); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("different expected argument"); + } + + [Test] + public async Task NullOriginalWithEqualLiteralIsAlreadyApplied() + { + var source = Method(" await Snapshot(\"new\");"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out _, out _); + await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied); + } + + [Test] + public async Task NoCallFound() + { + var source = Method(" await Verify(value);"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out _, out var reason); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("Could not find a Snapshot call"); + } + + [Test] + public async Task PartialTokenIsNotMatched() + { + var source = Method(" await MySnapshotHelper(value);"); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out _, out _); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); } [Test] - public async Task InsertBeforeNamedSettingsArgument() + public async Task AppendToABareVerify() { - var source = Method(" await VerifyInline(value, settings: mySettings);"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + var source = Method(" await Verify(value);"); + + 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 VerifyInline(value, expected: \"\"\"\n new\n \"\"\", settings: mySettings);"); + await Assert.That(newSource).Contains( + " await Verify(value)\n" + + " .Snapshot(\"\"\"\n" + + " new\n" + + " \"\"\");"); } + // Snapshot terminates the chain, so it has to land after everything already chained on [Test] - public async Task InsertLeavesFluentContinuationIntact() + public async Task AppendGoesAfterAnExistingChain() { - var source = Method(" await VerifyInline(value)\n .UseDirectory(\"snapshots\");"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + var source = Method( + " await Verify(value)\n" + + " .UseDirectory(\"snapshots\")\n" + + " .ScrubLinesContaining(\"x\");"); + + 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(".UseDirectory(\"snapshots\");"); - await Assert.That(newSource).Contains("VerifyInline(value, \"\"\""); + await Assert.That(newSource).Contains( + " .ScrubLinesContaining(\"x\")\n" + + " .Snapshot(\"\"\"\n" + + " new\n" + + " \"\"\");"); } [Test] - public async Task NullOriginalWithDifferingLiteralIsNotFound() + public async Task AppendToAnEntryPointOverload() { - var source = Method(" await VerifyInline(value, \"different\");"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out _, out var reason); + var source = Method(" await VerifyXml(value);"); + + 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(\"\"\""); + } + + // The verify call spans lines, so the closing paren is nowhere near the hint + [Test] + public async Task AppendToAMultiLineVerifyCall() + { + var source = Method( + " await Verify(\n" + + " new\n" + + " {\n" + + " value\n" + + " });"); + + 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(\"\"\""); + } + + [Test] + public async Task AppendUsesTheFileEol() + { + var source = Method(" await Verify(value);").Replace("\n", "\r\n"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "a\nb", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await AssertEolConsistent(newSource, crlf); + } + + [Test] + public async Task AppendIsRefusedWhenOneIsAlreadyChained() + { + var source = Method(" await Verify(value)\n .Snapshot(\"already\");"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out _, out var reason); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); - await Assert.That(reason).Contains("different expected argument"); + await Assert.That(reason).Contains("already has a Snapshot call"); } [Test] - public async Task NullOriginalWithEqualLiteralIsAlreadyApplied() + public async Task AppendWithNoVerifyCall() { - var source = Method(" await VerifyInline(value, \"new\");"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out _, out _); - await Assert.That(status).IsEqualTo(PatchStatus.AlreadyApplied); + var source = Method(" await Something(value);"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out _, out var reason); + + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("Could not find a Verify call"); } [Test] - public async Task NoCallFound() + public async Task RemoveTakesTheWholeLine() { - var source = Method(" await Verify(value);"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out _, out var reason); + var source = Method( + " await Verify(value)\n" + + " .Snapshot(\"\"\"\n" + + " old\n" + + " \"\"\");"); + + var status = InlinePatcher.TryApply(source, 6, InlinePatchMode.Remove, null, "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).IsEqualTo(Method(" await Verify(value);")); + } + + [Test] + public async Task RemoveLeavesTheRestOfTheChain() + { + var source = Method( + " await Verify(value)\n" + + " .UseDirectory(\"snapshots\")\n" + + " .Snapshot(\"old\");"); + + var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Remove, null, "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).IsEqualTo( + Method( + " await Verify(value)\n" + + " .UseDirectory(\"snapshots\");")); + } + + [Test] + public async Task RemoveFromASingleLineChain() + { + var source = Method(" await Verify(value).Snapshot(\"old\");"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Remove, null, "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).IsEqualTo(Method(" await Verify(value);")); + } + + [Test] + public async Task RemoveWithCrlf() + { + var source = Method(" await Verify(value)\n .Snapshot(\"old\");").Replace("\n", "\r\n"); + + var status = InlinePatcher.TryApply(source, 6, InlinePatchMode.Remove, null, "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).IsEqualTo(Method(" await Verify(value);").Replace("\n", "\r\n")); + } + + [Test] + public async Task RemovePicksTheSiteNearestTheHint() + { + var source = TwoCallSites("\"a\"", "\"b\""); + + var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Remove, null, "", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + var (a, b) = Segments(newSource); + await Assert.That(a).Contains(".Snapshot(\"a\")"); + await Assert.That(b).DoesNotContain("Snapshot"); + } + + [Test] + public async Task RemoveWhenTheCallIsNotChained() + { + var source = Method(" await Snapshot(\"old\");"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Remove, null, "", out _, out var reason); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); - await Assert.That(reason).Contains("Could not find a VerifyInline call"); + await Assert.That(reason).Contains("not a chained call"); } [Test] - public async Task PartialTokenIsNotMatched() + public async Task RemoveWithNoSnapshotCall() { - var source = Method(" await MyVerifyInlineHelper(value);"); - var status = InlinePatcher.TryApply(source, 5, null, "new", out _, out _); + var source = Method(" await Verify(value);"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Remove, null, "", out _, out var reason); + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("Could not find a Snapshot call"); } [Test] public async Task TabIndentedFileUsesTabUnit() { - var source = "class Tests\n{\n\tasync Task Test()\n\t{\n\t\tawait VerifyInline(value);\n\t}\n}"; - var status = InlinePatcher.TryApply(source, 5, null, "new", out var newSource, out _); + 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("VerifyInline(value, \"\"\"\n\t\t\tnew\n\t\t\t\"\"\");"); + await Assert.That(newSource).Contains("Snapshot(\"\"\"\n\t\t\tnew\n\t\t\t\"\"\");"); } [Test] public async Task HintBeyondEndOfFile() { - var source = "await VerifyInline(value);"; - var status = InlinePatcher.TryApply(source, 500, null, "new", out var newSource, out _); + 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("VerifyInline(value, \"\"\""); + await Assert.That(newSource).Contains("Snapshot(\"\"\""); } [Test] public async Task LfExpressionFoundInCrlfFile() { - var source = Method($" await VerifyInline(value, {rawOld.Replace("\n", "\n ")});").Replace("\n", "\r\n"); + var source = Method($" await Snapshot({rawOld.Replace("\n", "\n ")});").Replace("\n", "\r\n"); var expression = rawOld.Replace("\n", "\n "); - var status = InlinePatcher.TryApply(source, 5, expression, "new", out var newSource, out _); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, expression, "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).DoesNotContain("old"); } @@ -330,9 +495,9 @@ public async Task LfExpressionFoundInCrlfFile() [Test] public async Task OutsideSpanIsCharacterIdentical() { - var body = " await VerifyInline(value, \"old\");"; + var body = " await Snapshot(\"old\");"; var source = Method(body); - InlinePatcher.TryApply(source, 5, "\"old\"", "new", out var newSource, out _); + InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _); var prefix = source.Substring(0, source.IndexOf("\"old\"", StringComparison.Ordinal)); var suffix = source.Substring(source.IndexOf("\"old\"", StringComparison.Ordinal) + 5); await Assert.That(newSource.StartsWith(prefix)).IsTrue(); @@ -349,7 +514,7 @@ static string BuildMultiLineSource(string eol) => "{", " async Task Test()", " {", - " await VerifyInline(value, \"\"\"", + " await Snapshot(\"\"\"", " old1", " old2", " \"\"\");", @@ -400,7 +565,7 @@ public async Task EolCombinations(string fileEol, string expressionEol, string c var expression = BuildExpression(expressionEol); var content = "new1" + contentEol + "new2"; - var status = InlinePatcher.TryApply(source, 5, expression, content, out var newSource, out var reason); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, expression, content, out var newSource, out var reason); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(reason).IsEmpty(); @@ -424,12 +589,12 @@ public async Task EolCombinationsForInsert(string fileEol, string contentEol) "{", " async Task Test()", " {", - " await VerifyInline(value);", + " await Snapshot();", " }", "}"); var content = "new1" + contentEol + "new2"; - var status = InlinePatcher.TryApply(source, 5, null, content, out var newSource, out _); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, content, out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).Contains("new1"); @@ -443,7 +608,7 @@ public async Task LoneCarriageReturnInContentIsNormalized() var source = BuildMultiLineSource(lf); var expression = BuildExpression(lf); - var status = InlinePatcher.TryApply(source, 5, expression, "new1\rnew2", out var newSource, out _); + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, expression, "new1\rnew2", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).DoesNotContain("\r"); @@ -462,13 +627,13 @@ public async Task MixedEolFileLeavesUntouchedRegionsAlone() "{", " async Task Test()", " {", - " await VerifyInline(value, \"old\");", + " await Snapshot(\"old\");", " }", "}"); var suffix = "\r\n// trailing\n// mixed tail\n"; var source = prefix + body + suffix; - var status = InlinePatcher.TryApply(source, 7, "\"old\"", "new", out var newSource, out _); + var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); // Untouched regions keep their original endings byte for byte @@ -481,7 +646,7 @@ public async Task MixedEolFileLeavesUntouchedRegionsAlone() [Test] public async Task SingleLineFileWithNoNewlines() { - var status = InlinePatcher.TryApply("await VerifyInline(value, \"old\");", 1, "\"old\"", "new", out var newSource, out _); + var status = InlinePatcher.TryApply("await Snapshot(\"old\");", 1, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _); await Assert.That(status).IsEqualTo(PatchStatus.Applied); await Assert.That(newSource).Contains("new"); diff --git a/src/DiffEngine/DiffRunner_Inline.cs b/src/DiffEngine/DiffRunner_Inline.cs index a4841076..865b94ef 100644 --- a/src/DiffEngine/DiffRunner_Inline.cs +++ b/src/DiffEngine/DiffRunner_Inline.cs @@ -39,8 +39,17 @@ public static partial class DiffRunner /// thread per call. /// /// + /// + /// , which has nothing for a user to review. Apply it with + /// instead. + /// public static async Task AddInlineAsync(InlinePatch patch, Cancel cancel = default) { + if (patch.Mode == InlinePatchMode.Remove) + { + throw new ArgumentException($"{InlinePatchMode.Remove} patches are not reviewable. Use InlineApplier.", nameof(patch)); + } + var check = CheckInline(); if (check != InlineResult.Queued) { diff --git a/src/DiffEngine/Inline/InlineApplier.cs b/src/DiffEngine/Inline/InlineApplier.cs index a2de8c79..b09d2874 100644 --- a/src/DiffEngine/Inline/InlineApplier.cs +++ b/src/DiffEngine/Inline/InlineApplier.cs @@ -1,4 +1,4 @@ -using System.Security.Cryptography; +using System.Security.Cryptography; namespace DiffEngine; @@ -21,7 +21,8 @@ public static InlineApplyResult Apply(InlinePatch patch) return InlineApplyResult.Failed("InlinePatch.SourceFile is empty"); } - if (patch.NewContent is null) + if (patch.NewContent is null && + patch.Mode != InlinePatchMode.Remove) { return InlineApplyResult.Failed("InlinePatch.NewContent is null"); } @@ -46,7 +47,7 @@ public static InlineApplyResult Apply(InlinePatch patch) return InlineApplyResult.Failed($"Source file does not exist: {fullPath}"); } - var newContent = CsStringLiteral.NormalizeNewlines(patch.NewContent); + var newContent = patch.NewContent is null ? "" : CsStringLiteral.NormalizeNewlines(patch.NewContent); var normalizedPath = fullPath.ToLowerInvariant(); lock (gates.GetOrAdd(normalizedPath, static _ => new())) { @@ -106,6 +107,7 @@ static InlineApplyResult LockedApply(string fullPath, InlinePatch patch, string var status = InlinePatcher.TryApply( source, patch.LineHint, + patch.Mode, patch.OriginalExpression, newContent, out var newSource, diff --git a/src/DiffEngine/Inline/InlinePatch.cs b/src/DiffEngine/Inline/InlinePatch.cs index 6f6a50bb..1e11e653 100644 --- a/src/DiffEngine/Inline/InlinePatch.cs +++ b/src/DiffEngine/Inline/InlinePatch.cs @@ -1,4 +1,4 @@ -namespace DiffEngine; +namespace DiffEngine; /// /// Describes a pending inline-snapshot edit to a C# source file. @@ -10,12 +10,18 @@ public InlinePatch() { } - public InlinePatch(string sourceFile, int lineHint, string? originalExpression, string newContent) + public InlinePatch( + string sourceFile, + int lineHint, + string? originalExpression, + string newContent, + InlinePatchMode mode = InlinePatchMode.Set) { SourceFile = sourceFile; LineHint = lineHint; OriginalExpression = originalExpression; NewContent = newContent; + Mode = mode; } /// @@ -24,7 +30,7 @@ public InlinePatch(string sourceFile, int lineHint, string? originalExpression, public string SourceFile { get; set; } = null!; /// - /// 1 based line of the VerifyInline call. A hint only; content search is the locator. + /// 1 based line of the verify or Snapshot call. A hint only; content search is the locator. /// public int LineHint { get; set; } @@ -35,7 +41,9 @@ public InlinePatch(string sourceFile, int lineHint, string? originalExpression, public string? OriginalExpression { get; set; } /// - /// The new snapshot text. Newlines are \n. + /// The new snapshot text. Newlines are \n. Ignored for . /// public string NewContent { get; set; } = null!; + + public InlinePatchMode Mode { get; set; } } diff --git a/src/DiffEngine/Inline/InlinePatchFile.cs b/src/DiffEngine/Inline/InlinePatchFile.cs index 815a05ff..c373f7df 100644 --- a/src/DiffEngine/Inline/InlinePatchFile.cs +++ b/src/DiffEngine/Inline/InlinePatchFile.cs @@ -1,4 +1,4 @@ -namespace DiffEngine; +namespace DiffEngine; /// /// Reads and writes the staged inline patch file. Plain text with base64 encoded @@ -22,8 +22,8 @@ public static string Build(InlinePatch patch) var expression = patch.OriginalExpression is null ? "" : Convert.ToBase64String(Encoding.UTF8.GetBytes(patch.OriginalExpression)); - var content = Convert.ToBase64String(Encoding.UTF8.GetBytes(patch.NewContent)); - return $"version: 1\nsourceFile: {patch.SourceFile}\nlineHint: {patch.LineHint}\noriginalExpression: {expression}\nnewContent: {content}\n"; + var content = Convert.ToBase64String(Encoding.UTF8.GetBytes(patch.NewContent ?? "")); + return $"version: 2\nsourceFile: {patch.SourceFile}\nlineHint: {patch.LineHint}\nmode: {patch.Mode}\noriginalExpression: {expression}\nnewContent: {content}\n"; } public static bool TryRead(string path, [NotNullWhen(true)] out InlinePatch? patch) @@ -53,15 +53,17 @@ public static bool TryParse(string text, [NotNullWhen(true)] out InlinePatch? pa var lines = text .Replace("\r\n", "\n") .Split('\n'); - if (lines.Length < 5 || + if (lines.Length < 6 || !TryValue(lines[0], "version", out var version) || - version != "1" || + version != "2" || !TryValue(lines[1], "sourceFile", out var sourceFile) || sourceFile.Length == 0 || !TryValue(lines[2], "lineHint", out var lineText) || !int.TryParse(lineText, out var lineHint) || - !TryValue(lines[3], "originalExpression", out var expressionBase64) || - !TryValue(lines[4], "newContent", out var contentBase64)) + !TryValue(lines[3], "mode", out var modeText) || + !Enum.TryParse(modeText, out var mode) || + !TryValue(lines[4], "originalExpression", out var expressionBase64) || + !TryValue(lines[5], "newContent", out var contentBase64)) { return false; } @@ -80,7 +82,7 @@ public static bool TryParse(string text, [NotNullWhen(true)] out InlinePatch? pa return false; } - patch = new(sourceFile, lineHint, expression, content); + patch = new(sourceFile, lineHint, expression, content, mode); return true; } diff --git a/src/DiffEngine/Inline/InlinePatchMode.cs b/src/DiffEngine/Inline/InlinePatchMode.cs new file mode 100644 index 00000000..9831ea07 --- /dev/null +++ b/src/DiffEngine/Inline/InlinePatchMode.cs @@ -0,0 +1,25 @@ +namespace DiffEngine; + +/// +/// What an does to the source. +/// +public enum InlinePatchMode +{ + /// + /// Set the expected argument of an existing Snapshot call: replace + /// when it is set, otherwise insert an argument. + /// + Set, + + /// + /// Append a Snapshot call after the verify invocation. Used for a snapshot that has never been + /// accepted, where there is no Snapshot call to set an argument on yet. + /// + Append, + + /// + /// Remove the Snapshot call. Used when inline is switched off and the snapshot migrates back + /// to a file. + /// + Remove +} diff --git a/src/DiffEngine/Inline/InlinePatcher.cs b/src/DiffEngine/Inline/InlinePatcher.cs index 3a34621a..302aedef 100644 --- a/src/DiffEngine/Inline/InlinePatcher.cs +++ b/src/DiffEngine/Inline/InlinePatcher.cs @@ -1,4 +1,4 @@ -enum PatchStatus +enum PatchStatus { Applied, AlreadyApplied, @@ -11,11 +11,21 @@ /// static class InlinePatcher { - const string methodName = "VerifyInline"; + /// + /// The fluent call that carries the snapshot literal. + /// + const string methodName = "Snapshot"; + + /// + /// Append mode has no Snapshot call to find, so it locates the verify invocation instead. + /// Matched by prefix because every entry point is Verify, VerifyXml, VerifyJson and so on. + /// + const string verifyPrefix = "Verify"; public static PatchStatus TryApply( string source, int lineHint, + InlinePatchMode mode, string? originalExpression, string newContent, out string newSource, @@ -26,6 +36,16 @@ public static PatchStatus TryApply( var eol = DetectEol(source); var lineStarts = BuildLineStarts(source); + if (mode == InlinePatchMode.Remove) + { + return TryRemove(source, lineStarts, lineHint, ref newSource, ref failReason); + } + + if (mode == InlinePatchMode.Append) + { + return TryAppend(source, lineStarts, lineHint, newContent, eol, ref newSource, ref failReason); + } + if (!string.IsNullOrEmpty(originalExpression)) { // Search for the previous expression verbatim, with newlines matched to the file's EOL @@ -75,43 +95,37 @@ static PatchStatus InsertOrCheck( return PatchStatus.NotFound; } - if (topCommas.Count == 0) - { - // Only the target argument exists - if (source.Substring(openParen + 1, closeParen - openParen - 1).Trim().Length == 0) - { - failReason = $"The {methodName} call near line {lineHint} has no arguments."; - return PatchStatus.NotFound; - } + // expected is the first parameter of Snapshot, so the argument to set is the first one + var argStart = openParen + 1; + var argEnd = topCommas.Count > 0 ? topCommas[0] : closeParen; + TrimSpan(source, ref argStart, ref argEnd); + if (argStart == argEnd) + { + // The argument was left to its default if (alreadyOnly) { - failReason = $"The previous expected expression was not found near line {lineHint}. The source may have changed since the test run. Re-run the test."; + failReason = StaleReason(lineHint); return PatchStatus.NotFound; } - var insertAt = EndOfLastNonWhitespace(source, openParen + 1, closeParen); - var indent = IndentForSpan(source, lineStarts, insertAt) ; - var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol); - newSource = Splice(source, insertAt, insertAt, ", " + rendered); + var emptyIndent = IndentForSpan(source, lineStarts, argStart); + var emptyRendered = CsStringLiteral.RenderRaw(newContent, emptyIndent, eol); + newSource = Splice(source, argStart, argStart, emptyRendered); return PatchStatus.Applied; } - // A second argument exists - var argStart = topCommas[0] + 1; - var argEnd = topCommas.Count > 1 ? topCommas[1] : closeParen; - TrimSpan(source, ref argStart, ref argEnd); var argOriginalStart = argStart; var named = TryStripArgumentName(source, ref argStart, out var argumentName); var argText = source.Substring(argStart, argEnd - argStart); if (named && argumentName != "expected") { - // Second argument is some other named argument (eg settings:). + // Some other named argument came first (eg file:). // Insert a named expected argument before it. if (alreadyOnly) { - failReason = $"The previous expected expression was not found near line {lineHint}. The source may have changed since the test run. Re-run the test."; + failReason = StaleReason(lineHint); return PatchStatus.NotFound; } @@ -125,7 +139,7 @@ static PatchStatus InsertOrCheck( { if (alreadyOnly) { - failReason = $"The previous expected expression was not found near line {lineHint}. The source may have changed since the test run. Re-run the test."; + failReason = StaleReason(lineHint); return PatchStatus.NotFound; } @@ -152,8 +166,204 @@ static PatchStatus InsertOrCheck( return PatchStatus.NotFound; } - static bool TryFindCall(string source, List lineStarts, int lineHint, out int openParen) + static string StaleReason(int lineHint) => + $"The previous expected expression was not found near line {lineHint}. The source may have changed since the test run. Re-run the test."; + + /// + /// Appends a Snapshot call to the verify invocation, for a snapshot that has never been + /// accepted. Snapshot terminates the chain, so the insertion point is the end of any calls + /// already chained onto the invocation rather than the invocation's own closing paren. + /// + static PatchStatus TryAppend( + string source, + List lineStarts, + int lineHint, + string newContent, + string eol, + ref string newSource, + ref string failReason) + { + if (!TryFindCall(source, lineStarts, lineHint, verifyPrefix, true, out var nameStart, out var openParen)) + { + failReason = $"Could not find a {verifyPrefix} call near line {lineHint}. The source may have changed since the test run. Re-run the test."; + return PatchStatus.NotFound; + } + + if (!TryScanArguments(source, openParen, out var closeParen, out _)) + { + failReason = $"Could not parse the argument list of the {verifyPrefix} call near line {lineHint}."; + return PatchStatus.NotFound; + } + + var insertAt = WalkChain(source, closeParen + 1, methodName, out var alreadyChained); + // Another process may have appended one between the run and the accept + if (alreadyChained) + { + failReason = $"The call near line {lineHint} already has a {methodName} call. Re-run the test."; + return PatchStatus.NotFound; + } + + var statementIndent = LeadingWhitespace(source, lineStarts, nameStart); + var unit = statementIndent.Contains('\t') ? "\t" : " "; + // Line up with the existing chain when there is one, otherwise start it one level in + 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})"); + return PatchStatus.Applied; + } + + /// + /// Removes the Snapshot call, along with the whitespace and line break that preceded it so no + /// blank line is left behind. + /// + static PatchStatus TryRemove( + string source, + List lineStarts, + int lineHint, + ref string newSource, + ref string failReason) { + if (!TryFindCall(source, lineStarts, lineHint, methodName, false, 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."; + return PatchStatus.NotFound; + } + + if (!TryScanArguments(source, openParen, out var closeParen, out _)) + { + failReason = $"Could not parse the argument list of the {methodName} call near line {lineHint}."; + return PatchStatus.NotFound; + } + + var start = nameStart; + // Back over the dot that made it a chained call + while (start > 0 && + char.IsWhiteSpace(source[start - 1])) + { + start--; + } + + if (start == 0 || + source[start - 1] != '.') + { + failReason = $"The {methodName} call near line {lineHint} is not a chained call."; + return PatchStatus.NotFound; + } + + start--; + // Then back over the indentation and line break it sat on + while (start > 0 && + (source[start - 1] == ' ' || source[start - 1] == '\t')) + { + start--; + } + + if (start > 0 && + source[start - 1] == '\n') + { + start--; + if (start > 0 && + source[start - 1] == '\r') + { + start--; + } + } + + newSource = Splice(source, start, closeParen + 1, ""); + return PatchStatus.Applied; + } + + /// + /// Walks the calls chained onto an invocation and returns the end of the chain. + /// is set when one of them is a call to . + /// + static int WalkChain(string source, int index, string name, out bool found) + { + found = false; + while (true) + { + var cursor = index; + if (!TrySkipTo(source, ref cursor, '.')) + { + return index; + } + + cursor++; + while (cursor < source.Length && + char.IsWhiteSpace(source[cursor])) + { + cursor++; + } + + var nameStart = cursor; + while (cursor < source.Length && + IsIdentifierChar(source[cursor])) + { + cursor++; + } + + if (cursor == nameStart || + !TrySkipToParen(source, cursor, out var paren) || + !TryScanArguments(source, paren, out var closeParen, out _)) + { + return index; + } + + if (string.CompareOrdinal(source, nameStart, name, 0, name.Length) == 0 && + cursor - nameStart == name.Length) + { + found = true; + } + + index = closeParen + 1; + } + } + + static bool TrySkipTo(string source, ref int index, char ch) + { + while (index < source.Length && + char.IsWhiteSpace(source[index])) + { + index++; + } + + return index < source.Length && + source[index] == ch; + } + + static string LeadingWhitespace(string source, List lineStarts, int offset) + { + var lineStart = lineStarts[LineOf(lineStarts, offset) - 1]; + var index = lineStart; + while (index < source.Length && + (source[index] == ' ' || source[index] == '\t')) + { + index++; + } + + return source.Substring(lineStart, index - lineStart); + } + + static bool TryFindCall(string source, List lineStarts, int lineHint, out int openParen) => + TryFindCall(source, lineStarts, lineHint, methodName, false, out _, out openParen); + + /// + /// Locates a call by name, searching outward from the hint. matches + /// any identifier starting with , which is how the several Verify + /// overloads are found with one search. + /// + static bool TryFindCall( + string source, + List lineStarts, + int lineHint, + string name, + bool byPrefix, + out int nameStart, + out int openParen) + { + nameStart = -1; openParen = -1; var lineCount = lineStarts.Count; lineHint = Math.Min(Math.Max(lineHint, 1), lineCount); @@ -175,20 +385,37 @@ static bool TryFindCall(string source, List lineStarts, int lineHint, out i var index = start; while (true) { - index = source.IndexOf(methodName, index, StringComparison.Ordinal); + index = source.IndexOf(name, index, StringComparison.Ordinal); if (index < 0 || index >= end) { break; } - if (IsToken(source, index, methodName.Length) && - TrySkipToParen(source, index + methodName.Length, out var paren)) + var identifierEnd = index + name.Length; + if (byPrefix) + { + while (identifierEnd < source.Length && + IsIdentifierChar(source[identifierEnd])) + { + identifierEnd++; + } + } + else if (identifierEnd < source.Length && + IsIdentifierChar(source[identifierEnd])) { + index += name.Length; + continue; + } + + if (StartsToken(source, index) && + TrySkipToParen(source, identifierEnd, out var paren)) + { + nameStart = index; openParen = paren; return true; } - index += methodName.Length; + index += name.Length; } } } @@ -196,16 +423,9 @@ static bool TryFindCall(string source, List lineStarts, int lineHint, out i return false; } - static bool IsToken(string source, int index, int length) - { - if (index > 0 && IsIdentifierChar(source[index - 1])) - { - return false; - } - - var after = index + length; - return after >= source.Length || !IsIdentifierChar(source[after]); - } + static bool StartsToken(string source, int index) => + index == 0 || + !IsIdentifierChar(source[index - 1]); static bool IsIdentifierChar(char ch) => char.IsLetterOrDigit(ch) || ch == '_'; @@ -618,18 +838,6 @@ static void TrimSpan(string source, ref int start, ref int end) } } - static int EndOfLastNonWhitespace(string source, int start, int end) - { - var index = end; - while (index > start && char.IsWhiteSpace(source[index - 1])) - { - index--; - } - - return index; - } - - static string Splice(string source, int start, int end, string replacement) => new StringBuilder(source.Length - (end - start) + replacement.Length) .Append(source, 0, start) diff --git a/src/DiffEngineViewer.Tests/EngineInlineTests.cs b/src/DiffEngineViewer.Tests/EngineInlineTests.cs index 9734f754..1b4bbe5f 100644 --- a/src/DiffEngineViewer.Tests/EngineInlineTests.cs +++ b/src/DiffEngineViewer.Tests/EngineInlineTests.cs @@ -91,6 +91,20 @@ public async Task DisabledDoesNotReachTheViewer() await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty(); } + /// + /// Removing a literal is a configuration change with nothing to review. Rejected at the entry + /// point rather than at the far end, so a caller cannot spend a viewer launch to find out. + /// + [Test] + public async Task ARemovePatchIsRefused() + { + using var scope = new EngineScope(); + var patch = new EnginePatch("Sample.cs", 42, "\"old\"", "", engine::DiffEngine.InlinePatchMode.Remove); + + await Assert.That(() => EngineRunner.AddInlineAsync(patch)).Throws(); + await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty(); + } + [Test] public async Task TheOptOutDoesNotReachTheViewer() { diff --git a/src/DiffEngineViewer.Tests/IpcTests.cs b/src/DiffEngineViewer.Tests/IpcTests.cs index 744c4d99..8fd4838f 100644 --- a/src/DiffEngineViewer.Tests/IpcTests.cs +++ b/src/DiffEngineViewer.Tests/IpcTests.cs @@ -253,6 +253,21 @@ public async Task GarbageIsRejectedWithoutKillingTheServer() await Assert.That(fixture.Send(new(ViewerVerb.List)).Ok).IsTrue(); } + /// + /// Removing a literal is a configuration change with nothing to review, so it must never reach + /// the queue and sit there waiting for an accept that means nothing. + /// + [Test] + public async Task ARemovePatchIsRejected() + { + using var fixture = new ServerFixture(); + + var response = fixture.Send(Inline(new("Sample.cs", 1, "\"old\"", "", InlinePatchMode.Remove))); + + await Assert.That(response.Ok).IsFalse(); + await Assert.That(fixture.Host.State.Queue).IsEmpty(); + } + static ViewerMessage Inline(InlinePatch patch) => new(ViewerVerb.Inline, Body: InlinePatchFile.Build(patch)); } diff --git a/src/DiffEngineViewer.Tests/ViewerProtocolTests.cs b/src/DiffEngineViewer.Tests/ViewerProtocolTests.cs index 14708942..c7f4d102 100644 --- a/src/DiffEngineViewer.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngineViewer.Tests/ViewerProtocolTests.cs @@ -1,5 +1,6 @@ extern alias engine; +using EngineMode = engine::DiffEngine.InlinePatchMode; using EnginePatch = engine::DiffEngine.InlinePatch; using EnginePatchFile = engine::DiffEngine.InlinePatchFile; using EnginePayload = engine::DiffEngine.ViewerPayload; @@ -45,6 +46,29 @@ public async Task AwkwardSnapshotTextSurvivesTheRoundTrip() await Assert.That(roundTripped.OriginalExpression).IsNull(); } + /// + /// The mode is written by name, so the two enums have to stay member for member identical. + /// Both sides are enumerated rather than listed, so a member added to one and not the other + /// fails here rather than at a call site nobody wrote yet. + /// + [Test] + public async Task ModesAgree() + { + var engineModes = Enum.GetNames(typeof(EngineMode)); + + await Assert.That(engineModes).IsEquivalentTo(Enum.GetNames()); + + foreach (var name in engineModes) + { + var engineMode = (EngineMode) Enum.Parse(typeof(EngineMode), name); + var payload = EnginePayload.Inline(EnginePatchFile.Build(new("Tests.cs", 1, null, "content", engineMode))); + + await Assert.That(ViewerMessage.TryParse(payload, out var message)).IsTrue(); + await Assert.That(InlinePatchFile.TryParse(message!.Body!, out var roundTripped)).IsTrue(); + await Assert.That(roundTripped!.Mode).IsEqualTo(Enum.Parse(name)); + } + } + [Test] public async Task EngineSettleMessageIsReadableByTheViewer() { diff --git a/src/DiffEngineViewer/Ipc/MessageHandler.cs b/src/DiffEngineViewer/Ipc/MessageHandler.cs index 57493c54..bb172a54 100644 --- a/src/DiffEngineViewer/Ipc/MessageHandler.cs +++ b/src/DiffEngineViewer/Ipc/MessageHandler.cs @@ -50,6 +50,13 @@ ViewerResponse Inline(string? body) return ViewerResponse.Error("Inline body is not a readable patch payload"); } + // Remove strips a literal when inline is switched off. That is a configuration change with + // nothing to review, so the sender applies it directly rather than queueing it here. + if (patch.Mode == InlinePatchMode.Remove) + { + return ViewerResponse.Error($"{InlinePatchMode.Remove} patches are not reviewable"); + } + var state = host.Mutate(_ => ViewerSession.Enqueue(_, QueueEntry.ForInline(patch))); return ViewerResponse.Success($"Queued {state.Queue.Count}"); } diff --git a/src/DiffEngineViewer/QueueEntry.cs b/src/DiffEngineViewer/QueueEntry.cs index 174caf76..240dc6a5 100644 --- a/src/DiffEngineViewer/QueueEntry.cs +++ b/src/DiffEngineViewer/QueueEntry.cs @@ -39,7 +39,7 @@ public static QueueEntry ForInline(InlinePatch patch) Name: $"{Path.GetFileName(patch.SourceFile)}:{patch.LineHint}", LeftHeader: "received", RightHeader: rightHeader, - LeftText: CsStringLiteral.NormalizeNewlines(patch.NewContent), + LeftText: CsStringLiteral.NormalizeNewlines(patch.NewContent ?? ""), RightText: rightText, Patch: patch, LeftFile: null, diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 2891995c..0f847663 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;CS0649;NU1608;NU1109 - 20.0.0-beta.4 + 20.0.0-beta.5 1.0.0 Testing, Snapshot, Diff, Compare Launches diff tools based on file extensions. Designed to be consumed by snapshot testing libraries.