Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Switch to STJ for client side LSP serialization #74364

Merged
merged 4 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Microsoft.VisualStudio.Composition;
using Microsoft.VisualStudio.LanguageServer.Client;
using Microsoft.VisualStudio.Threading;
using Microsoft.VisualStudio.Utilities;
using Nerdbank.Streams;
using Roslyn.LanguageServer.Protocol;
using StreamJsonRpc;
Expand All @@ -31,12 +32,10 @@ internal abstract partial class AbstractInProcLanguageClient(
ILspServiceLoggerFactory lspLoggerFactory,
IThreadingContext threadingContext,
ExportProvider exportProvider,
AbstractLanguageClientMiddleLayer? middleLayer = null) : ILanguageClient, ILanguageServerFactory, ICapabilitiesProvider, ILanguageClientCustomMessage2
AbstractLanguageClientMiddleLayer? middleLayer = null) : ILanguageClient, ILanguageServerFactory, ICapabilitiesProvider, ILanguageClientCustomMessage2, IPropertyOwner
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly IThreadingContext _threadingContext = threadingContext;
#pragma warning disable CS0618 // Type or member is obsolete - blocked on Razor switching to new APIs for STJ - https://github.com/dotnet/roslyn/issues/73317
private readonly ILanguageClientMiddleLayer? _middleLayer = middleLayer;
#pragma warning restore CS0618 // Type or member is obsolete
private readonly ILanguageClientMiddleLayer2<JsonElement>? _middleLayer = middleLayer;
private readonly ILspServiceLoggerFactory _lspLoggerFactory = lspLoggerFactory;
private readonly ExportProvider _exportProvider = exportProvider;

Expand Down Expand Up @@ -102,6 +101,8 @@ internal abstract partial class AbstractInProcLanguageClient(
/// </summary>
public IEnumerable<string>? FilesToWatch { get; }

public PropertyCollection Properties { get; } = CreateStjPropertyCollection();
dibarbet marked this conversation as resolved.
Show resolved Hide resolved

public event AsyncEventHandler<EventArgs>? StartAsync;

/// <summary>
Expand Down Expand Up @@ -267,6 +268,13 @@ public virtual AbstractLanguageServer<RequestContext> Create(
/// </summary>
public Task AttachForCustomMessageAsync(JsonRpc rpc) => Task.CompletedTask;

private static PropertyCollection CreateStjPropertyCollection()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the client will be opting into STJ by default in 17.12 Preview 1, at which point we can remove this. However we have additional changes, so we need to go in first with an explicit opt-in.

{
var collection = new PropertyCollection();
collection.AddProperty("lsp-serialization", "stj");
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
return collection;
}

internal TestAccessor GetTestAccessor()
{
return new TestAccessor(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Text.Json;
using System.Threading.Tasks;
using Microsoft.VisualStudio.LanguageServer.Client;
using Newtonsoft.Json.Linq;

namespace Microsoft.CodeAnalysis.Editor.Implementation.LanguageClient;

#pragma warning disable CS0618 // Type or member is obsolete - blocked on Razor switching to new APIs for STJ - https://github.com/dotnet/roslyn/issues/73317
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Type or member is obsolete

Is this pragma or the NewtonSoft using still needed?

internal abstract class AbstractLanguageClientMiddleLayer : ILanguageClientMiddleLayer
internal abstract class AbstractLanguageClientMiddleLayer : ILanguageClientMiddleLayer2<JsonElement>
#pragma warning restore CS0618 // Type or member is obsolete
{
public abstract bool CanHandle(string methodName);

public abstract Task HandleNotificationAsync(string methodName, JToken methodParam, Func<JToken, Task> sendNotification);
public abstract Task HandleNotificationAsync(string methodName, JsonElement methodParam, Func<JsonElement, Task> sendNotification);

public abstract Task<JToken?> HandleRequestAsync(string methodName, JToken methodParam, Func<JToken, Task<JToken?>> sendRequest);
public abstract Task<JsonElement> HandleRequestAsync(string methodName, JsonElement methodParam, Func<JsonElement, Task<JsonElement>> sendRequest);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Editor.Implementation.LanguageClient;
using Newtonsoft.Json.Linq;
using System.Text.Json;
dibarbet marked this conversation as resolved.
Show resolved Hide resolved

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor
{
Expand All @@ -27,10 +28,17 @@ public RazorCSharpInterceptionMiddleLayerWrapper(IRazorCSharpInterceptionMiddleL
public override bool CanHandle(string methodName)
=> _razorCSharpInterceptionMiddleLayer.CanHandle(methodName);

public override Task HandleNotificationAsync(string methodName, JToken methodParam, Func<JToken, Task> sendNotification)
=> _razorCSharpInterceptionMiddleLayer.HandleNotificationAsync(methodName, methodParam, sendNotification);
public override Task HandleNotificationAsync(string methodName, JsonElement methodParam, Func<JsonElement, Task> sendNotification)
{
// Razor only ever looks at the method name, so it is safe to pass null for all the Newtonsoft JToken params.
return _razorCSharpInterceptionMiddleLayer.HandleNotificationAsync(methodName, null!, null!);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

named parameters?

}

public override Task<JToken?> HandleRequestAsync(string methodName, JToken methodParam, Func<JToken, Task<JToken?>> sendRequest)
=> _razorCSharpInterceptionMiddleLayer.HandleRequestAsync(methodName, methodParam, sendRequest);
public override Task<JsonElement> HandleRequestAsync(string methodName, JsonElement methodParam, Func<JsonElement, Task<JsonElement>> sendRequest)
{
// Razor only implements a middlelayer for smeantic tokens refresh, which is a notification.
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
// Cohosting makes all this unnecessary, so keeping this as minimal as possible until then.
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Editor.Test;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.LanguageServer.UnitTests;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down Expand Up @@ -43,7 +44,7 @@ protected class DocumentOutlineTestMocks : IAsyncDisposable
private readonly IAsyncDisposable _disposable;

internal DocumentOutlineTestMocks(
LanguageServiceBrokerCallback<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]> languageServiceBrokerCallback,
LanguageServiceBrokerCallback<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]> languageServiceBrokerCallback,
IThreadingContext threadingContext,
EditorTestWorkspace workspace,
IAsyncDisposable disposable)
Expand All @@ -55,7 +56,7 @@ internal DocumentOutlineTestMocks(
TextBuffer = workspace.Documents.Single().GetTextBuffer();
}

internal LanguageServiceBrokerCallback<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]> LanguageServiceBrokerCallback { get; }
internal LanguageServiceBrokerCallback<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]> LanguageServiceBrokerCallback { get; }

internal IThreadingContext ThreadingContext { get; }

Expand All @@ -78,30 +79,22 @@ protected async Task<DocumentOutlineTestMocks> CreateMocksAsync(string code)
var workspace = EditorTestWorkspace.CreateCSharp(code, composition: s_composition);
var threadingContext = workspace.GetService<IThreadingContext>();

var testLspServer = await CreateTestLspServerAsync(workspace, new InitializationOptions
{
// Set the message formatter to use newtonsoft on the client side to match real behavior.
// Also avoid calling initialize / initialized as the test harness uses types only compatible with STJ.
// TODO - switch back to STJ with https://github.com/dotnet/roslyn/issues/73317
ClientMessageFormatter = new JsonMessageFormatter(),
CallInitialize = false,
CallInitialized = false
});
var testLspServer = await CreateTestLspServerAsync(workspace);

var mocks = new DocumentOutlineTestMocks(RequestAsync, threadingContext, workspace, testLspServer);
return mocks;

async Task<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]?> RequestAsync(Request<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]> request, CancellationToken cancellationToken)
async Task<RoslynDocumentSymbol[]?> RequestAsync(Request<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]> request, CancellationToken cancellationToken)
{
var docRequest = (DocumentRequest<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]>)request;
var docRequest = (DocumentRequest<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]>)request;
var parameters = docRequest.ParameterFactory(docRequest.TextBuffer.CurrentSnapshot);
var response = await testLspServer.ExecuteRequestAsync<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]>(request.Method, parameters, cancellationToken);
var response = await testLspServer.ExecuteRequestAsync<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]>(request.Method, parameters, cancellationToken);

return response;
}
}

private async Task<TestLspServer> CreateTestLspServerAsync(EditorTestWorkspace workspace, InitializationOptions initializationOptions)
private async Task<TestLspServer> CreateTestLspServerAsync(EditorTestWorkspace workspace)
{
var solution = workspace.CurrentSolution;

Expand Down Expand Up @@ -132,13 +125,7 @@ private async Task<TestLspServer> CreateTestLspServerAsync(EditorTestWorkspace w
var workspaceWaiter = operations.GetWaiter(FeatureAttribute.Workspace);
await workspaceWaiter.ExpeditedWaitAsync();

var server = await TestLspServer.CreateAsync(workspace, initializationOptions, _logger);

// We disable the default test initialize call because the default test harness intialize types only support STJ (not newtonsoft).
// We only care that initialize has been called with some capability, so call with simple objects.
// TODO - remove with switch to STJ in https://github.com/dotnet/roslyn/issues/73317
await server.ExecuteRequestAsync<object, object>(Roslyn.LanguageServer.Protocol.Methods.InitializeName, new NewtonsoftInitializeParams() { Capabilities = new object() }, CancellationToken.None);

var server = await TestLspServer.CreateAsync(workspace, new InitializationOptions(), _logger);
return server;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.LanguageServer;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.PatternMatching;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -25,25 +26,30 @@ internal sealed partial class DocumentOutlineViewModel
/// Makes an LSP document symbol request and returns the response and the text snapshot used at
/// the time the LSP client sends the request to the server.
/// </summary>
public static async Task<(DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[] response, ITextSnapshot snapshot)?> DocumentSymbolsRequestAsync(
public static async Task<(RoslynDocumentSymbol[] response, ITextSnapshot snapshot)?> DocumentSymbolsRequestAsync(
ITextBuffer textBuffer,
LanguageServiceBrokerCallback<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]> callbackAsync,
LanguageServiceBrokerCallback<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]> callbackAsync,
string textViewFilePath,
CancellationToken cancellationToken)
{
ITextSnapshot? requestSnapshot = null;

var request = new DocumentRequest<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]>()
var request = new DocumentRequest<RoslynDocumentSymbolParams, RoslynDocumentSymbol[]>()
{
Method = Methods.TextDocumentDocumentSymbolName,
LanguageServerName = WellKnownLspServerKinds.AlwaysActiveVSLspServer.ToUserVisibleString(),
TextBuffer = textBuffer,
ParameterFactory = (snapshot) =>
{
requestSnapshot = snapshot;
return new DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams(
new DocumentSymbolNewtonsoft.NewtonsoftTextDocumentIdentifier(ProtocolConversions.CreateAbsoluteUri(textViewFilePath)),
UseHierarchicalSymbols: true);
return new RoslynDocumentSymbolParams
{
TextDocument = new TextDocumentIdentifier
{
Uri = ProtocolConversions.CreateAbsoluteUri(textViewFilePath),
},
UseHierarchicalSymbols = true
};
}
};

Expand Down Expand Up @@ -88,7 +94,7 @@ internal sealed partial class DocumentOutlineViewModel
/// ]
/// }
/// ]
public static ImmutableArray<DocumentSymbolData> CreateDocumentSymbolData(DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[] documentSymbols, ITextSnapshot textSnapshot)
public static ImmutableArray<DocumentSymbolData> CreateDocumentSymbolData(RoslynDocumentSymbol[] documentSymbols, ITextSnapshot textSnapshot)
{
// Obtain a flat list of all the document symbols sorted by location in the document.
var allSymbols = documentSymbols
Expand All @@ -108,7 +114,7 @@ public static ImmutableArray<DocumentSymbolData> CreateDocumentSymbolData(Docume

// Returns the symbol in the list at index start (the parent symbol) with the following symbols in the list
// (descendants) appropriately nested into the parent.
DocumentSymbolData NestDescendantSymbols(ImmutableArray<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol> allSymbols, int start, out int newStart)
DocumentSymbolData NestDescendantSymbols(ImmutableArray<RoslynDocumentSymbol> allSymbols, int start, out int newStart)
{
var currentParent = allSymbols[start];
start++;
Expand Down Expand Up @@ -141,18 +147,20 @@ DocumentSymbolData NestDescendantSymbols(ImmutableArray<DocumentSymbolNewtonsoft
}

// Returns whether the child symbol is in range of the parent symbol.
static bool Contains(DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol parent, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol child)
static bool Contains(RoslynDocumentSymbol parent, RoslynDocumentSymbol child)
{
var parentRange = RangeToLinePositionSpan(parent.Range);
var childRange = RangeToLinePositionSpan(child.Range);
return childRange.Start > parentRange.Start && childRange.End <= parentRange.End;

static LinePositionSpan RangeToLinePositionSpan(DocumentSymbolNewtonsoft.NewtonsoftRange range)
=> new(new LinePosition(range.Start.Line, range.Start.Character), new LinePosition(range.End.Line, range.End.Character));
static LinePositionSpan RangeToLinePositionSpan(Range range)
{
return new(new LinePosition(range.Start.Line, range.Start.Character), new LinePosition(range.End.Line, range.End.Character));
}
}

// Converts a Document Symbol Range to a SnapshotSpan within the text snapshot used for the LSP request.
SnapshotSpan GetSymbolRangeSpan(DocumentSymbolNewtonsoft.NewtonsoftRange symbolRange)
SnapshotSpan GetSymbolRangeSpan(Range symbolRange)
{
var originalStartPosition = textSnapshot.GetLineFromLineNumber(symbolRange.Start.Line).Start.Position + symbolRange.Start.Character;
var originalEndPosition = textSnapshot.GetLineFromLineNumber(symbolRange.End.Line).Start.Position + symbolRange.End.Character;
Expand Down
Loading
Loading