diff --git a/src/LanguageServer/Protocol/Extensions/Extensions.cs b/src/LanguageServer/Protocol/Extensions/Extensions.cs index d9b8e4078f89f..2523b9c721708 100644 --- a/src/LanguageServer/Protocol/Extensions/Extensions.cs +++ b/src/LanguageServer/Protocol/Extensions/Extensions.cs @@ -243,5 +243,20 @@ private static bool TryGetVSCompletionListSetting(ClientCapabilities clientCapab return true; } + + public static int CompareTo(this Position p1, Position p2) + { + if (p1.Line > p2.Line) + return 1; + else if (p1.Line < p2.Line) + return -1; + + if (p1.Character > p2.Character) + return 1; + else if (p1.Character < p2.Character) + return -1; + + return 0; + } } } diff --git a/src/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs b/src/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs index e4586f2f83a01..b13b4ae983006 100644 --- a/src/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs +++ b/src/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs @@ -4,9 +4,11 @@ using System; using System.Composition; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Text; using Roslyn.LanguageServer.Protocol; using Roslyn.Utilities; @@ -28,15 +30,53 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(DidChangeTextDocumentPar { var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri); - // Per the LSP spec, each text change builds upon the previous, so we don't need to translate any text - // positions between changes, which makes this quite easy. See - // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#didChangeTextDocumentParams - // for more details. - foreach (var change in request.ContentChanges) - text = text.WithChanges(ProtocolConversions.ContentChangeEventToTextChange(change, text)); + text = GetUpdatedSourceText(request.ContentChanges, text); context.UpdateTrackedDocument(request.TextDocument.Uri, text); return SpecializedTasks.Default(); } + + internal static bool AreChangesInReverseOrder(TextDocumentContentChangeEvent[] contentChanges) + { + for (var i = 1; i < contentChanges.Length; i++) + { + var prevChange = contentChanges[i - 1]; + var curChange = contentChanges[i]; + + if (prevChange.Range.Start.CompareTo(curChange.Range.End) < 0) + { + return false; + } + } + + return true; + } + + private static SourceText GetUpdatedSourceText(TextDocumentContentChangeEvent[] contentChanges, SourceText text) + { + // Per the LSP spec, each text change builds upon the previous, so we don't need to translate any text + // positions between changes. See + // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#didChangeTextDocumentParams + // for more details. + // + // If the host sends us changes in a way such that no earlier change can affect the position of a later change, + // then we can merge the changes into a single TextChange, allowing creation of only a single new + // source text. + if (AreChangesInReverseOrder(contentChanges)) + { + // The changes were in reverse document order, so we can merge them into a single operation on the source text. + // Note that the WithChanges implementation works more efficiently with it's input in forward document order. + var newChanges = contentChanges.Reverse().SelectAsArray(change => ProtocolConversions.ContentChangeEventToTextChange(change, text)); + text = text.WithChanges(newChanges); + } + else + { + // The host didn't send us the items ordered, so we'll apply each one independently. + foreach (var change in contentChanges) + text = text.WithChanges(ProtocolConversions.ContentChangeEventToTextChange(change, text)); + } + + return text; + } } diff --git a/src/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs b/src/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs index 3bafb1e06a808..2c2164f5ad65d 100644 --- a/src/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.LanguageServer.Handler.DocumentChanges; using Roslyn.Test.Utilities; using Xunit; using Xunit.Abstractions; @@ -245,7 +246,7 @@ void M() } [Theory, CombinatorialData] - public async Task DidChange_MultipleChanges1(bool mutatingLspWorkspace) + public async Task DidChange_MultipleChanges_ForwardOrder(bool mutatingLspWorkspace) { var source = """ @@ -285,7 +286,7 @@ void M() } [Theory, CombinatorialData] - public async Task DidChange_MultipleChanges2(bool mutatingLspWorkspace) + public async Task DidChange_MultipleChanges_Overlapping(bool mutatingLspWorkspace) { var source = """ @@ -323,6 +324,89 @@ void M() } } + [Theory, CombinatorialData] + public async Task DidChange_MultipleChanges_ReverseOrder(bool mutatingLspWorkspace) + { + var source = + """ + class A + { + void M() + { + {|type:|} + } + } + """; + var expected = + """ + class A + { + void M() + { + // hi there + // this builds on that + } + } + """; + + var (testLspServer, locationTyped, _) = await GetTestLspServerAndLocationAsync(source, mutatingLspWorkspace); + + await using (testLspServer) + { + await DidOpen(testLspServer, locationTyped.Uri); + + await DidChange(testLspServer, locationTyped.Uri, (5, 0, " // this builds on that\r\n"), (4, 8, "// hi there")); + + var document = testLspServer.GetTrackedTexts().FirstOrDefault(); + + AssertEx.NotNull(document); + Assert.Equal(expected, document.ToString()); + } + } + + private LSP.TextDocumentContentChangeEvent CreateTextDocumentContentChangeEvent(int startLine, int startCol, int endLine, int endCol, string newText) + { + return new LSP.TextDocumentContentChangeEvent() + { + Range = new LSP.Range() + { + Start = new LSP.Position(startLine, startCol), + End = new LSP.Position(endLine, endCol) + }, + Text = newText + }; + } + + [Fact] + public void DidChange_AreChangesInReverseOrder_True() + { + LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 7, endLine: 0, endCol: 9, newText: "test3"); + LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 5, endLine: 0, endCol: 7, newText: "test2"); + LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1"); + + Assert.True(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3])); + } + + [Fact] + public void DidChange_AreChangesInReverseOrder_InForwardOrder() + { + LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1"); + LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 5, endLine: 0, endCol: 7, newText: "test2"); + LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 7, endLine: 0, endCol: 9, newText: "test3"); + + Assert.False(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3])); + } + + [Fact] + public void DidChange_AreChangesInReverseOrder_Overlapping() + { + LSP.TextDocumentContentChangeEvent change1 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 1, endLine: 0, endCol: 3, newText: "test1"); + LSP.TextDocumentContentChangeEvent change2 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 2, endLine: 0, endCol: 4, newText: "test2"); + LSP.TextDocumentContentChangeEvent change3 = CreateTextDocumentContentChangeEvent(startLine: 0, startCol: 3, endLine: 0, endCol: 5, newText: "test3"); + + Assert.False(DidChangeHandler.AreChangesInReverseOrder([change1, change2, change3])); + } + [Theory, CombinatorialData] public async Task DidChange_MultipleRequests(bool mutatingLspWorkspace) {