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 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
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,14 @@ internal partial class Session
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 service = document.GetRequiredLanguageService<SignatureHelpService>();
var (provider, items) = await service.GetSignatureHelpAsync(
providers,
document,
caretPosition,
triggerInfo,
options,
cancellationToken).ConfigureAwait(false);

if (provider == null)
{
Expand Down Expand Up @@ -174,66 +177,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
@@ -0,0 +1,32 @@
// 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.Composition;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.SignatureHelp;

namespace Microsoft.CodeAnalysis.CSharp.SignatureHelp;

[ExportLanguageServiceFactory(typeof(SignatureHelpService), LanguageNames.CSharp), Shared]
internal class CSharpSignatureHelpServiceFactory : ILanguageServiceFactory
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpSignatureHelpServiceFactory()
{
}
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved

public ILanguageService CreateLanguageService(HostLanguageServices languageServices)
=> new CSharpSignatureHelpService(languageServices.LanguageServices);
}

internal class CSharpSignatureHelpService : SignatureHelpServiceWithProviders
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
{
internal CSharpSignatureHelpService(LanguageServices services)
: base(services)
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
{
}
}
103 changes: 103 additions & 0 deletions src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// 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.Host;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.SignatureHelp;

/// <summary>
/// A service that is used to determine the appropriate signature help for a position in a document.
/// </summary>
internal abstract class SignatureHelpService : ILanguageService
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Gets the appropriate <see cref="SignatureHelpService"/> for the specified document.
/// </summary>
public static SignatureHelpService? GetService(Document? document)
=> document?.GetLanguageService<SignatureHelpService>();
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets the <see cref="ISignatureHelpProvider"/> and <see cref="SignatureHelpItems"/> associated with
/// the position in the document.
/// </summary>
public abstract Task<(ISignatureHelpProvider? provider, SignatureHelpItems? bestItems)> GetSignatureHelpAsync(
Document document,
int position,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
CancellationToken cancellationToken = default);
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets the <see cref="ISignatureHelpProvider"/> and <see cref="SignatureHelpItems"/> associated with
/// the position in the document.
/// </summary>
public async Task<(ISignatureHelpProvider? provider, SignatureHelpItems? bestItems)> GetSignatureHelpAsync(

Check failure on line 39 in src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs#L39

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs(39,90): error CA1822: (NETCORE_ENGINEERING_TELEMETRY=Build) Member 'GetSignatureHelpAsync' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
ImmutableArray<ISignatureHelpProvider> providers,
Document document,
int position,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
CancellationToken cancellationToken)
{
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 TryGetItemsAsync(document, position, triggerInfo, options, provider, cancellationToken).ConfigureAwait(false);
if (items is null)
{
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);
}

private static async Task<SignatureHelpItems?> TryGetItemsAsync(Document document, int position, SignatureHelpTriggerInfo triggerInfo, SignatureHelpOptions options, ISignatureHelpProvider provider, CancellationToken cancellationToken)
{
// We're calling into extensions, we need to make ourselves resilient
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
// to the extension crashing.
try
{
var items = await provider.GetItemsAsync(document, position, triggerInfo, options, cancellationToken).ConfigureAwait(false);
if (items is null)
{
return null;
}

if (!items.ApplicableSpan.IntersectsWith(position))
{
return null;
}

return items;
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken, ErrorSeverity.Critical))

Check failure on line 98 in src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs#L98

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs(98,16): error CS0246: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'Exception' could not be found (are you missing a using directive or an assembly reference?)

Check failure on line 98 in src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs#L98

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs(98,35): error CS0103: (NETCORE_ENGINEERING_TELEMETRY=Build) The name 'FatalError' does not exist in the current context

Check failure on line 98 in src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs#L98

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs(98,97): error CS0103: (NETCORE_ENGINEERING_TELEMETRY=Build) The name 'ErrorSeverity' does not exist in the current context

Check failure on line 98 in src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs#L98

src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs(98,16): error CS0246: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'Exception' could not be found (are you missing a using directive or an assembly reference?)
{
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// 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.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.SignatureHelp;

/// <summary>
/// Base class for <see cref="SignatureHelpService"/>'s that delegate to <see cref="ISignatureHelpProvider"/>'s.
/// </summary>
internal abstract class SignatureHelpServiceWithProviders : SignatureHelpService
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly LanguageServices _services;
private ImmutableArray<ISignatureHelpProvider> _providers;

protected SignatureHelpServiceWithProviders(LanguageServices services)
{
_services = services;
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
}

private ImmutableArray<ISignatureHelpProvider> GetProviders()
{
if (_providers.IsDefault)
{
var mefExporter = _services.SolutionServices.ExportProvider;

var providers = ExtensionOrderer
.Order(mefExporter.GetExports<ISignatureHelpProvider, OrderableLanguageMetadata>()
.Where(lz => lz.Metadata.Language == _services.Language))
.Select(lz => lz.Value)
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
.ToImmutableArray();

ImmutableInterlocked.InterlockedCompareExchange(ref _providers, providers, default);
}

return _providers;
}

public override Task<(ISignatureHelpProvider? provider, SignatureHelpItems? bestItems)> GetSignatureHelpAsync(
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
Document document,
int position,
SignatureHelpTriggerInfo triggerInfo,
SignatureHelpOptions options,
CancellationToken cancellationToken)
{
return GetSignatureHelpAsync(
GetProviders(),
document,
position,
triggerInfo,
options,
cancellationToken);
}
}