Skip to content

Commit

Permalink
Merge pull request #70407 from CyrusNajmabadi/lspText
Browse files Browse the repository at this point in the history 
Fix lsp didChange processing.
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Oct 17, 2023
2 parents d16d2ba + 872924e commit 2e99c97
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 67 deletions.
16 changes: 8 additions & 8 deletions src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,14 @@ public async Task OpenDocumentAsync(Uri documentUri, string? text = null, string
await ExecuteRequestAsync<LSP.DidOpenTextDocumentParams, object>(LSP.Methods.TextDocumentDidOpenName, didOpenParams, CancellationToken.None);
}

public Task ReplaceTextAsync(Uri documentUri, params (LSP.Range Range, string Text)[] changes)
{
var didChangeParams = CreateDidChangeTextDocumentParams(
documentUri,
changes.ToImmutableArray());
return ExecuteRequestAsync<LSP.DidChangeTextDocumentParams, object>(LSP.Methods.TextDocumentDidChangeName, didChangeParams, CancellationToken.None);
}

public Task InsertTextAsync(Uri documentUri, params (int Line, int Column, string Text)[] changes)
{
return ReplaceTextAsync(documentUri, changes.Select(change => (new LSP.Range
Expand All @@ -677,14 +685,6 @@ public Task InsertTextAsync(Uri documentUri, params (int Line, int Column, strin
}, change.Text)).ToArray());
}

public Task ReplaceTextAsync(Uri documentUri, params (LSP.Range Range, string Text)[] changes)
{
var didChangeParams = CreateDidChangeTextDocumentParams(
documentUri,
changes.Select(change => (change.Range, change.Text)).ToImmutableArray());
return ExecuteRequestAsync<LSP.DidChangeTextDocumentParams, object>(LSP.Methods.TextDocumentDidChangeName, didChangeParams, CancellationToken.None);
}

public Task DeleteTextAsync(Uri documentUri, params (int StartLine, int StartColumn, int EndLine, int EndColumn)[] changes)
{
return ReplaceTextAsync(documentUri, changes.Select(change => (new LSP.Range
Expand Down
49 changes: 22 additions & 27 deletions src/Features/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,39 @@

using System;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Roslyn.Utilities;
using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler.DocumentChanges
{
[ExportCSharpVisualBasicStatelessLspService(typeof(DidChangeHandler)), Shared]
[Method(LSP.Methods.TextDocumentDidChangeName)]
internal class DidChangeHandler : ILspServiceDocumentRequestHandler<LSP.DidChangeTextDocumentParams, object?>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public DidChangeHandler()
{
}

public bool MutatesSolutionState => true;
public bool RequiresLSPSolution => false;
namespace Microsoft.CodeAnalysis.LanguageServer.Handler.DocumentChanges;

public TextDocumentIdentifier GetTextDocumentIdentifier(LSP.DidChangeTextDocumentParams request) => request.TextDocument;
[ExportCSharpVisualBasicStatelessLspService(typeof(DidChangeHandler)), Shared]
[Method(Methods.TextDocumentDidChangeName)]
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal class DidChangeHandler() : ILspServiceDocumentRequestHandler<DidChangeTextDocumentParams, object?>
{
public bool MutatesSolutionState => true;
public bool RequiresLSPSolution => false;

public Task<object?> HandleRequestAsync(LSP.DidChangeTextDocumentParams request, RequestContext context, CancellationToken cancellationToken)
{
var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri);
public TextDocumentIdentifier GetTextDocumentIdentifier(DidChangeTextDocumentParams request)
=> request.TextDocument;

// 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.
var changes = request.ContentChanges.Select(change => ProtocolConversions.ContentChangeEventToTextChange(change, text));
public Task<object?> HandleRequestAsync(DidChangeTextDocumentParams request, RequestContext context, CancellationToken cancellationToken)
{
var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri);

text = text.WithChanges(changes);
// 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));

context.UpdateTrackedDocument(request.TextDocument.Uri, text);
context.UpdateTrackedDocument(request.TextDocument.Uri, text);

return SpecializedTasks.Default<object>();
}
return SpecializedTasks.Default<object>();
}
}
110 changes: 78 additions & 32 deletions src/Features/LanguageServer/ProtocolUnitTests/DocumentChanges/DocumentChangesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,25 +247,29 @@ void M()
}

[Theory, CombinatorialData]
public async Task DidChange_MultipleChanges(bool mutatingLspWorkspace)
public async Task DidChange_MultipleChanges1(bool mutatingLspWorkspace)
{
var source =
@"class A
{
void M()
{
{|type:|}
}
}";
"""
class A
{
void M()
{
{|type:|}
}
}
""";
var expected =
@"class A
{
void M()
{
// hi there
// this builds on that
}
}";
"""
class A
{
void M()
{
// hi there
// this builds on that
}
}
""";

var (testLspServer, locationTyped, _) = await GetTestLspServerAndLocationAsync(source, mutatingLspWorkspace);

Expand All @@ -282,26 +286,69 @@ void M()
}
}

[Theory, CombinatorialData]
public async Task DidChange_MultipleChanges2(bool mutatingLspWorkspace)
{
var source =
"""
class A
{
void M()
{
{|type:|}
}
}
""";
var expected =
"""
class A
{
void M()
{
// hi there
}
}
""";

var (testLspServer, locationTyped, _) = await GetTestLspServerAndLocationAsync(source, mutatingLspWorkspace);

await using (testLspServer)
{
await DidOpen(testLspServer, locationTyped.Uri);

await DidChange(testLspServer, locationTyped.Uri, (4, 8, "// there"), (4, 11, "hi "));

var document = testLspServer.GetTrackedTexts().FirstOrDefault();

AssertEx.NotNull(document);
Assert.Equal(expected, document.ToString());
}
}

[Theory, CombinatorialData]
public async Task DidChange_MultipleRequests(bool mutatingLspWorkspace)
{
var source =
@"class A
{
void M()
{
{|type:|}
}
}";
"""
class A
{
void M()
{
{|type:|}
}
}
""";
var expected =
@"class A
{
void M()
{
// hi there
// this builds on that
}
}";
"""
class A
{
void M()
{
// hi there
// this builds on that
}
}
""";

var (testLspServer, locationTyped, _) = await GetTestLspServerAndLocationAsync(source, mutatingLspWorkspace);

Expand All @@ -310,7 +357,6 @@ void M()
await DidOpen(testLspServer, locationTyped.Uri);

await DidChange(testLspServer, locationTyped.Uri, (4, 8, "// hi there"));

await DidChange(testLspServer, locationTyped.Uri, (5, 0, " // this builds on that\r\n"));

var document = testLspServer.GetTrackedTexts().FirstOrDefault();
Expand Down

0 comments on commit 2e99c97

Please sign in to comment.