Use System.Text.Json for serialization (#10551)

Fixes #10539 This PR switches us to use System.Text.Json for server to client communication, and opts us in to using it for client to server communication too. Needs https://devdiv.visualstudio.com/DevDiv/_git/WebTools/pullrequest/561188 to flow to VS main, then I'll kick off a final integration test run for good measure. Without that PR the Wrap With Tag functionality doesn't work. TODO: - [x] Test VS Code
dotnet · Jul 3, 2024 · 50bc118 · 50bc118
2 parents e7bc602 + 66a18d7
commit 50bc118
Show file tree

Hide file tree

Showing 122 changed files with 673 additions and 1,942 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -6,9 +6,9 @@
   -->
   <PropertyGroup>
     <_MicrosoftWebToolsPackageVersion>17.9.67-preview-0001</_MicrosoftWebToolsPackageVersion>
-    <_MicrosoftVisualStudioShellPackagesVersion>17.10.39563</_MicrosoftVisualStudioShellPackagesVersion>
-    <_MicrosoftVisualStudioPackagesVersion>17.10.151</_MicrosoftVisualStudioPackagesVersion>
-    <_VisualStudioLanguageServerProtocolVersion>17.11.1-preview</_VisualStudioLanguageServerProtocolVersion>
+    <_MicrosoftVisualStudioShellPackagesVersion>17.11.39721</_MicrosoftVisualStudioShellPackagesVersion>
+    <_MicrosoftVisualStudioPackagesVersion>17.11.191</_MicrosoftVisualStudioPackagesVersion>
+    <_VisualStudioLanguageServerProtocolVersion>17.12.1-preview</_VisualStudioLanguageServerProtocolVersion>
     <_MicrosoftExtensionsPackageVersion>8.0.0</_MicrosoftExtensionsPackageVersion>
     <_BasicReferenceAssembliesVersion>1.7.2</_BasicReferenceAssembliesVersion>
     <_BenchmarkDotNetPackageVersion>0.13.5.2136</_BenchmarkDotNetPackageVersion>
@@ -26,7 +26,7 @@
     <PackageVersion Include="BenchmarkDotNet.Diagnostics.Windows" Version="$(_BenchmarkDotNetPackageVersion)" />
     <PackageVersion Include="DiffPlex" Version="1.7.2" />
     <PackageVersion Include="FluentAssertions" Version="6.7.0" />
-    <PackageVersion Include="MessagePack" Version="2.5.108" />
+    <PackageVersion Include="MessagePack" Version="2.5.168" />
     <PackageVersion Include="Microsoft.AspNetCore.App.Runtime.$(NetCoreSDKRuntimeIdentifier)" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Build.Framework" Version="$(_MicrosoftBuildPackageVersion)" />
     <PackageVersion Include="Microsoft.Build.Locator" Version="1.4.1" />
@@ -70,7 +70,7 @@
     <PackageVersion Include="Microsoft.Internal.VisualStudio.Interop" Version="$(_MicrosoftVisualStudioShellPackagesVersion)" />
     <PackageVersion Include="Microsoft.NET.Sdk.Razor" Version="$(MicrosoftNETSdkRazorPackageVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Copilot" Version="0.2.28-beta" />
-    <PackageVersion Include="Microsoft.VisualStudio.ComponentModelHost" Version="17.10.151" />
+    <PackageVersion Include="Microsoft.VisualStudio.ComponentModelHost" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Editor" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Extensibility.Testing.Xunit" Version="$(_MicrosoftVisualStudioExtensibilityTestingVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Extensibility.Testing.SourceGenerator" Version="$(_MicrosoftVisualStudioExtensibilityTestingVersion)" />
@@ -86,14 +86,14 @@
     <PackageVersion Include="Microsoft.VisualStudio.LanguageServices.Implementation.Symbols" Version="$(_MicrosoftVisualStudioLanguageServicesPackageVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.LiveShare" Version="0.3.1074" />
     <PackageVersion Include="Microsoft.VisualStudio.ProjectSystem.SDK" Version="17.7.294-pre" />
-    <PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.10.21" />
+    <PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.11.8" />
     <PackageVersion Include="Microsoft.VisualStudio.Shell.Framework" Version="$(_MicrosoftVisualStudioShellPackagesVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Shell.15.0" Version="$(_MicrosoftVisualStudioShellPackagesVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Telemetry" Version="17.9.102" />
     <PackageVersion Include="Microsoft.VisualStudio.Text.Data" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
     <PackageVersion Include="Microsoft.VisualStudio.Text.Implementation" Version="$(_MicrosoftVisualStudioPackagesVersion)" NoWarn="NU1701" />
     <PackageVersion Include="Microsoft.VisualStudio.Text.Logic" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
-    <PackageVersion Include="Microsoft.VisualStudio.Threading" Version="17.10.41" />
+    <PackageVersion Include="Microsoft.VisualStudio.Threading" Version="17.11.20" />
     <PackageVersion Include="Microsoft.VisualStudio.Validation" Version="17.8.8" />
     <PackageVersion Include="Microsoft.VisualStudio.Web" Version="16.10.0-preview-1-31008-014" />
     <PackageVersion Include="Microsoft.WebTools.Languages.Html" Version="$(_MicrosoftWebToolsPackageVersion)" />
@@ -105,7 +105,7 @@
     <PackageVersion Include="Microsoft.WebTools.Shared" Version="$(_MicrosoftWebToolsPackageVersion)" />
     <PackageVersion Include="Moq" Version="4.16.0" />
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
-    <PackageVersion Include="Nerdbank.Streams" Version="2.10.69" />
+    <PackageVersion Include="Nerdbank.Streams" Version="2.11.74" />
     <PackageVersion Include="NuGet.SolutionRestoreManager.Interop" Version="4.8.0" />
     <PackageVersion Include="Roslyn.Diagnostics.Analyzers" Version="$(_MicrosoftCodeAnalysisAnalyzersPackageVersion)" />
     <PackageVersion Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutablePackageVersion)" />

diff --git a/NuGet.config b/NuGet.config
@@ -84,6 +84,7 @@
       <package pattern="roslyn.diagnostics.analyzers" />
     </packageSource>
     <packageSource key="dotnet-public">
+      <package pattern="azure.core" />
       <package pattern="basic.reference.assemblies" />
       <package pattern="basic.reference.assemblies.*" />
       <package pattern="castle.core" />

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -222,6 +222,11 @@ stages:
           displayName: Restore, Build and Test
           condition: succeeded()
 
+        - publish: artifacts/log/$(_BuildConfig)
+          artifact: $(Agent.Os)_$(Agent.JobName) Attempt $(System.JobAttempt) Logs
+          displayName: Publish Build Artifacts
+          condition: always()
+
         - publish: artifacts/TestResults/$(_BuildConfig)
           artifact: $(Agent.Os)_$(Agent.JobName) Attempt $(System.JobAttempt) TestResults
           displayName: Publish Test Results
@@ -263,6 +268,11 @@ stages:
           displayName: Restore, Build and Test
           condition: succeeded()
 
+        - publish: artifacts/log/$(_BuildConfig)
+          artifact: $(Agent.Os)_$(Agent.JobName) Attempt $(System.JobAttempt) Logs
+          displayName: Publish Build Artifacts
+          condition: always()
+
         - publish: artifacts/TestResults/$(_BuildConfig)/
           artifact: $(Agent.Os)_$(Agent.JobName) Attempt $(System.JobAttempt) TestResults
           displayName: Publish Test Results

diff --git a/...ft.AspNetCore.Razor.Microbenchmarks/Serialization/CompletionListSerializationBenchmark.cs b/...ft.AspNetCore.Razor.Microbenchmarks/Serialization/CompletionListSerializationBenchmark.cs
@@ -2,7 +2,7 @@
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
 using System.IO;
-using System.Text;
+using System.Text.Json;
 using BenchmarkDotNet.Attributes;
 using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Razor.LanguageServer;
@@ -11,15 +11,13 @@
 using Microsoft.CodeAnalysis.Razor.Completion;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
-using Newtonsoft.Json;
 
 namespace Microsoft.AspNetCore.Razor.Microbenchmarks.Serialization;
 
 public class CompletionListSerializationBenchmark
 {
     private readonly byte[] _completionListBuffer;
 
-    private readonly JsonSerializer _serializer;
     private readonly CompletionList _completionList;
 
     public CompletionListSerializationBenchmark()
@@ -29,8 +27,6 @@ public CompletionListSerializationBenchmark()
         var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);
         var tagHelperCompletionProvider = new TagHelperCompletionProvider(completionService, optionsMonitor);
 
-        _serializer = JsonSerializer.Create();
-
         var documentContent = "<";
         var queryIndex = 1;
         _completionList = GenerateCompletionList(documentContent, queryIndex, tagHelperCompletionProvider);
@@ -43,36 +39,32 @@ public void ComponentElement_CompletionList_Serialization_RoundTrip()
         // Serialize back to json.
         MemoryStream originalStream;
         using (originalStream = new MemoryStream())
-        using (var writer = new StreamWriter(originalStream, Encoding.UTF8, bufferSize: 4096))
         {
-            _serializer.Serialize(writer, _completionList);
+            JsonSerializer.Serialize(originalStream, _completionList);
         }
 
         CompletionList deserializedCompletions;
         var stream = new MemoryStream(originalStream.GetBuffer());
         using (stream)
-        using (var reader = new JsonTextReader(new StreamReader(stream)))
         {
-            deserializedCompletions = _serializer.Deserialize<CompletionList>(reader).AssumeNotNull();
+            deserializedCompletions = JsonSerializer.Deserialize<CompletionList>(stream).AssumeNotNull();
         }
     }
 
     [Benchmark(Description = "Component Completion List Serialization")]
     public void ComponentElement_CompletionList_Serialization()
     {
         using var stream = new MemoryStream();
-        using var writer = new StreamWriter(stream, Encoding.UTF8, bufferSize: 4096);
-        _serializer.Serialize(writer, _completionList);
+        JsonSerializer.Serialize(stream, _completionList);
     }
 
     [Benchmark(Description = "Component Completion List Deserialization")]
     public void ComponentElement_CompletionList_Deserialization()
     {
         // Deserialize from json file.
         using var stream = new MemoryStream(_completionListBuffer);
-        using var reader = new JsonTextReader(new StreamReader(stream));
         CompletionList deserializedCompletions;
-        deserializedCompletions = _serializer.Deserialize<CompletionList>(reader).AssumeNotNull();
+        deserializedCompletions = JsonSerializer.Deserialize<CompletionList>(stream).AssumeNotNull();
     }
 
     private CompletionList GenerateCompletionList(string documentContent, int queryIndex, TagHelperCompletionProvider componentCompletionProvider)
@@ -111,8 +103,7 @@ private CompletionList GenerateCompletionList(string documentContent, int queryI
     private byte[] GenerateBuffer(CompletionList completionList)
     {
         using var stream = new MemoryStream();
-        using var writer = new StreamWriter(stream, Encoding.UTF8, bufferSize: 4096);
-        _serializer.Serialize(writer, completionList);
+        JsonSerializer.Serialize(stream, completionList);
         var buffer = stream.GetBuffer();
 
         return buffer;

diff --git a/.../Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/BaseDelegatedCodeActionResolver.cs b/.../Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/BaseDelegatedCodeActionResolver.cs
@@ -12,14 +12,9 @@
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;
 
-internal abstract class BaseDelegatedCodeActionResolver : ICodeActionResolver
+internal abstract class BaseDelegatedCodeActionResolver(IClientConnection clientConnection) : ICodeActionResolver
 {
-    protected readonly IClientConnection ClientConnection;
-
-    public BaseDelegatedCodeActionResolver(IClientConnection clientConnection)
-    {
-        ClientConnection = clientConnection ?? throw new ArgumentNullException(nameof(clientConnection));
-    }
+    protected readonly IClientConnection ClientConnection = clientConnection;
 
     public abstract string Action { get; }
 

diff --git a/...oft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionProvider.cs b/...oft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionProvider.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.Language;
@@ -12,15 +13,14 @@
 using Microsoft.AspNetCore.Razor.Threading;
 using Microsoft.CodeAnalysis.ExternalAccess.Razor;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
-using Newtonsoft.Json.Linq;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;
 
-internal sealed class DefaultCSharpCodeActionProvider : ICSharpCodeActionProvider
+internal sealed class DefaultCSharpCodeActionProvider(LanguageServerFeatureOptions languageServerFeatureOptions) : ICSharpCodeActionProvider
 {
     // Internal for testing
-    internal static readonly HashSet<string> SupportedDefaultCodeActionNames = new HashSet<string>()
-    {
+    internal static readonly HashSet<string> SupportedDefaultCodeActionNames =
+    [
         RazorPredefinedCodeRefactoringProviderNames.GenerateEqualsAndGetHashCodeFromMembers,
         RazorPredefinedCodeRefactoringProviderNames.AddAwait,
         RazorPredefinedCodeRefactoringProviderNames.AddDebuggerDisplay,
@@ -39,39 +39,24 @@ internal sealed class DefaultCSharpCodeActionProvider : ICSharpCodeActionProvide
         RazorPredefinedCodeFixProviderNames.ImplementAbstractClass,
         RazorPredefinedCodeFixProviderNames.ImplementInterface,
         RazorPredefinedCodeFixProviderNames.RemoveUnusedVariable,
-    };
+    ];
 
     // We don't support any code actions in implicit expressions at the moment, but rather than simply returning early
     // I thought it best to create an allow list, empty, so that we can easily add them later if we identify any big
     // hitters that we want to enable.
     // The one example commented out here should not be taken as an opinion as to what that allow list should look like.
-    internal static readonly HashSet<string> SupportedImplicitExpressionCodeActionNames = new HashSet<string>()
-    {
+    internal static readonly HashSet<string> SupportedImplicitExpressionCodeActionNames =
+    [
         // RazorPredefinedCodeFixProviderNames.RemoveUnusedVariable,
-    };
-
-    private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
+    ];
 
-    public DefaultCSharpCodeActionProvider(LanguageServerFeatureOptions languageServerFeatureOptions)
-    {
-        _languageServerFeatureOptions = languageServerFeatureOptions;
-    }
+    private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions;
 
     public Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(
         RazorCodeActionContext context,
         IEnumerable<RazorVSInternalCodeAction> codeActions,
         CancellationToken cancellationToken)
     {
-        if (context is null)
-        {
-            throw new ArgumentNullException(nameof(context));
-        }
-
-        if (codeActions is null)
-        {
-            throw new ArgumentNullException(nameof(codeActions));
-        }
-
         // Used to identify if this is VSCode which doesn't support
         // code action resolve.
         if (!context.SupportsCodeActionResolve)
@@ -93,24 +78,14 @@ public DefaultCSharpCodeActionProvider(LanguageServerFeatureOptions languageServ
         {
             var isOnAllowList = codeAction.Name is not null && allowList.Contains(codeAction.Name);
 
-            if (_languageServerFeatureOptions.ShowAllCSharpCodeActions && codeAction.Data is not null)
+            // If this code action isn't on the allow list, it might have been handled by another provider, which means
+            // it will already have been wrapped, so we have to check not to double-wrap it.
+            if (_languageServerFeatureOptions.ShowAllCSharpCodeActions &&
+                CanDeserializeTo<RazorCodeActionResolutionParams>(codeAction.Data))
             {
-                // If this code action isn't on the allow list, it might have been handled by another provider, which means
-                // it will already have been wrapped, so we have to check not to double-wrap it. Unfortunately there isn't a
-                // good way to do this, but to try and deserialize some Json. Since this only needs to happen if the feature
-                // flag is on, any perf hit here isn't going to affect real users.
-                try
-                {
-                    if (((JToken)codeAction.Data).ToObject<RazorCodeActionResolutionParams>() is not null)
-                    {
-                        // This code action has already been wrapped by something else, so skip it here, or it could
-                        // be marked as experimental when its not, and more importantly would be duplicated in the list.
-                        continue;
-                    }
-                }
-                catch
-                {
-                }
+                // This code action has already been wrapped by something else, so skip it here, or it could
+                // be marked as experimental when its not, and more importantly would be duplicated in the list.
+                continue;
             }
 
             if (_languageServerFeatureOptions.ShowAllCSharpCodeActions || isOnAllowList)
@@ -120,5 +95,23 @@ public DefaultCSharpCodeActionProvider(LanguageServerFeatureOptions languageServ
         }
 
         return Task.FromResult<IReadOnlyList<RazorVSInternalCodeAction>?>(results);
+
+        static bool CanDeserializeTo<T>(object? data)
+        {
+            // We don't care about errors here, and there is no TryDeserialize method, so we can just brute force this.
+            // Since this only happens if the feature flag is on, which is internal only and intended only for users of
+            // this repo, any perf hit here isn't going to affect real users.
+            try
+            {
+                if (data is JsonElement element &&
+                    element.Deserialize<RazorCodeActionResolutionParams>() is not null)
+                {
+                    return true;
+                }
+            }
+            catch { }
+
+            return false;
+        }
     }
 }
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs
@@ -6,6 +6,7 @@
 using System.Collections.Immutable;
 using System.Linq;
 using System.Reflection;
+using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.Language;
@@ -22,7 +23,6 @@
 using Microsoft.CodeAnalysis.Razor.Protocol.CodeActions;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
-using Newtonsoft.Json.Linq;
 using StreamJsonRpc;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;
@@ -215,9 +215,9 @@ private RazorVSInternalCodeAction[] ExtractCSharpCodeActionNamesFromData(RazorVS
 
         foreach (var codeAction in codeActions)
         {
-            // Note: we may see a perf benefit from using a JsonConverter
-            var tags = ((JToken?)codeAction.Data)?["CustomTags"]?.ToObject<string[]>();
-            if (tags is null || tags.Length == 0)
+            if (codeAction.Data is not JsonElement jsonData ||
+                !jsonData.TryGetProperty("CustomTags", out var value) ||
+                value.Deserialize<string[]>() is not [..] tags)
             {
                 continue;
             }