Skip to content

Expose Go To Impl and Spell Check to Razor #74978

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions src/LanguageServer/Protocol/Extensions/ProtocolConversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using Microsoft.CodeAnalysis.NavigateTo;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.SpellCheck;
using Microsoft.CodeAnalysis.Tags;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Text.Adornments;
Expand Down Expand Up @@ -582,6 +583,15 @@ public static LSP.DocumentHighlightKind HighlightSpanKindToDocumentHighlightKind
}
}

public static LSP.VSInternalSpellCheckableRangeKind SpellCheckSpanKindToSpellCheckableRangeKind(SpellCheckKind kind)
=> kind switch
{
SpellCheckKind.Identifier => LSP.VSInternalSpellCheckableRangeKind.Identifier,
SpellCheckKind.Comment => LSP.VSInternalSpellCheckableRangeKind.Comment,
SpellCheckKind.String => LSP.VSInternalSpellCheckableRangeKind.String,
_ => throw ExceptionUtilities.UnexpectedValue(kind),
};

public static Glyph SymbolKindToGlyph(LSP.SymbolKind kind)
{
switch (kind)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using LSP = Roslyn.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler
Expand All @@ -33,26 +34,32 @@ public FindImplementationsHandler(IGlobalOptionService globalOptions)

public LSP.TextDocumentIdentifier GetTextDocumentIdentifier(LSP.TextDocumentPositionParams request) => request.TextDocument;

public async Task<LSP.Location[]> HandleRequestAsync(LSP.TextDocumentPositionParams request, RequestContext context, CancellationToken cancellationToken)
public Task<LSP.Location[]> HandleRequestAsync(LSP.TextDocumentPositionParams request, RequestContext context, CancellationToken cancellationToken)
Copy link
Member

@maryamariyan maryamariyan Sep 3, 2024

Choose a reason for hiding this comment

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

assuming you intentionally dropped async here

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed it would be a compiler warning if I left it in place.

If an async method only has a single await at the end, it is usually better to remove it, and the async modifier, and just return the Task directly. This avoids the creation of a state machine and so gives a slight perf bump. It does however change the call stack for exceptions that might be thrown so I wouldn't say its a hard and fast rule.

{
var document = context.GetRequiredDocument();
var clientCapabilities = context.GetRequiredClientCapabilities();
var supportsVisualStudioExtensions = context.GetRequiredClientCapabilities().HasVisualStudioLspCapability();
var linePosition = ProtocolConversions.PositionToLinePosition(request.Position);
var classificationOptions = _globalOptions.GetClassificationOptionsProvider();

return FindImplementationsAsync(document, linePosition, classificationOptions, supportsVisualStudioExtensions, cancellationToken);
}

internal static async Task<LSP.Location[]> FindImplementationsAsync(Document document, LinePosition linePosition, OptionsProvider<ClassificationOptions> classificationOptions, bool supportsVisualStudioExtensions, CancellationToken cancellationToken)
{
var locations = ArrayBuilder<LSP.Location>.GetInstance();

var findUsagesService = document.GetRequiredLanguageService<IFindUsagesLSPService>();
var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false);
var position = await document.GetPositionFromLinePositionAsync(linePosition, cancellationToken).ConfigureAwait(false);

var findUsagesContext = new SimpleFindUsagesContext();
var classificationOptions = _globalOptions.GetClassificationOptionsProvider();
await findUsagesService.FindImplementationsAsync(findUsagesContext, document, position, classificationOptions, cancellationToken).ConfigureAwait(false);

foreach (var definition in findUsagesContext.GetDefinitions())
{
var text = definition.GetClassifiedText();
foreach (var sourceSpan in definition.SourceSpans)
{
if (clientCapabilities.HasVisualStudioLspCapability() == true)
if (supportsVisualStudioExtensions)
{
locations.AddIfNotNull(await ProtocolConversions.DocumentSpanToLocationWithTextAsync(sourceSpan, text, cancellationToken).ConfigureAwait(false));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ private async IAsyncEnumerable<TReport> ComputeAndReportCurrentSpansAsync(
{
var textDocumentIdentifier = ProtocolConversions.DocumentToTextDocumentIdentifier(document);

var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
var spans = await service.GetSpansAsync(document, cancellationToken).ConfigureAwait(false);

// protocol requires the results be in sorted order
Expand Down Expand Up @@ -186,13 +185,7 @@ private async IAsyncEnumerable<TReport> ComputeAndReportCurrentSpansAsync(
{
var span = spans[i];

var kind = span.Kind switch
{
SpellCheckKind.Identifier => VSInternalSpellCheckableRangeKind.Identifier,
SpellCheckKind.Comment => VSInternalSpellCheckableRangeKind.Comment,
SpellCheckKind.String => VSInternalSpellCheckableRangeKind.String,
_ => throw ExceptionUtilities.UnexpectedValue(span.Kind),
};
var kind = ProtocolConversions.SpellCheckSpanKindToSpellCheckableRangeKind(span.Kind);

triples[triplesIndex++] = (int)kind;
triples[triplesIndex++] = span.TextSpan.Start - lastSpanEnd;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// 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.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.LanguageServer;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.SpellCheck;
using Roslyn.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers;

internal static class SpellCheck
{
public readonly record struct SpellCheckSpan(int StartIndex, int Length, VSInternalSpellCheckableRangeKind Kind);

public static async Task<ImmutableArray<SpellCheckSpan>> GetSpellCheckSpansAsync(Document document, CancellationToken cancellationToken)
Copy link
Member

Choose a reason for hiding this comment

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

would it be helpful to include try catch block in the GetSpellCheckSpansAsync method here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think for our purposes we're better off letting the exception bubble out of the ExternalAccess layer here, and if necessary then we can handle it in Razor. If we handled the exceptions here it would go through Roslyn's exception reporting and telemetry, which might make it harder to get in front of the right team.

{
var service = document.GetLanguageService<ISpellCheckSpanService>();
if (service is null)
{
return [];
}

var spans = await service.GetSpansAsync(document, cancellationToken).ConfigureAwait(false);

using var _ = ArrayBuilder<SpellCheckSpan>.GetInstance(spans.Length, out var razorSpans);
foreach (var span in spans)
{
var kind = ProtocolConversions.SpellCheckSpanKindToSpellCheckableRangeKind(span.Kind);
razorSpans.Add(new SpellCheckSpan(span.TextSpan.Start, span.TextSpan.Length, kind));
}

return razorSpans.ToImmutable();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers;

using Location = Roslyn.LanguageServer.Protocol.Location;

internal static class GoToImplementation
{
public static Task<Location[]> FindImplementationsAsync(Document document, LinePosition linePosition, bool supportsVisualStudioExtensions, CancellationToken cancellationToken)
{
var globalOptions = document.Project.Solution.Services.ExportProvider.GetService<IGlobalOptionService>();
var classificationOptions = globalOptions.GetClassificationOptionsProvider();

return FindImplementationsHandler.FindImplementationsAsync(document, linePosition, classificationOptions, supportsVisualStudioExtensions, cancellationToken);
}
}
Loading