diff --git a/src/DiffEngine.Tests/InlineApplierTests.cs b/src/DiffEngine.Tests/InlineApplierTests.cs index 64b5f9e0..2f07a21a 100644 --- a/src/DiffEngine.Tests/InlineApplierTests.cs +++ b/src/DiffEngine.Tests/InlineApplierTests.cs @@ -82,6 +82,52 @@ public async Task Utf16Preserved() } } + // The whole file is rewritten, not just the patched span, so a byte that does not decode + // would come back as a replacement character everywhere it appears + [Test] + public async Task NonUtf8FileIsRefused() + { + byte[] bytes = + [ + .. Utf8("class C\n{\n // caf", bom: false), + 0xE9, // é in Latin-1, not valid UTF-8 + .. Utf8("\n void M() => Verify(value).Snapshot(\"old\");\n}", bom: false) + ]; + var path = WriteTemp(bytes); + try + { + var result = InlineApplier.Apply(new(path, 4, "\"old\"", "new")); + + await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Failed); + await Assert.That(result.Message!).Contains("Convert it to UTF-8"); + await Assert.That((await File.ReadAllBytesAsync(path)).SequenceEqual(bytes)).IsTrue(); + } + finally + { + File.Delete(path); + } + } + + [Test] + public async Task NonAsciiUtf8IsPreserved() + { + var text = "class C\n{\n // café ☕\n void M() => Verify(value).Snapshot(\"old\");\n}"; + var path = WriteTemp(Utf8(text, bom: false)); + try + { + var result = InlineApplier.Apply(new(path, 4, "\"old\"", "naïve ☕")); + + await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); + var after = await File.ReadAllTextAsync(path); + await Assert.That(after).Contains("// café ☕"); + await Assert.That(after).Contains("naïve ☕"); + } + finally + { + File.Delete(path); + } + } + [Test] public async Task CrlfPreserved() { diff --git a/src/DiffEngine.Tests/InlinePatcherTests.cs b/src/DiffEngine.Tests/InlinePatcherTests.cs index 71fc6bdf..c57741e5 100644 --- a/src/DiffEngine.Tests/InlinePatcherTests.cs +++ b/src/DiffEngine.Tests/InlinePatcherTests.cs @@ -652,4 +652,246 @@ public async Task SingleLineFileWithNoNewlines() await Assert.That(newSource).Contains("new"); await Assert.That(newSource).DoesNotContain("\"old\""); } + + // The verify argument is the same text as the snapshot, and comes first on the line + [Test] + public async Task SameLiteralInTheVerifyArgumentIsNotPatched() + { + var source = Method(" await Verify(\"same\").Snapshot(\"same\");"); + + 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("changed"); + } + + // The expression search must match a whole argument, not the quoted part of a longer literal + [Test] + public async Task PrefixedLiteralIsNotPatchedThroughItsQuote() + { + var source = Method(" await Verify(x).Snapshot(@\"old\");"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "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 SuffixedLiteralIsNotPatchedThroughItsQuote() + { + var source = Method(" await Verify(x).Snapshot(\"old\"u8);"); + + var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "new", out _, out var reason); + + await Assert.That(status).IsEqualTo(PatchStatus.NotFound); + await Assert.That(reason).Contains("not a string literal"); + } + + [Test] + public async Task CommentedOutCallIsSkipped() + { + var source = string.Join( + "\n", + "class Tests", + "{", + " // await Verify(x).Snapshot(\"doc example\");", + " async Task Test() =>", + " await Verify(x).Snapshot();", + "}"); + + var status = InlinePatcher.TryApply(source, 3, InlinePatchMode.Set, null, "new", out var newSource, out _); + + 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(\"\"\""); + } + + [Test] + public async Task CallInsideAStringIsSkipped() + { + var source = Method( + " var text = \"await Snapshot(\\\"x\\\")\";\n" + + " await Verify(x).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("var text = \"await Snapshot(\\\"x\\\")\";\n"); + await Assert.That(newSource).Contains("await Verify(x).Snapshot(\"\"\""); + } + + // A declaration is a name followed by parens too, so it has to be told apart by what precedes it + [Test] + public async Task SnapshotDeclarationIsNotMistakenForACall() + { + var source = string.Join( + "\n", + "static class Extensions", + "{", + " public static Task Snapshot(this Task task, string? expected = null) =>", + " task;", + "}"); + + var status = InlinePatcher.TryApply(source, 3, 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 AppendSkipsAVerifyPrefixedDeclaration() + { + var source = string.Join( + "\n", + "class Tests", + "{", + " Task VerifyThing(string value) => Verify(value);", + "}"); + + var status = InlinePatcher.TryApply(source, 3, InlinePatchMode.Append, null, "new", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).Contains( + " Task VerifyThing(string value) => Verify(value)\n" + + " .Snapshot(\"\"\""); + } + + // Snapshot terminates the chain, so a comment in the middle of one must not end the walk + [Test] + public async Task AppendGoesAfterACommentInTheChain() + { + var source = Method( + " await Verify(value) // note\n" + + " .UseDirectory(\"snapshots\");"); + + 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\")\n" + + " .Snapshot(\"\"\""); + } + + [Test] + public async Task LiteralInACommentIsNotPatched() + { + var source = Method( + " // was \"old\"\n" + + " await Verify(x).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("// was \"old\"\n"); + await Assert.That(newSource).Contains("Snapshot(\"\"\""); + } + + [Test] + public async Task LiteralInAnotherMethodIsNotPatched() + { + var source = string.Join( + "\n", + "class Tests", + "{", + " void Helper() => Log(\"old\");", + "", + " async Task Test() =>", + " await Verify(x).Snapshot(\"old\");", + "}"); + + var status = InlinePatcher.TryApply(source, 3, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _); + + await Assert.That(status).IsEqualTo(PatchStatus.Applied); + await Assert.That(newSource).Contains("void Helper() => Log(\"old\");"); + await Assert.That(newSource).Contains("Snapshot(\"\"\""); + } + + // The empty literal is two characters, and the opening of every raw literal holds a pair + [Test] + public async Task EmptyOriginalIsNotMatchedInsideARawDelimiter() + { + var source = string.Join( + "\n", + "class Tests", + "{", + " Task A() =>", + " Verify(a).Snapshot(\"\"\"", + " content", + " \"\"\");", + "", + " Task B() =>", + " Verify(b).Snapshot(\"\");", + "}"); + + var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Set, "\"\"", "new", out var newSource, out _); + + 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 \"\"\");"); + } + + [Test] + public async Task GenericSnapshotCall() + { + var source = Method(" await Verify(x).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(".Snapshot(\"\"\""); + } + + [Test] + public async Task AppendToAGenericVerify() + { + 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 Verify(value)\n .Snapshot(\"\"\""); + } + + [Test] + public async Task CommentInTheArgumentListIsNotTheArgument() + { + var source = Method(" await Verify(x).Snapshot(/* keep */);"); + + 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 */\"\"\""); + } + + [Test] + public async Task CommentAfterTheArgumentIsKept() + { + var source = Method(" await Verify(x).Snapshot(\"old\" /* why */);"); + + 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("\"\"\" /* why */);"); + } + + // Pulling the call up onto the line above would take the semicolon into the comment + [Test] + public async Task RemoveLeavesALineCommentAboveIntact() + { + var source = Method( + " await Verify(value)\n" + + " // note\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" + + " // note\n" + + " ;")); + } } diff --git a/src/DiffEngine/Inline/CsScan.cs b/src/DiffEngine/Inline/CsScan.cs new file mode 100644 index 00000000..dfeeec9a --- /dev/null +++ b/src/DiffEngine/Inline/CsScan.cs @@ -0,0 +1,510 @@ +/// +/// A one pass lexical map of a C# file: where the comments, strings and char literals are, and +/// therefore which offsets are code. +/// +/// The patcher finds its call sites by scanning text, and a text scan that cannot see a comment +/// or a string patches a commented out example, or the middle of another test's snapshot content, +/// as readily as the real call. Lexing once and asking the map is cheaper than lexing per search, +/// and it is one implementation: every search agrees on what a string is because there is only +/// one answer to ask. +/// +/// +sealed class CsScan +{ + readonly string source; + readonly bool[] code; + + /// + /// Start of a comment or literal to the offset just past it. + /// + readonly Dictionary skips = new(); + + /// + /// The same spans keyed the other way round, for a scan working backwards. Ends are unique + /// because the spans cannot overlap. + /// + readonly Dictionary skipEnds = new(); + + public CsScan(string source) + { + this.source = source; + code = new bool[source.Length]; + var index = 0; + while (index < source.Length) + { + var start = index; + switch (source[index]) + { + case '/': + if (TrySkipComment(source, ref index)) + { + AddSkip(start, index); + continue; + } + + break; + case '\'': + if (TrySkipCharLiteral(source, ref index)) + { + AddSkip(start, index); + continue; + } + + // Unterminated: take the quote as code rather than swallowing the rest of + // the file over one stray character + index = start; + break; + case '"': + case '@': + case '$': + if (TrySkipStringLike(source, ref index)) + { + // A suffix (u8) is part of the literal token, so a search for "x" cannot + // match "x"u8 and splice over only the quoted part + while (index < source.Length && + IsIdentifierChar(source[index])) + { + index++; + } + + AddSkip(start, index); + continue; + } + + break; + } + + code[index] = true; + index++; + } + } + + void AddSkip(int start, int end) + { + skips.Add(start, end); + skipEnds[end] = start; + } + + /// + /// True when the offset is outside every comment, string and char literal. + /// + public bool IsCode(int index) => + index >= 0 && + index < code.Length && + code[index]; + + /// + /// When a comment or literal starts at , is the + /// offset just past it. Lets a structural scan step over trivia without lexing it again. + /// + public bool TryGetSkip(int index, out int end) => + skips.TryGetValue(index, out end); + + /// + /// True when a comment ends at , with set to + /// where it began. Only comments: a literal is content, and trimming one off a span would be + /// trimming off the value. + /// + public bool TryGetCommentEndingAt(int end, out int start) => + skipEnds.TryGetValue(end, out start) && + source[start] == '/'; + + /// + /// Advances past whitespace and comments. + /// + public void SkipTrivia(ref int index) + { + while (index < source.Length) + { + if (char.IsWhiteSpace(source[index])) + { + index++; + continue; + } + + if (source[index] == '/' && + skips.TryGetValue(index, out var end)) + { + index = end; + continue; + } + + return; + } + } + + /// + /// True when the identifier at is being declared rather than + /// called. The two are otherwise identical - name, parens, body - so the tell is the token in + /// front: a declaration is preceded by its return type, a call by a dot, an operator, or one + /// of the keywords that can introduce an expression. + /// + public bool IsDeclaration(int nameStart) + { + var index = PreviousSignificant(nameStart); + if (index < 0) + { + return false; + } + + var ch = source[index]; + if (ch == '>') + { + // Close of a generic return type (Task Name), unless it is a lambda arrow + return index == 0 || + source[index - 1] != '='; + } + + if (!IsIdentifierChar(ch)) + { + return false; + } + + var start = index; + while (start > 0 && + IsIdentifierChar(source[start - 1])) + { + start--; + } + + return !callablePredecessors.Contains(source.Substring(start, index - start + 1)); + } + + /// + /// The keywords that can sit immediately before a call. Anything else that reads as an + /// identifier there is a return type or a modifier, which makes what follows a declaration. + /// + static readonly HashSet callablePredecessors = new(StringComparer.Ordinal) + { + "and", "as", "await", "by", "case", "catch", "checked", "default", "do", "else", + "equals", "fixed", "foreach", "from", "goto", "group", "if", "in", "into", "is", + "join", "let", "lock", "new", "not", "on", "or", "orderby", "out", "params", "ref", + "return", "select", "stackalloc", "switch", "throw", "unchecked", "using", "when", + "where", "while", "with", "yield" + }; + + /// + /// The offset of the last character before that is neither + /// whitespace nor inside a comment, or -1 when there is none. + /// + int PreviousSignificant(int index) + { + index--; + while (index >= 0 && + (char.IsWhiteSpace(source[index]) || !code[index])) + { + index--; + } + + return index; + } + + /// + /// Advances past a type argument list, so Foo<Bar>(...) is located as readily as + /// Foo(...). Only the characters a type argument list can hold are accepted, and the caller + /// still has to find a '(' after it, so a comparison cannot be mistaken for one. + /// + public static bool TrySkipTypeArguments(string source, ref int index) + { + var cursor = index; + if (cursor >= source.Length || + source[cursor] != '<') + { + return false; + } + + cursor++; + var depth = 1; + while (cursor < source.Length) + { + var ch = source[cursor]; + if (ch == '<') + { + depth++; + cursor++; + continue; + } + + if (ch == '>') + { + depth--; + cursor++; + if (depth == 0) + { + index = cursor; + return true; + } + + continue; + } + + if (IsIdentifierChar(ch) || + ch is ',' or '.' or '?' or '[' or ']' or ':' or ' ' or '\t') + { + cursor++; + continue; + } + + return false; + } + + return false; + } + + public static bool IsIdentifierChar(char ch) => + char.IsLetterOrDigit(ch) || ch == '_'; + + static bool TrySkipComment(string source, ref int index) + { + if (index + 1 >= source.Length) + { + return false; + } + + var next = source[index + 1]; + if (next == '/') + { + var end = source.IndexOf('\n', index); + index = end < 0 ? source.Length : end + 1; + return true; + } + + if (next == '*') + { + var end = source.IndexOf("*/", index + 2, StringComparison.Ordinal); + index = end < 0 ? source.Length : end + 2; + return true; + } + + return false; + } + + static bool TrySkipCharLiteral(string source, ref int index) + { + // index at the opening quote + index++; + while (index < source.Length) + { + var ch = source[index]; + if (ch == '\\') + { + index += 2; + continue; + } + + if (ch == '\'') + { + index++; + return true; + } + + if (ch == '\n') + { + return false; + } + + index++; + } + + return false; + } + + // index at '$', '@' or '"'. Returns false when the characters do not start a string literal + // (eg '@identifier'); the caller then advances by one. + static bool TrySkipStringLike(string source, ref int index) + { + var cursor = index; + var dollars = 0; + var verbatim = false; + while (cursor < source.Length) + { + var ch = source[cursor]; + if (ch == '$') + { + dollars++; + cursor++; + continue; + } + + if (ch == '@') + { + verbatim = true; + cursor++; + continue; + } + + break; + } + + if (cursor >= source.Length || source[cursor] != '"') + { + return false; + } + + var quotes = QuoteRun(source, cursor); + if (quotes >= 3) + { + // Raw string (interpolated or not): skip blindly to a closing run of >= quotes. + // Interpolation holes are skipped as part of the content. + var search = cursor + quotes; + while (true) + { + if (search >= source.Length) + { + index = source.Length; + return true; + } + + if (source[search] != '"') + { + search++; + continue; + } + + var run = QuoteRun(source, search); + if (run >= quotes) + { + index = search + run; + return true; + } + + search += run; + } + } + + cursor += quotes == 2 ? 2 : 1; + if (quotes == 2) + { + // Empty string "" or interpolated empty string $"" + index = cursor; + return true; + } + + while (cursor < source.Length) + { + var ch = source[cursor]; + if (ch == '"') + { + if (verbatim && + cursor + 1 < source.Length && + source[cursor + 1] == '"') + { + cursor += 2; + continue; + } + + index = cursor + 1; + return true; + } + + if (!verbatim && ch == '\\') + { + cursor += 2; + continue; + } + + if (!verbatim && ch == '\n') + { + // Malformed: unterminated regular string. Stop at the line end. + index = cursor; + return true; + } + + if (dollars > 0 && ch == '{') + { + if (cursor + 1 < source.Length && source[cursor + 1] == '{') + { + cursor += 2; + continue; + } + + if (!TrySkipHole(source, ref cursor)) + { + index = source.Length; + return true; + } + + continue; + } + + if (dollars > 0 && ch == '}' && + cursor + 1 < source.Length && source[cursor + 1] == '}') + { + cursor += 2; + continue; + } + + cursor++; + } + + index = source.Length; + return true; + } + + // cursor at '{' of an interpolation hole; skips past the matching '}' + static bool TrySkipHole(string source, ref int cursor) + { + var depth = 1; + cursor++; + while (cursor < source.Length) + { + var ch = source[cursor]; + switch (ch) + { + case '/': + if (!TrySkipComment(source, ref cursor)) + { + cursor++; + } + + continue; + case '\'': + if (!TrySkipCharLiteral(source, ref cursor)) + { + return false; + } + + continue; + case '"': + case '@': + case '$': + if (!TrySkipStringLike(source, ref cursor)) + { + cursor++; + } + + continue; + case '{': + depth++; + cursor++; + continue; + case '}': + depth--; + cursor++; + if (depth == 0) + { + return true; + } + + continue; + default: + cursor++; + continue; + } + } + + return false; + } + + static int QuoteRun(string source, int index) + { + var count = 0; + while (index + count < source.Length && + source[index + count] == '"') + { + count++; + } + + return count; + } +} diff --git a/src/DiffEngine/Inline/InlineApplier.cs b/src/DiffEngine/Inline/InlineApplier.cs index 2a195984..57920193 100644 --- a/src/DiffEngine/Inline/InlineApplier.cs +++ b/src/DiffEngine/Inline/InlineApplier.cs @@ -88,6 +88,12 @@ static InlineApplyResult LockedApply(string fullPath, InlinePatch patch, string { source = encoding.GetString(bytes, bomLength, bytes.Length - bomLength); } + catch (DecoderFallbackException exception) + { + return InlineApplyResult.Failed( + $"Could not decode as {encoding.WebName}: {fullPath}. Every byte that failed to decode would be replaced on write, so the file is left alone. Convert it to UTF-8 and re-run the test.", + exception); + } catch (Exception exception) { return InlineApplyResult.Failed($"Failed to decode: {fullPath}", exception); @@ -136,29 +142,36 @@ static InlineApplyResult LockedApply(string fullPath, InlinePatch patch, string return InlineApplyResult.Applied; } + /// + /// The encoding to read and write the file with. Every one of them throws rather than + /// substituting: the applier rewrites the whole file, not just the patched span, so a + /// replacement character for an undecodable byte is not a local defect but a file wide one. + /// A source file that is not what its BOM says, or is not UTF-8 when it has no BOM, has to + /// fail loudly and stay as it was. + /// static (Encoding encoding, int bomLength) DetectEncoding(byte[] bytes) { if (bytes is [0xFF, 0xFE, 0x00, 0x00, ..]) { - return (new UTF32Encoding(false, true), 4); + return (new UTF32Encoding(false, true, true), 4); } if (bytes is [0xEF, 0xBB, 0xBF, ..]) { - return (new UTF8Encoding(true), 3); + return (new UTF8Encoding(true, true), 3); } if (bytes is [0xFF, 0xFE, ..]) { - return (new UnicodeEncoding(false, true), 2); + return (new UnicodeEncoding(false, true, true), 2); } if (bytes is [0xFE, 0xFF, ..]) { - return (new UnicodeEncoding(true, true), 2); + return (new UnicodeEncoding(true, true, true), 2); } - return (new UTF8Encoding(false), 0); + return (new UTF8Encoding(false, true), 0); } static string MutexName(string normalizedPath) diff --git a/src/DiffEngine/Inline/InlinePatcher.cs b/src/DiffEngine/Inline/InlinePatcher.cs index 984c0af3..cdc97ad5 100644 --- a/src/DiffEngine/Inline/InlinePatcher.cs +++ b/src/DiffEngine/Inline/InlinePatcher.cs @@ -16,6 +16,12 @@ static class InlinePatcher /// const string methodName = "Snapshot"; + /// + /// The parameter the snapshot literal binds to. Positional in a normal call, but it can be + /// written by name. + /// + const string parameterName = "expected"; + /// /// 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. @@ -35,47 +41,58 @@ public static PatchStatus TryApply( failReason = ""; var eol = DetectEol(source); var lineStarts = BuildLineStarts(source); + var scan = new CsScan(source); if (mode == InlinePatchMode.Remove) { - return TryRemove(source, lineStarts, lineHint, ref newSource, ref failReason); + return TryRemove(source, scan, lineStarts, lineHint, ref newSource, ref failReason); } if (mode == InlinePatchMode.Append) { - return TryAppend(source, lineStarts, lineHint, newContent, eol, ref newSource, ref failReason); + return TryAppend(source, scan, 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 + // Located by content: the Snapshot call whose expected argument is still the text the + // test run saw, nearest the hint first. Matched against expected arguments rather than + // searched for as plain text, because the same literal is just as likely to sit in a + // comment, in another test's snapshot content, or in the verify call on the same line, + // and splicing into one of those leaves a file that no longer compiles and a snapshot + // still unaccepted. // ReSharper disable once RedundantSuppressNullableWarningExpression var needle = NormalizeTo(originalExpression!, eol); - var occurrences = FindAll(source, needle); - if (occurrences.Count > 0) + foreach (var (_, openParen) in FindCalls(source, scan, lineStarts, lineHint, methodName, false)) { - var start = Nearest(occurrences, lineStarts, lineHint); + if (!TryReadArguments(source, scan, openParen, out var expected) || + !expected.Matches(source, needle)) + { + continue; + } + if (CsStringLiteral.TryParse(needle, out var oldValue) && oldValue == newContent) { return PatchStatus.AlreadyApplied; } - var indent = IndentForSpan(source, lineStarts, start); + var indent = IndentForSpan(source, lineStarts, expected.Start); var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol); - newSource = Splice(source, start, start + needle.Length, rendered); + newSource = Splice(source, expected.Start, expected.End, rendered); return PatchStatus.Applied; } // Expression gone: another process may have applied the same patch already - return InsertOrCheck(source, lineStarts, lineHint, newContent, eol, alreadyOnly: true, ref newSource, ref failReason); + return InsertOrCheck(source, scan, lineStarts, lineHint, newContent, eol, alreadyOnly: true, ref newSource, ref failReason); } - return InsertOrCheck(source, lineStarts, lineHint, newContent, eol, alreadyOnly: false, ref newSource, ref failReason); + return InsertOrCheck(source, scan, lineStarts, lineHint, newContent, eol, alreadyOnly: false, ref newSource, ref failReason); } static PatchStatus InsertOrCheck( string source, + CsScan scan, List lineStarts, int lineHint, string newContent, @@ -84,24 +101,19 @@ static PatchStatus InsertOrCheck( ref string newSource, ref string failReason) { - if (!TryFindCall(source, lineStarts, lineHint, out var openParen)) + if (!TryFindCall(source, scan, lineStarts, lineHint, 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 var topCommas)) + if (!TryReadArguments(source, scan, openParen, out var expected)) { failReason = $"Could not parse the argument list of the {methodName} call near line {lineHint}."; 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) + if (expected.IsAbsent) { // The argument was left to its default if (alreadyOnly) @@ -110,17 +122,13 @@ static PatchStatus InsertOrCheck( return PatchStatus.NotFound; } - var emptyIndent = IndentForSpan(source, lineStarts, argStart); + var emptyIndent = IndentForSpan(source, lineStarts, expected.Start); var emptyRendered = CsStringLiteral.RenderRaw(newContent, emptyIndent, eol); - newSource = Splice(source, argStart, argStart, emptyRendered); + newSource = Splice(source, expected.Start, expected.Start, emptyRendered); return PatchStatus.Applied; } - var argOriginalStart = argStart; - var named = TryStripArgumentName(source, ref argStart, out var argumentName); - var argText = source.Substring(argStart, argEnd - argStart); - - if (named && argumentName != "expected") + if (expected.BlockedByName) { // Some other named argument came first (eg file:). // Insert a named expected argument before it. @@ -130,12 +138,13 @@ static PatchStatus InsertOrCheck( return PatchStatus.NotFound; } - var namedIndent = IndentForSpan(source, lineStarts, argOriginalStart); + var namedIndent = IndentForSpan(source, lineStarts, expected.ListStart); var namedRendered = CsStringLiteral.RenderRaw(newContent, namedIndent, eol); - newSource = Splice(source, argOriginalStart, argOriginalStart, "expected: " + namedRendered + ", "); + newSource = Splice(source, expected.ListStart, expected.ListStart, $"{parameterName}: {namedRendered}, "); return PatchStatus.Applied; } + var argText = source.Substring(expected.Start, expected.End - expected.Start); if (argText == "null") { if (alreadyOnly) @@ -144,9 +153,9 @@ static PatchStatus InsertOrCheck( return PatchStatus.NotFound; } - var indent = IndentForSpan(source, lineStarts, argStart); + var indent = IndentForSpan(source, lineStarts, expected.Start); var rendered = CsStringLiteral.RenderRaw(newContent, indent, eol); - newSource = Splice(source, argStart, argEnd, rendered); + newSource = Splice(source, expected.Start, expected.End, rendered); return PatchStatus.Applied; } @@ -170,6 +179,66 @@ static PatchStatus InsertOrCheck( 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."; + /// + /// Where the expected argument of a call is, and in what shape. Worked out once because the + /// content search and the insert path need the same answer: one compares the span, the other + /// decides from the shape what to splice. + /// + readonly struct ExpectedArgument(int start, int end, int listStart, bool blockedByName) + { + /// + /// Start of the argument, past any expected: name. + /// + public int Start { get; } = start; + + public int End { get; } = end; + + /// + /// Start of the first argument, before any argument name. + /// + public int ListStart { get; } = listStart; + + /// + /// The first argument is named, and is not the expected one, so an expected argument has + /// to be inserted in front of it. + /// + public bool BlockedByName { get; } = blockedByName; + + /// + /// The argument was left to its default. + /// + public bool IsAbsent => Start == End; + + /// + /// True when the argument is character for character the given expression. + /// + public bool Matches(string source, string expression) => + !IsAbsent && + !BlockedByName && + End - Start == expression.Length && + string.CompareOrdinal(source, Start, expression, 0, expression.Length) == 0; + } + + static bool TryReadArguments(string source, CsScan scan, int openParen, out ExpectedArgument expected) + { + expected = default; + if (!TryScanArguments(source, scan, openParen, out var closeParen, out var topCommas)) + { + return false; + } + + // expected is the first parameter of Snapshot, so the argument to read is the first one + var start = openParen + 1; + var end = topCommas.Count > 0 ? topCommas[0] : closeParen; + TrimSpan(source, scan, ref start, ref end); + var listStart = start; + var blockedByName = start != end && + TryStripArgumentName(source, ref start, out var argumentName) && + argumentName != parameterName; + expected = new(start, end, listStart, blockedByName); + return true; + } + /// /// 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 @@ -177,6 +246,7 @@ static string StaleReason(int lineHint) => /// static PatchStatus TryAppend( string source, + CsScan scan, List lineStarts, int lineHint, string newContent, @@ -184,19 +254,19 @@ static PatchStatus TryAppend( ref string newSource, ref string failReason) { - if (!TryFindCall(source, lineStarts, lineHint, verifyPrefix, true, out var nameStart, out var openParen)) + if (!TryFindCall(source, scan, 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 _)) + if (!TryScanArguments(source, scan, 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); + 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) { @@ -221,18 +291,19 @@ static PatchStatus TryAppend( /// static PatchStatus TryRemove( string source, + CsScan scan, List lineStarts, int lineHint, ref string newSource, ref string failReason) { - if (!TryFindCall(source, lineStarts, lineHint, methodName, false, out var nameStart, out var openParen)) + if (!TryFindCall(source, scan, 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 _)) + if (!TryScanArguments(source, scan, openParen, out var closeParen, out _)) { failReason = $"Could not parse the argument list of the {methodName} call near line {lineHint}."; return PatchStatus.NotFound; @@ -254,6 +325,7 @@ static PatchStatus TryRemove( } start--; + var dotStart = start; // Then back over the indentation and line break it sat on while (start > 0 && (source[start - 1] == ' ' || source[start - 1] == '\t')) @@ -264,12 +336,16 @@ static PatchStatus TryRemove( if (start > 0 && source[start - 1] == '\n') { - start--; - if (start > 0 && - source[start - 1] == '\r') + var lineBreak = start - 1; + if (lineBreak > 0 && + source[lineBreak - 1] == '\r') { - start--; + lineBreak--; } + + // Not when the line above ends in a line comment: pulling the call up would take the + // semicolon that follows it into the comment + start = scan.IsCode(lineBreak) ? lineBreak : dotStart; } newSource = Splice(source, start, closeParen + 1, ""); @@ -280,34 +356,32 @@ static PatchStatus TryRemove( /// 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) + static int WalkChain(string source, CsScan scan, int index, string name, out bool found) { found = false; while (true) { var cursor = index; - if (!TrySkipTo(source, ref cursor, '.')) + scan.SkipTrivia(ref cursor); + if (cursor >= source.Length || + source[cursor] != '.') { return index; } cursor++; - while (cursor < source.Length && - char.IsWhiteSpace(source[cursor])) - { - cursor++; - } + scan.SkipTrivia(ref cursor); var nameStart = cursor; while (cursor < source.Length && - IsIdentifierChar(source[cursor])) + CsScan.IsIdentifierChar(source[cursor])) { cursor++; } if (cursor == nameStart || - !TrySkipToParen(source, cursor, out var paren) || - !TryScanArguments(source, paren, out var closeParen, out _)) + !TrySkipToParen(source, scan, cursor, out var paren) || + !TryScanArguments(source, scan, paren, out var closeParen, out _)) { return index; } @@ -322,18 +396,6 @@ static int WalkChain(string source, int index, string name, out bool found) } } - 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]; @@ -347,16 +409,12 @@ static string LeadingWhitespace(string source, List lineStarts, int offset) 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); + static bool TryFindCall(string source, CsScan scan, List lineStarts, int lineHint, out int openParen) => + TryFindCall(source, scan, 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, + CsScan scan, List lineStarts, int lineHint, string name, @@ -364,16 +422,39 @@ static bool TryFindCall( out int nameStart, out int openParen) { + foreach (var call in FindCalls(source, scan, lineStarts, lineHint, name, byPrefix)) + { + (nameStart, openParen) = call; + return true; + } + nameStart = -1; openParen = -1; + return false; + } + + /// + /// Locates calls by name, searching outward from the hint: hint, hint+1, hint-1, hint+2 and so + /// on. A tie goes to the line at or after the hint, because a file that moved under a pending + /// patch usually grew above the call rather than below it. + /// matches any identifier starting with , + /// which is how the several Verify overloads are found with one search. + /// + static IEnumerable<(int nameStart, int openParen)> FindCalls( + string source, + CsScan scan, + List lineStarts, + int lineHint, + string name, + bool byPrefix) + { var lineCount = lineStarts.Count; lineHint = Math.Min(Math.Max(lineHint, 1), lineCount); - // Outward from the hint: hint, hint-1, hint+1, hint-2, ... for (var distance = 0; distance < lineCount; distance++) { var candidates = distance == 0 ? new[] { lineHint } - : new[] { lineHint - distance, lineHint + distance }; + : new[] { lineHint + distance, lineHint - distance }; foreach (var line in candidates) { if (line < 1 || line > lineCount) @@ -396,50 +477,51 @@ static bool TryFindCall( if (byPrefix) { while (identifierEnd < source.Length && - IsIdentifierChar(source[identifierEnd])) + CsScan.IsIdentifierChar(source[identifierEnd])) { identifierEnd++; } } else if (identifierEnd < source.Length && - IsIdentifierChar(source[identifierEnd])) + CsScan.IsIdentifierChar(source[identifierEnd])) { index += name.Length; continue; } - if (StartsToken(source, index) && - TrySkipToParen(source, identifierEnd, out var paren)) + // In code, a whole token, an invocation rather than a declaration, and + // followed by an argument list. A commented out example passes none of these + if (scan.IsCode(index) && + StartsToken(source, index) && + !scan.IsDeclaration(index) && + TrySkipToParen(source, scan, identifierEnd, out var paren)) { - nameStart = index; - openParen = paren; - return true; + yield return (index, paren); } index += name.Length; } } } - - return false; } static bool StartsToken(string source, int index) => index == 0 || - !IsIdentifierChar(source[index - 1]); + !CsScan.IsIdentifierChar(source[index - 1]); - static bool IsIdentifierChar(char ch) => - char.IsLetterOrDigit(ch) || ch == '_'; - - static bool TrySkipToParen(string source, int index, out int paren) + static bool TrySkipToParen(string source, CsScan scan, int index, out int paren) { paren = -1; - while (index < source.Length && char.IsWhiteSpace(source[index])) + scan.SkipTrivia(ref index); + if (index < source.Length && + source[index] == '<' && + CsScan.TrySkipTypeArguments(source, ref index)) { - index++; + scan.SkipTrivia(ref index); } - if (index < source.Length && source[index] == '(') + if (index < source.Length && + source[index] == '(') { paren = index; return true; @@ -449,8 +531,8 @@ static bool TrySkipToParen(string source, int index, out int paren) } // Scans a balanced argument list starting at the open paren. - // Records top level comma positions. Skips strings, chars and comments. - static bool TryScanArguments(string source, int openParen, out int closeParen, out List topCommas) + // Records top level comma positions. Comments and literals are stepped over whole. + static bool TryScanArguments(string source, CsScan scan, int openParen, out int closeParen, out List topCommas) { closeParen = -1; topCommas = []; @@ -458,32 +540,14 @@ static bool TryScanArguments(string source, int openParen, out int closeParen, o var index = openParen + 1; while (index < source.Length) { - var ch = source[index]; - switch (ch) + if (scan.TryGetSkip(index, out var skipTo)) { - case '/': - if (!TrySkipComment(source, ref index)) - { - index++; - } - - continue; - case '\'': - if (!TrySkipCharLiteral(source, ref index)) - { - return false; - } - - continue; - case '"': - case '@': - case '$': - if (!TrySkipStringLike(source, ref index)) - { - index++; - } + index = skipTo; + continue; + } - continue; + switch (source[index]) + { case '(': case '[': case '{': @@ -527,267 +591,6 @@ static bool TryScanArguments(string source, int openParen, out int closeParen, o return false; } - static bool TrySkipComment(string source, ref int index) - { - if (index + 1 >= source.Length) - { - return false; - } - - var next = source[index + 1]; - if (next == '/') - { - var end = source.IndexOf('\n', index); - index = end < 0 ? source.Length : end + 1; - return true; - } - - if (next == '*') - { - var end = source.IndexOf("*/", index + 2, StringComparison.Ordinal); - index = end < 0 ? source.Length : end + 2; - return true; - } - - return false; - } - - static bool TrySkipCharLiteral(string source, ref int index) - { - // index at the opening quote - index++; - while (index < source.Length) - { - var ch = source[index]; - if (ch == '\\') - { - index += 2; - continue; - } - - if (ch == '\'') - { - index++; - return true; - } - - if (ch == '\n') - { - return false; - } - - index++; - } - - return false; - } - - // index at '$', '@' or '"'. Returns false when the characters do not start a string literal - // (eg '@identifier'); the caller then advances by one. - static bool TrySkipStringLike(string source, ref int index) - { - var cursor = index; - var dollars = 0; - var verbatim = false; - while (cursor < source.Length) - { - var ch = source[cursor]; - if (ch == '$') - { - dollars++; - cursor++; - continue; - } - - if (ch == '@') - { - verbatim = true; - cursor++; - continue; - } - - break; - } - - if (cursor >= source.Length || source[cursor] != '"') - { - return false; - } - - var quotes = QuoteRun(source, cursor); - if (quotes >= 3) - { - // Raw string (interpolated or not): skip blindly to a closing run of >= quotes. - // Interpolation holes are skipped as part of the content. - var search = cursor + quotes; - while (true) - { - if (search >= source.Length) - { - index = source.Length; - return true; - } - - if (source[search] != '"') - { - search++; - continue; - } - - var run = QuoteRun(source, search); - if (run >= quotes) - { - index = search + run; - return true; - } - - search += run; - } - } - - cursor += quotes == 2 ? 2 : 1; - if (quotes == 2 && dollars == 0) - { - // Empty string "" - index = cursor; - return true; - } - - if (quotes == 2) - { - // Interpolated empty string $"" - index = cursor; - return true; - } - - while (cursor < source.Length) - { - var ch = source[cursor]; - if (ch == '"') - { - if (verbatim && - cursor + 1 < source.Length && - source[cursor + 1] == '"') - { - cursor += 2; - continue; - } - - index = cursor + 1; - return true; - } - - if (!verbatim && ch == '\\') - { - cursor += 2; - continue; - } - - if (!verbatim && ch == '\n') - { - // Malformed: unterminated regular string. Stop at the line end. - index = cursor; - return true; - } - - if (dollars > 0 && ch == '{') - { - if (cursor + 1 < source.Length && source[cursor + 1] == '{') - { - cursor += 2; - continue; - } - - if (!TrySkipHole(source, ref cursor)) - { - index = source.Length; - return true; - } - - continue; - } - - if (dollars > 0 && ch == '}' && - cursor + 1 < source.Length && source[cursor + 1] == '}') - { - cursor += 2; - continue; - } - - cursor++; - } - - index = source.Length; - return true; - } - - // cursor at '{' of an interpolation hole; skips past the matching '}' - static bool TrySkipHole(string source, ref int cursor) - { - var depth = 1; - cursor++; - while (cursor < source.Length) - { - var ch = source[cursor]; - switch (ch) - { - case '/': - if (!TrySkipComment(source, ref cursor)) - { - cursor++; - } - - continue; - case '\'': - if (!TrySkipCharLiteral(source, ref cursor)) - { - return false; - } - - continue; - case '"': - case '@': - case '$': - if (!TrySkipStringLike(source, ref cursor)) - { - cursor++; - } - - continue; - case '{': - depth++; - cursor++; - continue; - case '}': - depth--; - cursor++; - if (depth == 0) - { - return true; - } - - continue; - default: - cursor++; - continue; - } - } - - return false; - } - - static int QuoteRun(string source, int index) - { - var count = 0; - while (index + count < source.Length && - source[index + count] == '"') - { - count++; - } - - return count; - } - static bool TryStripArgumentName(string source, ref int start, out string name) { name = ""; @@ -797,7 +600,7 @@ static bool TryStripArgumentName(string source, ref int start, out string name) return false; } - while (index < source.Length && IsIdentifierChar(source[index])) + while (index < source.Length && CsScan.IsIdentifierChar(source[index])) { index++; } @@ -826,16 +629,48 @@ static bool TryStripArgumentName(string source, ref int start, out string name) return true; } - static void TrimSpan(string source, ref int start, ref int end) + /// + /// Narrows a span to the expression in it: whitespace and comments are not part of the + /// argument, and leaving a comment in makes the argument read as something other than the + /// literal it is. + /// + static void TrimSpan(string source, CsScan scan, ref int start, ref int end) { - while (start < end && char.IsWhiteSpace(source[start])) + while (start < end) { - start++; + if (char.IsWhiteSpace(source[start])) + { + start++; + continue; + } + + if (source[start] == '/' && + scan.TryGetSkip(start, out var afterComment) && + afterComment <= end) + { + start = afterComment; + continue; + } + + break; } - while (end > start && char.IsWhiteSpace(source[end - 1])) + while (end > start) { - end--; + if (char.IsWhiteSpace(source[end - 1])) + { + end--; + continue; + } + + if (scan.TryGetCommentEndingAt(end, out var commentStart) && + commentStart >= start) + { + end = commentStart; + continue; + } + + break; } } @@ -920,47 +755,6 @@ static int LineOf(List lineStarts, int offset) return low + 1; } - static List FindAll(string source, string needle) - { - List result = []; - var index = 0; - while (true) - { - index = source.IndexOf(needle, index, StringComparison.Ordinal); - if (index < 0) - { - break; - } - - result.Add(index); - index++; - } - - return result; - } - - static int Nearest(List occurrences, List lineStarts, int lineHint) - { - var best = occurrences[0]; - var bestDistance = int.MaxValue; - var bestAfter = false; - foreach (var occurrence in occurrences) - { - var line = LineOf(lineStarts, occurrence); - var distance = Math.Abs(line - lineHint); - var after = line >= lineHint; - if (distance < bestDistance || - distance == bestDistance && after && !bestAfter) - { - best = occurrence; - bestDistance = distance; - bestAfter = after; - } - } - - return best; - } - static string IndentForSpan(string source, List lineStarts, int spanStart) { var line = LineOf(lineStarts, spanStart);