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

Create a Razor language server cohosted in Roslyn #70819

Merged
merged 22 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
821420b
Create a Razor language server cohosted in Roslyn
davidwengier Nov 9, 2023
079c504
Allow handlers to operate on TextDocuments
davidwengier Nov 14, 2023
223d01b
Add test to validate that the server has all required services and ha…
davidwengier Nov 15, 2023
342d5de
More realistic test
davidwengier Nov 15, 2023
f1ed068
Add endpoints that need to access Roslyn internals
davidwengier Nov 15, 2023
3c3d285
Allow project context handler to work with any document type
davidwengier Nov 15, 2023
38bffad
Switch to ClientName to prevent activation, and avoid an immediate ye…
davidwengier Nov 24, 2023
1db5ad9
Revert use of Client Name, switch to just reporting no capabilities
davidwengier Dec 4, 2023
464e873
PR feedback
davidwengier Dec 4, 2023
b93538b
Add IRazorLspService in case any lsp services need to be exported fro…
davidwengier Dec 6, 2023
af6b3ab
Add version number to didChange extension point
davidwengier Dec 6, 2023
06f72e9
Add a way for Razor to make calls back to the client
davidwengier Dec 7, 2023
bc1768f
Allow getting services from the RazorRequestContext
davidwengier Dec 7, 2023
8c3c4a9
Put types in their own files
davidwengier Dec 10, 2023
0cc3ac8
Add more data to extension points, and allow async
davidwengier Dec 10, 2023
ad0302f
Add empty base class for service exports, just in case
davidwengier Dec 10, 2023
789c5cc
Make sure we do no work if the Razor cohost server isn't active
davidwengier Dec 11, 2023
da42b86
PR feedback
davidwengier Dec 12, 2023
c537048
Merge remote-tracking branch 'upstream/main' into RazorCohost
davidwengier Dec 12, 2023
28b837f
Update after merge
davidwengier Dec 12, 2023
97689b5
Fix capabilities so didOpen doesn't get sent at all
davidwengier Dec 12, 2023
64fa385
Add exception message
davidwengier Dec 12, 2023
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 @@ -63,7 +63,7 @@ internal abstract partial class AbstractInProcLanguageClient(
/// Unused, implementing <see cref="ILanguageClientCustomMessage2"/>.
/// Gets the optional target object for receiving custom messages not covered by the language server protocol.
/// </summary>
public object? CustomMessageTarget => null;
public virtual object? CustomMessageTarget => null;

/// <summary>
/// An enum representing this server instance.
Expand Down Expand Up @@ -256,5 +256,22 @@ public virtual AbstractLanguageServer<RequestContext> Create(
/// This method is called after the language server has been activated, but connection has not been established.
/// </summary>
public Task AttachForCustomMessageAsync(JsonRpc rpc) => Task.CompletedTask;

internal TestAccessor GetTestAccessor()
{
return new TestAccessor(this);
}

internal readonly struct TestAccessor
{
private readonly AbstractInProcLanguageClient _instance;

internal TestAccessor(AbstractInProcLanguageClient instance)
{
_instance = instance;
}

public AbstractLanguageServer<RequestContext>? LanguageServer => _instance._languageServer;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.FSharp" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.FSharp.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor.UnitTests" />
<!-- Eventually a bunch of these unit tests should move into Roslyn.Services.Implementation.UnitTests
and this should be removed. -->
<RestrictedInternalsVisibleTo Include="Microsoft.CodeAnalysis.TypeScript.EditorFeatures" Key="$(TypeScriptKey)" Partner="VSTypeScript" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
<InternalsVisibleTo Include="Microsoft.VisualStudio.LanguageServices.UnitTests" />
<InternalsVisibleTo Include="Microsoft.VisualStudio.LanguageServices.Test.Utilities2" />
<InternalsVisibleTo Include="IdeBenchmarks" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor.UnitTests"/>
</ItemGroup>
<ItemGroup>
<!-- TODO: Remove the below IVTs to CodeStyle Unit test projects once all analyzer/code fix tests are switched to Microsoft.CodeAnalysis.Testing -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,11 @@ internal TestAccessor(AbstractLanguageServer<TRequestContext> server)
return null;
}

internal Task<TResponse> ExecuteRequestAsync<TRequest, TResponse>(string methodName, TRequest request, CancellationToken cancellationToken)
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
{
return _server._queue.Value.ExecuteAsync<TRequest, TResponse>(request, methodName, _server._lspServices.Value, cancellationToken);
}

internal JsonRpc GetServerRpc() => _server._jsonRpc;

internal bool HasShutdownStarted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.EditorFeatures.Test.Utilities" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.LanguageServer.Protocol.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CommonLanguageServerProtocol.Framework.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor.UnitTests" />
</ItemGroup>
</Project>
15 changes: 15 additions & 0 deletions src/Features/LanguageServer/Protocol/Extensions/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,21 @@ public static ImmutableArray<Document> GetDocuments(this Solution solution, stri
return documents;
}

/// <summary>
/// Get all regular and additional <see cref="TextDocument"/>s for the given <paramref name="documentUri"/>.
/// </summary>
public static ImmutableArray<TextDocument> GetTextDocuments(this Solution solution, Uri documentUri)
{
var documentIds = GetDocumentIds(solution, documentUri);

var documents = documentIds
.Select(solution.GetDocument)
.Concat(documentIds.Select(solution.GetAdditionalDocument))
.WhereNotNull()
.ToImmutableArray();
return documents;
}

public static ImmutableArray<DocumentId> GetDocumentIds(this Solution solution, Uri documentUri)
=> solution.GetDocumentIdsWithFilePath(ProtocolConversions.GetDocumentFilePathFromUri(documentUri));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,20 @@ public GetTextDocumentWithContextHandler()
Contract.ThrowIfNull(context.Workspace);
Contract.ThrowIfNull(context.Solution);

// We specifically don't use context.Document here because we want multiple
var documents = context.Solution.GetDocuments(request.TextDocument.Uri);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing this to call GetTextDocuments seemed overly wasteful given only project info is needed, so I changed this handler a bit. Mainly wanted to avoid calling GetDocument and GetAdditionalDocument on every Id (and this endpoint is called a fair bit)

// We specifically don't use context.Document here because we want multiple. We also don't need
// all of the document info, just the Id is enough
var documentIds = context.Solution.GetDocumentIds(request.TextDocument.Uri);

if (!documents.Any())
if (!documentIds.Any())
{
return SpecializedTasks.Null<VSProjectContextList>();
}

var contexts = new List<VSProjectContext>();

foreach (var document in documents)
foreach (var documentId in documentIds)
{
var project = document.Project;
var project = context.Solution.GetRequiredProject(documentId.ProjectId);
var projectContext = ProtocolConversions.ProjectToProjectContext(project);
contexts.Add(projectContext);
}
Expand All @@ -57,13 +58,13 @@ public GetTextDocumentWithContextHandler()
// GetDocumentIdInCurrentContext will just return the same ID back, which means we're going to pick the first
// ID in GetDocumentIdsWithFilePath, but there's really nothing we can do since we don't have contexts for
// close documents anyways.
var openDocument = documents.First();
var currentContextDocumentId = context.Workspace.GetDocumentIdInCurrentContext(openDocument.Id);
var openDocumentId = documentIds.First();
var currentContextDocumentId = context.Workspace.GetDocumentIdInCurrentContext(openDocumentId);

return Task.FromResult<VSProjectContextList?>(new VSProjectContextList
{
ProjectContexts = contexts.ToArray(),
DefaultIndex = documents.IndexOf(d => d.Id == currentContextDocumentId)
DefaultIndex = documentIds.IndexOf(d => d == currentContextDocumentId)
});
}
}
Expand Down
45 changes: 41 additions & 4 deletions src/Features/LanguageServer/Protocol/Handler/RequestContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ internal readonly struct RequestContext
/// <remarks>
/// This field is only initialized for handlers that request solution context.
/// </remarks>
private readonly StrongBox<(Workspace Workspace, Solution Solution, Document? Document)>? _lspSolution;
private readonly StrongBox<(Workspace Workspace, Solution Solution, TextDocument? Document)>? _lspSolution;

/// <summary>
/// The workspace this request is for, if applicable. This will be present if <see cref="Document"/> is
Expand Down Expand Up @@ -116,6 +116,43 @@ public Document? Document
throw new InvalidOperationException();
}

if (_lspSolution.Value.Document is null)
{
return null;
}

if (_lspSolution.Value.Document is Document document)
{
return document;
}

// Explicitly throw for attempts to get a Document when only a TextDocument is available.
throw new InvalidOperationException($"Attempted to retrieve a Document but a TextDocument was found instead.");
}
}

/// <summary>
/// The text document that the request is for, if applicable. This comes from the <see cref="TextDocumentIdentifier"/> returned from the handler itself via a call to
/// <see cref="ITextDocumentIdentifierHandler{RequestType, TextDocumentIdentifierType}.GetTextDocumentIdentifier(RequestType)"/>.
/// </summary>
public TextDocument? TextDocument
{
get
{
if (_lspSolution is null)
{
// This request context never had a solution instance
return null;
}

// The solution is available unless it has been cleared by a call to ClearSolutionContext. Explicitly throw
// for attempts to access this property after it has been manually cleared. Note that we can't rely on
// Document being null for this check, because it is not always provided as part of the solution context.
if (_lspSolution.Value.Workspace is null)
{
throw new InvalidOperationException();
}

return _lspSolution.Value.Document;
}
}
Expand Down Expand Up @@ -149,7 +186,7 @@ public RequestContext(
string method,
ClientCapabilities? clientCapabilities,
WellKnownLspServerKinds serverKind,
Document? document,
TextDocument? document,
IDocumentChangeTracker documentChangeTracker,
ImmutableDictionary<Uri, (SourceText Text, string LanguageId)> trackedDocuments,
ImmutableArray<string> supportedLanguages,
Expand All @@ -159,7 +196,7 @@ public RequestContext(
if (workspace is not null)
{
RoslynDebug.Assert(solution is not null);
_lspSolution = new StrongBox<(Workspace Workspace, Solution Solution, Document? Document)>((workspace, solution, document));
_lspSolution = new StrongBox<(Workspace Workspace, Solution Solution, TextDocument? Document)>((workspace, solution, document));
}
else
{
Expand Down Expand Up @@ -228,7 +265,7 @@ public static async Task<RequestContext> CreateAsync(
{
Workspace? workspace = null;
Solution? solution = null;
Document? document = null;
TextDocument? document = null;
if (textDocument is not null)
{
// we were given a request associated with a document. Find the corresponding roslyn document for this.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
<InternalsVisibleTo Include="IdeBenchmarks" />
<InternalsVisibleTo Include="AnalyzerRunner" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.CompilerDeveloperSDK" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor.UnitTests"/>
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Features/LanguageServer/Protocol/ProtocolConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@ internal class ProtocolConstants
public const string RoslynLspLanguagesContract = "RoslynLspLanguages";

public const string TypeScriptLanguageContract = "TypeScriptLspLanguage";

public const string RazorCohostContract = "RazorLanguageServer";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ namespace Microsoft.CodeAnalysis.LanguageServer;

internal enum WellKnownLspServerKinds
{
/// <summary>
/// Razor LSP server for Razor document requests (.razor and .cshtml files)
/// </summary>
RazorCohostServer,
Copy link
Member

Choose a reason for hiding this comment

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

So trying to understand the steps here

  1. Create this server to handle Razor files (instead of the current Razor server). Migrate Razor support to here.
  2. (maybe) Once the rest of Marco's work goes in, we can unify this Razor server with the RazorCSharp server, and have a single server that handles both
  3. Once dynamic registration goes in we merge this with the normal C# server

Does that sound right? Potentially we can also skip 2) entirely if requests about C# are converted to non-LSP entirely before 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Item 2 hadn't really occurred to me, but I guess we could do that. Without dynamic registration though, we probably can't do that behind a feature flag though, as it would just be controlled by the content type attribute on the server. Interesting idea though.

The rest is correct though. Essentially this lets us work on migration, find bugs, maybe get some benefit, but without being blocked on dynamic registration.


/// <summary>
/// Roslyn LSP server for razor c# requests.
/// </summary>
Expand Down Expand Up @@ -52,6 +57,7 @@ public static string ToUserVisibleString(this WellKnownLspServerKinds server)
{
return server switch
{
WellKnownLspServerKinds.RazorCohostServer => "Razor Cohost Language Server Client",
WellKnownLspServerKinds.RazorLspServer => "Razor C# Language Server Client",
WellKnownLspServerKinds.LiveShareLspServer => "Live Share C#/Visual Basic Language Server Client",
WellKnownLspServerKinds.AlwaysActiveVSLspServer => "Roslyn Language Server Client",
Expand All @@ -69,6 +75,8 @@ public static string ToTelemetryString(this WellKnownLspServerKinds server)
{
return server switch
{
WellKnownLspServerKinds.RazorCohostServer => "RazorCohostLanguageClient",

// Telemetry was previously reported as RazorInProcLanguageClient.GetType().Name
WellKnownLspServerKinds.RazorLspServer => "RazorInProcLanguageClient",

Expand Down Expand Up @@ -96,6 +104,7 @@ public static string GetContractName(this WellKnownLspServerKinds server)
{
return server switch
{
WellKnownLspServerKinds.RazorCohostServer => ProtocolConstants.RazorCohostContract,
WellKnownLspServerKinds.RazorLspServer => ProtocolConstants.RoslynLspLanguagesContract,
WellKnownLspServerKinds.LiveShareLspServer => ProtocolConstants.RoslynLspLanguagesContract,
WellKnownLspServerKinds.AlwaysActiveVSLspServer => ProtocolConstants.RoslynLspLanguagesContract,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public void UpdateTrackedDocument(Uri uri, SourceText newSourceText)
///
/// This is always called serially in the <see cref="RequestExecutionQueue{RequestContextType}"/> when creating the <see cref="RequestContext"/>.
/// </summary>
public async Task<(Workspace?, Solution?, Document?)> GetLspDocumentInfoAsync(TextDocumentIdentifier textDocumentIdentifier, CancellationToken cancellationToken)
public async Task<(Workspace?, Solution?, TextDocument?)> GetLspDocumentInfoAsync(TextDocumentIdentifier textDocumentIdentifier, CancellationToken cancellationToken)
{
// Get the LSP view of all the workspace solutions.
var uri = textDocumentIdentifier.Uri;
Expand All @@ -222,10 +222,10 @@ public void UpdateTrackedDocument(Uri uri, SourceText newSourceText)
// Find the matching document from the LSP solutions.
foreach (var (workspace, lspSolution, isForked) in lspSolutions)
{
var documents = lspSolution.GetDocuments(textDocumentIdentifier.Uri);
var documents = lspSolution.GetTextDocuments(textDocumentIdentifier.Uri);
if (documents.Any())
{
var document = documents.FindDocumentInProjectContext(textDocumentIdentifier, (sln, id) => sln.GetRequiredDocument(id));
var document = documents.FindDocumentInProjectContext(textDocumentIdentifier, (sln, id) => sln.GetRequiredTextDocument(id));

// Record metadata on how we got this document.
var workspaceKind = document.Project.Solution.WorkspaceKind;
Expand Down Expand Up @@ -378,7 +378,7 @@ await workspace.TryOnDocumentOpenedAsync(
/// <summary>
/// Given a set of documents from the workspace current solution, verify that the LSP text is the same as the document contents.
/// </summary>
private async Task<bool> DoesAllTextMatchWorkspaceSolutionAsync(ImmutableDictionary<Uri, ImmutableArray<Document>> documentsInWorkspace, CancellationToken cancellationToken)
private async Task<bool> DoesAllTextMatchWorkspaceSolutionAsync(ImmutableDictionary<Uri, ImmutableArray<TextDocument>> documentsInWorkspace, CancellationToken cancellationToken)
{
foreach (var (uriInWorkspace, documentsForUri) in documentsInWorkspace)
{
Expand All @@ -396,7 +396,7 @@ private async Task<bool> DoesAllTextMatchWorkspaceSolutionAsync(ImmutableDiction
return true;
}

private static async ValueTask<bool> AreChecksumsEqualAsync(Document document, SourceText lspText, CancellationToken cancellationToken)
private static async ValueTask<bool> AreChecksumsEqualAsync(TextDocument document, SourceText lspText, CancellationToken cancellationToken)
{
var documentText = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
if (documentText == lspText)
Expand All @@ -410,12 +410,12 @@ private static async ValueTask<bool> AreChecksumsEqualAsync(Document document, S
/// <summary>
/// Using the workspace's current solutions, find the matching documents in for each URI.
/// </summary>
private static ImmutableDictionary<Uri, ImmutableArray<Document>> GetDocumentsForUris(ImmutableArray<Uri> trackedDocuments, Solution workspaceCurrentSolution)
private static ImmutableDictionary<Uri, ImmutableArray<TextDocument>> GetDocumentsForUris(ImmutableArray<Uri> trackedDocuments, Solution workspaceCurrentSolution)
{
using var _ = PooledDictionary<Uri, ImmutableArray<Document>>.GetInstance(out var documentsInSolution);
using var _ = PooledDictionary<Uri, ImmutableArray<TextDocument>>.GetInstance(out var documentsInSolution);
foreach (var trackedDoc in trackedDocuments)
{
var documents = workspaceCurrentSolution.GetDocuments(trackedDoc);
var documents = workspaceCurrentSolution.GetTextDocuments(trackedDoc);
if (documents.Any())
{
documentsInSolution[trackedDoc] = documents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ private static bool IsWorkspaceRegistered(Workspace workspace, TestLspServer tes
private static async Task<(Workspace? workspace, Document? document)> GetLspWorkspaceAndDocumentAsync(Uri uri, TestLspServer testLspServer)
{
var (workspace, _, document) = await testLspServer.GetManager().GetLspDocumentInfoAsync(CreateTextDocumentIdentifier(uri), CancellationToken.None).ConfigureAwait(false);
return (workspace, document);
return (workspace, document as Document);
}

private static Task<(Workspace?, Solution?)> GetLspHostWorkspaceAndSolutionAsync(TestLspServer testLspServer)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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 Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost;

internal abstract class AbstractRazorCohostDocumentRequestHandler<TRequestType, TResponseType> : AbstractRazorCohostRequestHandler<TRequestType, TResponseType>, ITextDocumentIdentifierHandler<TRequestType, TextDocumentIdentifier?>
{
TextDocumentIdentifier? ITextDocumentIdentifierHandler<TRequestType, TextDocumentIdentifier?>.GetTextDocumentIdentifier(TRequestType request)
{
var razorIdentifier = GetRazorTextDocumentIdentifier(request);
if (razorIdentifier == null)
{
return null;
}

var textDocumentIdentifier = new VSTextDocumentIdentifier
{
Uri = razorIdentifier.Value.Uri,
};

if (razorIdentifier.Value.ProjectContextId != null)
{
textDocumentIdentifier.ProjectContext = new VSProjectContext
{
Id = razorIdentifier.Value.ProjectContextId
};
}

return textDocumentIdentifier;
}

protected abstract RazorTextDocumentIdentifier? GetRazorTextDocumentIdentifier(TRequestType request);
}

/// <summary>
/// Custom type containing information in a <see cref="VSProjectContext"/> to avoid coupling LSP protocol versions.
/// </summary>
internal record struct RazorTextDocumentIdentifier(Uri Uri, string? ProjectContextId);
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.CodeAnalysis.LanguageServer;

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost;

/// <summary>
/// Base class for services that need to live in Razor but be exported using <see cref="ExportRazorStatelessLspServiceAttribute"/>
/// since those services must implement <see cref="ILspService"/> but the Razor code doesn't have IVT to it.
/// </summary>
internal abstract class AbstractRazorLspService : ILspService
{
}
Loading
Loading