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

Return the SignatureHelp items nearest to the cursor #73606

Merged
merged 6 commits into from
May 24, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.SignatureHelp;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Text;
using Roslyn.Utilities;

Expand Down Expand Up @@ -87,9 +85,13 @@ private async Task<Model> ComputeModelInBackgroundAsync(
var options = Controller.GlobalOptions.GetSignatureHelpOptions(document.Project.Language);

// first try to query the providers that can trigger on the specified character
var (provider, items) = await ComputeItemsAsync(
providers, caretPosition, triggerInfo,
options, document, cancellationToken).ConfigureAwait(false);
var (provider, items) = await SignatureHelpService.GetSignatureHelpAsync(
providers,
document,
caretPosition,
triggerInfo,
options,
cancellationToken).ConfigureAwait(false);

if (provider == null)
{
Expand Down Expand Up @@ -174,66 +176,6 @@ private static bool DisplayPartsMatch(SignatureHelpItem i1, SignatureHelpItem i2

private static bool CompareParts(TaggedText p1, TaggedText p2)
=> p1.ToString() == p2.ToString();

private static async Task<(ISignatureHelpProvider provider, SignatureHelpItems items)> ComputeItemsAsync(
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved into the SignatureHelpService.

Copy link
Member

Choose a reason for hiding this comment

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

i'm assuming it didn't change (beyond anything mechanical). if it did, please call that out.

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 will do a better job of this going forward. Added comments below.

ImmutableArray<ISignatureHelpProvider> providers,
SnapshotPoint caretPosition,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
Document document,
CancellationToken cancellationToken)
{
try
{
ISignatureHelpProvider bestProvider = null;
SignatureHelpItems bestItems = null;

// TODO(cyrusn): We're calling into extensions, we need to make ourselves resilient
// to the extension crashing.
foreach (var provider in providers)
{
cancellationToken.ThrowIfCancellationRequested();

var currentItems = await provider.GetItemsAsync(document, caretPosition, triggerInfo, options, cancellationToken).ConfigureAwait(false);
if (currentItems != null && currentItems.ApplicableSpan.IntersectsWith(caretPosition.Position))
{
// If another provider provides sig help items, then only take them if they
// start after the last batch of items. i.e. we want the set of items that
// conceptually are closer to where the caret position is. This way if you have:
//
// Goo(new Bar($$
//
// Then invoking sig help will only show the items for "new Bar(" and not also
// the items for "Goo(..."
if (IsBetter(bestItems, currentItems.ApplicableSpan))
{
bestItems = currentItems;
bestProvider = provider;
}
}
}

return (bestProvider, bestItems);
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken, ErrorSeverity.Critical))
{
return (null, null);
}
}

private static bool IsBetter(SignatureHelpItems bestItems, TextSpan? currentTextSpan)
{
// If we have no best text span, then this span is definitely better.
if (bestItems == null)
{
return true;
}

// Otherwise we want the one that is conceptually the innermost signature. So it's
// only better if the distance from it to the caret position is less than the best
// one so far.
return currentTextSpan.Value.Start > bestItems.ApplicableSpan.Start;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
using System.Linq;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.SignatureHelp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ Imports Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense
Imports Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.SignatureHelp
Imports Microsoft.CodeAnalysis.Editor.Shared.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Shared.TestHooks
Imports Microsoft.CodeAnalysis.SignatureHelp
Imports Microsoft.CodeAnalysis.Text
Expand Down Expand Up @@ -113,7 +112,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
Dim mre = New ManualResetEvent(False)
Dim controller = CreateController(CreateWorkspace(), items:=CreateItems(2), waitForPresentation:=False)
Dim slowProvider = New Mock(Of ISignatureHelpProvider)(MockBehavior.Strict)
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), Options, It.IsAny(Of CancellationToken))) _
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Function()
mre.WaitOne()
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing))
Expand Down
108 changes: 108 additions & 0 deletions src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// 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 System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Extensions;
using Microsoft.CodeAnalysis.Host.Mef;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.SignatureHelp;

/// <summary>
/// A service that is used to determine the appropriate signature help for a position in a document.
/// </summary>
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
[Export(typeof(SignatureHelpService)), Shared]
internal sealed class SignatureHelpService([ImportMany] IEnumerable<Lazy<ISignatureHelpProvider, OrderableLanguageMetadata>> allProviders)
{
private readonly ConcurrentDictionary<string, ImmutableArray<ISignatureHelpProvider>> _providersByLanguage = [];
private readonly IEnumerable<Lazy<ISignatureHelpProvider, OrderableLanguageMetadata>> _allProviders = allProviders;

private ImmutableArray<ISignatureHelpProvider> GetProviders(string language)
{
return _providersByLanguage.GetOrAdd(language, language =>
_allProviders
.Where(p => p.Metadata.Language == language)
.SelectAsArray(p => p.Value));
}

/// <summary>
/// Gets the <see cref="ISignatureHelpProvider"/> and <see cref="SignatureHelpItems"/> associated with
/// the position in the document.
/// </summary>
public Task<(ISignatureHelpProvider? provider, SignatureHelpItems? bestItems)> GetSignatureHelpAsync(
Document document,
int position,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
CancellationToken cancellationToken)
{
return GetSignatureHelpAsync(
GetProviders(document.Project.Language),
document,
position,
triggerInfo,
options,
cancellationToken);
}

/// <summary>
/// Gets the <see cref="ISignatureHelpProvider"/> and <see cref="SignatureHelpItems"/> associated with
/// the position in the document.
/// </summary>
public static async Task<(ISignatureHelpProvider? provider, SignatureHelpItems? bestItems)> GetSignatureHelpAsync(
ImmutableArray<ISignatureHelpProvider> providers,
Document document,
int position,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
CancellationToken cancellationToken)
{
var extensionManager = document.Project.Solution.Services.GetRequiredService<IExtensionManager>();

ISignatureHelpProvider? bestProvider = null;
SignatureHelpItems? bestItems = null;
Copy link
Member

Choose a reason for hiding this comment

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

i'm assuming this was a move, and didn't change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah I meant more generally the logic moved but not verbatim. On a suggestion from DavidB it uses the extension manager when invoking providers as well as inlines the IsBetter method.


// returns the first non-empty quick info found (based on provider order)
foreach (var provider in providers)
{
var items = await extensionManager.PerformFunctionAsync(
provider,
cancellationToken => provider.GetItemsAsync(document, position, triggerInfo, options, cancellationToken),
defaultValue: null,
cancellationToken).ConfigureAwait(false);

if (items is null || !items.ApplicableSpan.IntersectsWith(position))
{
continue;
}

// If another provider provides sig help items, then only take them if they
// start after the last batch of items. i.e. we want the set of items that
// conceptually are closer to where the caret position is. This way if you have:
//
// Goo(new Bar($$
//
// Then invoking sig help will only show the items for "new Bar(" and not also
// the items for "Goo(..."
if (bestItems is not null && items.ApplicableSpan.Start < bestItems.ApplicableSpan.Start)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the moved IsBetter method logic inlined.

{
continue;
}

bestProvider = provider;
bestItems = items;
}

return (bestProvider, bestItems);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Composition;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.SignatureHelp;
using Microsoft.CodeAnalysis.Options;
using Roslyn.Text.Adornments;
using LSP = Roslyn.LanguageServer.Protocol;

Expand All @@ -22,16 +21,16 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler
[Method(LSP.Methods.TextDocumentSignatureHelpName)]
internal class SignatureHelpHandler : ILspServiceDocumentRequestHandler<LSP.TextDocumentPositionParams, LSP.SignatureHelp?>
{
private readonly IEnumerable<Lazy<ISignatureHelpProvider, OrderableLanguageMetadata>> _allProviders;
private readonly SignatureHelpService _signatureHelpService;
private readonly IGlobalOptionService _globalOptions;

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public SignatureHelpHandler(
[ImportMany] IEnumerable<Lazy<ISignatureHelpProvider, OrderableLanguageMetadata>> allProviders,
SignatureHelpService signatureHelpService,
IGlobalOptionService globalOptions)
{
_allProviders = allProviders;
_signatureHelpService = signatureHelpService;
_globalOptions = globalOptions;
}

Expand All @@ -48,56 +47,50 @@ public SignatureHelpHandler(
return null;

var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false);

var providers = _allProviders.Where(p => p.Metadata.Language == document.Project.Language);
var triggerInfo = new SignatureHelpTriggerInfo(SignatureHelpTriggerReason.InvokeSignatureHelpCommand);
var options = _globalOptions.GetSignatureHelpOptions(document.Project.Language);

foreach (var provider in providers)
var (_, sigItems) = await _signatureHelpService.GetSignatureHelpAsync(document, position, triggerInfo, options, cancellationToken).ConfigureAwait(false);
if (sigItems is null)
{
var items = await provider.Value.GetItemsAsync(document, position, triggerInfo, options, cancellationToken).ConfigureAwait(false);
return null;
}

if (items != null)
{
var sigInfos = new ArrayBuilder<LSP.SignatureInformation>();
using var _ = ArrayBuilder<LSP.SignatureInformation>.GetInstance(out var sigInfos);

foreach (var item in items.Items)
{
LSP.SignatureInformation sigInfo;
if (clientCapabilities.HasVisualStudioLspCapability() == true)
{
sigInfo = new LSP.VSInternalSignatureInformation
{
ColorizedLabel = GetSignatureClassifiedText(item)
};
}
else
{
sigInfo = new LSP.SignatureInformation();
}

sigInfo.Label = GetSignatureText(item);
sigInfo.Documentation = new LSP.MarkupContent { Kind = LSP.MarkupKind.PlainText, Value = item.DocumentationFactory(cancellationToken).GetFullText() };
sigInfo.Parameters = item.Parameters.Select(p => new LSP.ParameterInformation
{
Label = p.Name,
Documentation = new LSP.MarkupContent { Kind = LSP.MarkupKind.PlainText, Value = p.DocumentationFactory(cancellationToken).GetFullText() }
}).ToArray();
sigInfos.Add(sigInfo);
}

var sigHelp = new LSP.SignatureHelp
foreach (var item in sigItems.Items)
{
LSP.SignatureInformation sigInfo;
if (clientCapabilities.HasVisualStudioLspCapability() == true)
{
sigInfo = new LSP.VSInternalSignatureInformation
{
ActiveSignature = GetActiveSignature(items),
ActiveParameter = items.ArgumentIndex,
Signatures = sigInfos.ToArrayAndFree()
ColorizedLabel = GetSignatureClassifiedText(item)
};

return sigHelp;
}
else
{
sigInfo = new LSP.SignatureInformation();
}

sigInfo.Label = GetSignatureText(item);
sigInfo.Documentation = new LSP.MarkupContent { Kind = LSP.MarkupKind.PlainText, Value = item.DocumentationFactory(cancellationToken).GetFullText() };
sigInfo.Parameters = item.Parameters.Select(p => new LSP.ParameterInformation
{
Label = p.Name,
Documentation = new LSP.MarkupContent { Kind = LSP.MarkupKind.PlainText, Value = p.DocumentationFactory(cancellationToken).GetFullText() }
}).ToArray();
sigInfos.Add(sigInfo);
}

return null;
var sigHelp = new LSP.SignatureHelp
{
ActiveSignature = GetActiveSignature(sigItems),
ActiveParameter = sigItems.ArgumentIndex,
Signatures = sigInfos.ToArray()
};

return sigHelp;
}

private static int GetActiveSignature(SignatureHelpItems items)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,29 @@ int M2(string a)
AssertJsonEquals(expected, results);
}

[Theory, CombinatorialData]
public async Task TestGetNestedSignatureHelpAsync(bool mutatingLspWorkspace)
{
var markup =
@"class Foo {
public Foo(int showMe) {}

public static void Do(Foo foo) {
Do(new Foo({|caret:|}
}
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
var expected = new LSP.SignatureHelp()
{
ActiveParameter = 0,
ActiveSignature = 0,
Signatures = [CreateSignatureInformation("Foo(int showMe)", "", "showMe", "")]
};

var results = await RunGetSignatureHelpAsync(testLspServer, testLspServer.GetLocations("caret").Single());
AssertJsonEquals(expected, results);
}

private static async Task<LSP.SignatureHelp?> RunGetSignatureHelpAsync(TestLspServer testLspServer, LSP.Location caret)
{
return await testLspServer.ExecuteRequestAsync<LSP.TextDocumentPositionParams, LSP.SignatureHelp?>(
Expand Down
Loading