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

Use System.Text.Json for serialization #10551

Merged
merged 26 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5da723f
Progress commit
davidwengier May 30, 2024
e70d1b9
All tests passing
davidwengier May 31, 2024
a1656ba
Remove some random Newtonsoft.Json references, and get tests passing …
davidwengier May 31, 2024
c4244d0
Remove some old serialization attributes
davidwengier May 31, 2024
191adb6
Found a sneaky params object
davidwengier May 31, 2024
069d5a7
Fix generated document communications
davidwengier Jun 3, 2024
20da0bd
Allow code actions to talk Newtonsoft as well, since thats what Rosly…
davidwengier Jun 4, 2024
b1f3906
Fix inlay hints
davidwengier Jun 5, 2024
7fbee1e
Fixes
davidwengier Jun 17, 2024
39a19a7
Fix build
davidwengier Jun 17, 2024
0f9263f
Add dual attributes so we can freely communicate with WebTools
davidwengier Jun 27, 2024
ea2cda8
Missed a TODO
davidwengier Jun 27, 2024
d17b4ff
Keep RazorTextChange shape in line with TextChange, and ServerTextCha…
davidwengier Jun 28, 2024
9d0f82a
PR feedback (and a little logging tweak)
davidwengier Jun 28, 2024
ec197b7
Merge remote-tracking branch 'upstream/main' into dev/dawengie/STJ
davidwengier Jun 30, 2024
36f28be
Cleanup
davidwengier Jul 2, 2024
efa67a4
Cleanup
davidwengier Jul 2, 2024
44688f6
Merge remote-tracking branch 'upstream/main' into dev/dawengie/STJ
davidwengier Jul 2, 2024
5981abd
Update all the packages!!
davidwengier Jul 2, 2024
ca2bba2
Nullability
davidwengier Jul 2, 2024
fb2ab65
Explicit null check because the VSSDK analyzer is fussy
davidwengier Jul 3, 2024
d70c96c
Merge remote-tracking branch 'upstream/main' into dev/dawengie/STJ
davidwengier Jul 3, 2024
fd6d6cc
Try bump some more packages
davidwengier Jul 3, 2024
db9efe4
Try publish binlogs from mac and linux
davidwengier Jul 3, 2024
c61b0d5
More slightly educated guesses at package versioning
davidwengier Jul 3, 2024
66a18d7
Add package source mapping
davidwengier Jul 3, 2024
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
16 changes: 8 additions & 8 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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" />
Expand Down Expand Up @@ -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)" />
Expand All @@ -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)" />
Expand All @@ -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)" />
Expand Down
1 change: 1 addition & 0 deletions NuGet.config
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
10 changes: 10 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
Expand All @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
{
continue;
}
Expand Down
Loading