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.

Sign up for GitHub

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

Jump to bottom

Switch Roslyn protocol types to System.Text.Json serialization #73207

Merged
merged 5 commits into from
May 15, 2024
Merged

Switch Roslyn protocol types to System.Text.Json serialization #73207

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6046471
Switch Roslyn protocol types to System.Text.Json serialization
dibarbet Apr 24, 2024
b292a51
Switch Roslyn language server to using System.Text.Json
dibarbet Apr 24, 2024
1a4c3f4
Use Newtonsoft for client side serialization in Document Outline as we
dibarbet May 2, 2024
cade388
Cleanup a few more instances of newtonsoft and warnings
dibarbet May 2, 2024
c1fd7b3
Merge branch 'main' into lsp_stj
dibarbet May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
Visual Studio
-->
<PackageVersion Include="Microsoft.VisualStudio.SDK" Version="17.10.234-preview.1" />
<PackageVersion Include="Microsoft.Internal.VisualStudio.Shell.Framework" Version="17.9.35796-preview.1" />
<PackageVersion Include="Microsoft.Internal.VisualStudio.Shell.Framework" Version="17.9.36524" />
<PackageVersion Include="Microsoft.ServiceHub.Client" Version="4.2.1017" />
<PackageVersion Include="Microsoft.VisualStudio.Extensibility" Version="$(MicrosoftVisualStudioExtensibilityVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Extensibility.Sdk" Version="$(MicrosoftVisualStudioExtensibilityVersion)" />
Expand Down Expand Up @@ -99,7 +99,7 @@
<PackageVersion Include="Microsoft.VisualStudio.Language" Version="$(MicrosoftVisualStudioCoreVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Language.NavigateTo.Interfaces" Version="$(MicrosoftVisualStudioCoreVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Language.StandardClassification" Version="$(MicrosoftVisualStudioCoreVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client" Version="17.10.11-preview" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client" Version="17.10.72-preview" />
Copy link
Member Author

@dibarbet dibarbet May 3, 2024 •
edited

Choose a reason for hiding this comment

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

switching to newer version that has APIs using concrete types instead of JToken. Will make the transition to STJ on the client side easier later.

<PackageVersion Include="Microsoft.VisualStudio.RemoteControl" Version="16.3.52" />
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.10.3-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Shell.15.0" Version="17.10.234-preview.1" />
Expand Down Expand Up @@ -138,7 +138,7 @@
<!--
Language Server
-->
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client.Implementation" Version="17.9.25-preview" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client.Implementation" Version="17.10.72-preview" />
<PackageVersion Include="NuGet.ProjectModel" Version="6.8.0-rc.112" />
<PackageVersion Include="Microsoft.TestPlatform.TranslationLayer" Version="$(MicrosoftTestPlatformVersion)" />
<PackageVersion Include="Microsoft.TestPlatform.ObjectModel" Version="$(MicrosoftTestPlatformVersion)" />
Expand Down
3 changes: 3 additions & 0 deletions ...TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.InitializationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServer;
using Microsoft.CodeAnalysis.Options;
using StreamJsonRpc;
using LSP = Roslyn.LanguageServer.Protocol;

namespace Roslyn.Test.Utilities
Expand All @@ -22,10 +23,12 @@ public abstract partial class AbstractLanguageServerProtocolTests
internal LSP.ClientCapabilities ClientCapabilities { get; init; } = new LSP.ClientCapabilities();
internal WellKnownLspServerKinds ServerKind { get; init; } = WellKnownLspServerKinds.AlwaysActiveVSLspServer;
internal Action<IGlobalOptionService>? OptionUpdater { get; init; } = null;
internal bool CallInitialize { get; init; } = true;
internal bool CallInitialized { get; init; } = true;
internal object? ClientTarget { get; init; } = null;
internal string? Locale { get; init; } = null;
internal IEnumerable<DiagnosticAnalyzer>? AdditionalAnalyzers { get; init; } = null;
internal IJsonRpcMessageFormatter? ClientMessageFormatter { get; init; } = null;
Copy link
Member Author

Choose a reason for hiding this comment

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

Allows us to use newtonsoft on the client side in tests for serialization

}
}
}
20 changes: 13 additions & 7 deletions src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,8 @@ internal sealed class TestLspServer : IAsyncDisposable
LSP.ClientCapabilities clientCapabilities,
RoslynLanguageServer target,
Stream clientStream,
object? clientTarget = null)
object? clientTarget = null,
IJsonRpcMessageFormatter? clientMessageFormatter = null)
{
TestWorkspace = testWorkspace;
ClientCapabilities = clientCapabilities;
Expand All @@ -540,7 +541,9 @@ internal sealed class TestLspServer : IAsyncDisposable

LanguageServer = target;

_clientRpc = new JsonRpc(new HeaderDelimitedMessageHandler(clientStream, clientStream, RoslynLanguageServer.CreateJsonMessageFormatter()), clientTarget)
clientMessageFormatter ??= RoslynLanguageServer.CreateJsonMessageFormatter();

_clientRpc = new JsonRpc(new HeaderDelimitedMessageHandler(clientStream, clientStream, clientMessageFormatter), clientTarget)
{
ExceptionStrategy = ExceptionProcessing.ISerializable,
};
Expand All @@ -566,13 +569,16 @@ internal static async Task<TestLspServer> CreateAsync(EditorTestWorkspace testWo
var (clientStream, serverStream) = FullDuplexStream.CreatePair();
var languageServer = CreateLanguageServer(serverStream, serverStream, testWorkspace, initializationOptions.ServerKind, logger);

var server = new TestLspServer(testWorkspace, locations, initializationOptions.ClientCapabilities, languageServer, clientStream, initializationOptions.ClientTarget);
var server = new TestLspServer(testWorkspace, locations, initializationOptions.ClientCapabilities, languageServer, clientStream, initializationOptions.ClientTarget, initializationOptions.ClientMessageFormatter);

await server.ExecuteRequestAsync<LSP.InitializeParams, LSP.InitializeResult>(LSP.Methods.InitializeName, new LSP.InitializeParams
if (initializationOptions.CallInitialize)
{
Capabilities = initializationOptions.ClientCapabilities,
Locale = initializationOptions.Locale,
}, CancellationToken.None);
await server.ExecuteRequestAsync<LSP.InitializeParams, LSP.InitializeResult>(LSP.Methods.InitializeName, new LSP.InitializeParams
{
Capabilities = initializationOptions.ClientCapabilities,
Locale = initializationOptions.Locale,
}, CancellationToken.None);
}

if (initializationOptions.CallInitialized)
{
Expand Down
5 changes: 3 additions & 2 deletions src/Features/LanguageServer/Protocol/Handler/Symbols/RoslynDocumentSymbolParams.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Text.Json.Serialization;
using Roslyn.LanguageServer.Protocol;
using Newtonsoft.Json;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler
{
Expand All @@ -18,7 +18,8 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler
/// </remarks>
internal class RoslynDocumentSymbolParams : DocumentSymbolParams
{
[JsonProperty(PropertyName = "useHierarchicalSymbols", DefaultValueHandling = DefaultValueHandling.Ignore)]
[JsonPropertyName("useHierarchicalSymbols")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public bool UseHierarchicalSymbols { get; set; }
}
}
58 changes: 33 additions & 25 deletions src/VisualStudio/CSharp/Test/DocumentOutline/DocumentOutlineTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Serialization;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Editor.Test;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.LanguageServer.UnitTests;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand All @@ -22,9 +19,7 @@
using Microsoft.VisualStudio.LanguageServer.Client;
using Microsoft.VisualStudio.LanguageServices.DocumentOutline;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Threading;
using Moq;
using Newtonsoft.Json.Linq;
using StreamJsonRpc;
using Xunit.Abstractions;
using static Roslyn.Test.Utilities.AbstractLanguageServerProtocolTests;
using IAsyncDisposable = System.IAsyncDisposable;
Expand All @@ -49,7 +44,7 @@ protected class DocumentOutlineTestMocks : IAsyncDisposable
private readonly IAsyncDisposable _disposable;

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

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

internal IThreadingContext ThreadingContext { get; }

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

var clientCapabilities = new LSP.ClientCapabilities()
var testLspServer = await CreateTestLspServerAsync(workspace, new InitializationOptions
{
TextDocument = new LSP.TextDocumentClientCapabilities()
{
DocumentSymbol = new LSP.DocumentSymbolSetting()
{
HierarchicalDocumentSymbolSupport = true
}
}
};

var testLspServer = await CreateTestLspServerAsync(workspace, new InitializationOptions { ClientCapabilities = clientCapabilities });
// 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 mocks = new DocumentOutlineTestMocks(RequestAsync, threadingContext, workspace, testLspServer);
return mocks;

async Task<ManualInvocationResponse?> RequestAsync(ITextBuffer textBuffer, Func<JToken, bool> capabilitiesFilter, string languageServerName, string method, Func<ITextSnapshot, JToken> parameterFactory, CancellationToken cancellationToken)
async Task<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]?> RequestAsync(Request<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]> request, CancellationToken cancellationToken)
{
var request = parameterFactory(textBuffer.CurrentSnapshot).ToObject<RoslynDocumentSymbolParams>();
var response = await testLspServer.ExecuteRequestAsync<RoslynDocumentSymbolParams, object[]>(method, request!, cancellationToken);
return new ManualInvocationResponse(string.Empty, JToken.FromObject(response!));
var docRequest = (DocumentRequest<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]>)request;
var parameters = docRequest.ParameterFactory(docRequest.TextBuffer.CurrentSnapshot);
var response = await testLspServer.ExecuteRequestAsync<DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbolParams, DocumentSymbolNewtonsoft.NewtonsoftRoslynDocumentSymbol[]>(request.Method, parameters, cancellationToken);

return response;
}
}

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

return await TestLspServer.CreateAsync(workspace, initializationOptions, _logger);
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);

return server;
}

[DataContract]
private class NewtonsoftInitializeParams
{
[DataMember(Name = "capabilities")]
internal object? Capabilities { get; set; }
}
}
}
1 change: 1 addition & 0 deletions src/VisualStudio/Core/Def/DocumentOutline/DocumentOutlineViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Editor.Tagging;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down