Unify command handler code

CyrusNajmabadi · CyrusNajmabadi · commit 40a85dfea593 · 2025-05-29T15:27:14.000+02:00
diff --git a/src/EditorFeatures/Core/FindReferences/FindReferencesCommandHandler.cs b/src/EditorFeatures/Core/FindReferences/FindReferencesCommandHandler.cs
@@ -2,26 +2,19 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
-using System;
 using System.ComponentModel.Composition;
 using System.Diagnostics.CodeAnalysis;
+using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Classification;
 using Microsoft.CodeAnalysis.Editor;
 using Microsoft.CodeAnalysis.Editor.Host;
-using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
-using Microsoft.CodeAnalysis.ErrorReporting;
+using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.FindUsages;
 using Microsoft.CodeAnalysis.GoToDefinition;
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
-using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.Commanding;
-using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;
 using Microsoft.VisualStudio.Utilities;
 
@@ -30,134 +23,37 @@ namespace Microsoft.CodeAnalysis.FindReferences;
 [Export(typeof(ICommandHandler))]
 [ContentType(ContentTypeNames.RoslynContentType)]
 [Name(PredefinedCommandHandlerNames.FindReferences)]
-internal sealed class FindReferencesCommandHandler : AbstractGoToCommandHandler<
-     IFindUsagesService, FindReferencesCommandArgs>
+[method: ImportingConstructor]
+[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
+internal sealed class FindReferencesCommandHandler(
+    IThreadingContext threadingContext,
+    IStreamingFindUsagesPresenter streamingPresenter,
+    IAsynchronousOperationListenerProvider listenerProvider,
+    IGlobalOptionService globalOptions) : AbstractGoOrFindCommandHandler<IFindUsagesService, FindReferencesCommandArgs>(
+        threadingContext,
+        streamingPresenter,
+        listenerProvider.GetListener(FeatureAttribute.FindReferences),
+        globalOptions)
 {
-#if false
-    private readonly IStreamingFindUsagesPresenter _streamingPresenter;
-    private readonly IGlobalOptionService _globalOptions;
-    private readonly IAsynchronousOperationListener _asyncListener;
-
-    public string DisplayName => EditorFeaturesResources.Find_References;
-
-    [ImportingConstructor]
-    [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
-    public FindReferencesCommandHandler(
-        IStreamingFindUsagesPresenter streamingPresenter,
-        IGlobalOptionService globalOptions,
-        IAsynchronousOperationListenerProvider listenerProvider)
-    {
-        Contract.ThrowIfNull(listenerProvider);
-
-        _streamingPresenter = streamingPresenter;
-        _globalOptions = globalOptions;
-        _asyncListener = listenerProvider.GetListener(FeatureAttribute.FindReferences);
-    }
-
-    public CommandState GetCommandState(FindReferencesCommandArgs args)
-    {
-        var (_, service) = GetDocumentAndService(args.SubjectBuffer.CurrentSnapshot);
-        return service != null
-            ? CommandState.Available
-            : CommandState.Unspecified;
-    }
-
-    public bool ExecuteCommand(FindReferencesCommandArgs args, CommandExecutionContext context)
-    {
-        var subjectBuffer = args.SubjectBuffer;
-
-        // Get the selection that user has in our buffer (this also works if there
-        // is no selection and the caret is just at a single position).  If we 
-        // can't get the selection, or there are multiple spans for it (i.e. a 
-        // box selection), then don't do anything.
-        var snapshotSpans = args.TextView.Selection.GetSnapshotSpansOnBuffer(subjectBuffer);
-        if (snapshotSpans.Count == 1)
-        {
-            var selectedSpan = snapshotSpans[0];
-            var (document, service) = GetDocumentAndService(subjectBuffer.CurrentSnapshot);
-            if (document != null)
-            {
-                // Do a find-refs at the *start* of the selection.  That way if the
-                // user has selected a symbol that has another symbol touching it
-                // on the right (i.e.  Goo++  ), then we'll do the find-refs on the
-                // symbol selected, not the symbol following.
-                if (TryExecuteCommand(selectedSpan.Start, document, service))
-                {
-                    return true;
-                }
-            }
-        }
-
-        return false;
-    }
+    public override string DisplayName => EditorFeaturesResources.Find_References;
 
-    private static (Document, IFindUsagesService) GetDocumentAndService(ITextSnapshot snapshot)
-    {
-        var document = snapshot.GetOpenDocumentInCurrentContextWithChanges();
-        return (document, document?.GetLanguageService<IFindUsagesService>());
-    }
-
-    private bool TryExecuteCommand(int caretPosition, Document document, IFindUsagesService findUsagesService)
-    {
-        // See if we're running on a host that can provide streaming results.
-        // We'll both need a FAR service that can stream results to us, and 
-        // a presenter that can accept streamed results.
-        if (findUsagesService != null && _streamingPresenter != null)
-        {
-            // kick this work off in a fire and forget fashion.  Importantly, this means we do
-            // not pass in any ambient cancellation information as the execution of this command
-            // will complete and will have no bearing on the computation of the references we compute.
-            _ = StreamingFindReferencesAsync(document, caretPosition, findUsagesService, _streamingPresenter);
-            return true;
-        }
+    protected override FunctionId FunctionId => FunctionId.CommandHandler_FindAllReference;
 
-        return false;
-    }
+    /// <summary>
+    /// For find-refs, we *always* use the window.  Even if there is only a single result.  This is not a 'go' command 
+    /// which imperatively tries to navigate to the location if possible.  The intent here is to keep the results in view
+    /// so that the user can always refer to them, even as they do other work.
+    /// </summary>
+    protected override bool NavigateToSingleResultIfQuick => false;
 
-    private async Task StreamingFindReferencesAsync(
-        Document document,
-        int caretPosition,
-        IFindUsagesService findUsagesService,
-        IStreamingFindUsagesPresenter presenter)
-    {
-        try
+    protected override StreamingFindUsagesPresenterOptions GetStreamingPresenterOptions(Document document)
+        => new()
         {
-            using var token = _asyncListener.BeginAsyncOperation(nameof(StreamingFindReferencesAsync));
-            var classificationOptions = _globalOptions.GetClassificationOptionsProvider();
+            SupportsReferences = true,
+            IncludeContainingTypeAndMemberColumns = document.Project.SupportsCompilation,
+            IncludeKindColumn = document.Project.Language != LanguageNames.FSharp
+        };
 
-            // Let the presented know we're starting a search.  It will give us back the context object that the FAR
-            // service will push results into. This operation is not externally cancellable.  Instead, the find refs
-            // window will cancel it if another request is made to use it.
-            var (context, cancellationToken) = presenter.StartSearch(
-                EditorFeaturesResources.Find_References,
-                new StreamingFindUsagesPresenterOptions()
-                {
-                    SupportsReferences = true,
-                    IncludeContainingTypeAndMemberColumns = document.Project.SupportsCompilation,
-                    IncludeKindColumn = document.Project.Language != LanguageNames.FSharp
-                });
-
-            using (Logger.LogBlock(
-                FunctionId.CommandHandler_FindAllReference,
-                KeyValueLogMessage.Create(LogType.UserAction, static m => m["type"] = "streaming"),
-                cancellationToken))
-            {
-                try
-                {
-                    await findUsagesService.FindReferencesAsync(context, document, caretPosition, classificationOptions, cancellationToken).ConfigureAwait(false);
-                }
-                finally
-                {
-                    await context.OnCompletedAsync(cancellationToken).ConfigureAwait(false);
-                }
-            }
-        }
-        catch (OperationCanceledException)
-        {
-        }
-        catch (Exception e) when (FatalError.ReportAndCatch(e))
-        {
-        }
-    }
-#endif
+    protected override Task FindActionAsync(IFindUsagesContext context, Document document, IFindUsagesService service, int caretPosition, CancellationToken cancellationToken)
+        => service.FindReferencesAsync(context, document, caretPosition, this.ClassificationOptionsProvider, cancellationToken);
 }
diff --git a/src/EditorFeatures/Core/FindUsages/BufferedFindUsagesContext.cs b/src/EditorFeatures/Core/FindUsages/BufferedFindUsagesContext.cs
@@ -8,6 +8,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Notification;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.Utilities;
 using Roslyn.Utilities;
 
@@ -28,6 +29,7 @@ private sealed class State
         public string? InformationalMessage;
         public string? SearchTitle;
         public ImmutableArray<DefinitionItem>.Builder Definitions = ImmutableArray.CreateBuilder<DefinitionItem>();
+        public ImmutableArray<SourceReferenceItem>.Builder References = ImmutableArray.CreateBuilder<SourceReferenceItem>();
     }
 
     /// <summary>
@@ -108,6 +110,8 @@ public async Task AttachToStreamingPresenterAsync(IFindUsagesContext presenterCo
         foreach (var definition in _state.Definitions)
             await presenterContext.OnDefinitionFoundAsync(definition, cancellationToken).ConfigureAwait(false);
 
+        await presenterContext.OnReferencesFoundAsync(_state.References.AsAsyncEnumerable(), cancellationToken).ConfigureAwait(false);
+
         // Now swap over to the presenter being the sink for all future callbacks, and clear any buffered data.
         _streamingPresenterContext = presenterContext;
         _state = null;
@@ -199,11 +203,18 @@ async ValueTask IFindUsagesContext.OnDefinitionFoundAsync(DefinitionItem definit
         }
     }
 
-    ValueTask IFindUsagesContext.OnReferencesFoundAsync(IAsyncEnumerable<SourceReferenceItem> references, CancellationToken cancellationToken)
+    async ValueTask IFindUsagesContext.OnReferencesFoundAsync(IAsyncEnumerable<SourceReferenceItem> references, CancellationToken cancellationToken)
     {
-        // Entirely ignored.  These features do not show references.
-        Contract.Fail("GoToImpl/Base should never report a reference.");
-        return ValueTaskFactory.CompletedTask;
+        using var _ = await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false);
+        if (IsSwapped)
+        {
+            await _streamingPresenterContext.OnReferencesFoundAsync(references, cancellationToken).ConfigureAwait(false);
+        }
+        else
+        {
+            await foreach (var reference in references)
+                _state.References.Add(reference);
+        }
     }
 
     #endregion
diff --git a/src/EditorFeatures/Core/GoToBase/GoToBaseCommandHandler.cs b/src/EditorFeatures/Core/GoToBase/GoToBaseCommandHandler.cs
@@ -30,7 +30,7 @@ internal sealed class GoToBaseCommandHandler(
     IThreadingContext threadingContext,
     IStreamingFindUsagesPresenter streamingPresenter,
     IAsynchronousOperationListenerProvider listenerProvider,
-    IGlobalOptionService globalOptions) : AbstractGoToCommandHandler<IGoToBaseService, GoToBaseCommandArgs>(
+    IGlobalOptionService globalOptions) : AbstractGoOrFindCommandHandler<IGoToBaseService, GoToBaseCommandArgs>(
         threadingContext,
         streamingPresenter,
         listenerProvider.GetListener(FeatureAttribute.GoToBase),
@@ -40,6 +40,12 @@ internal sealed class GoToBaseCommandHandler(
 
     protected override FunctionId FunctionId => FunctionId.CommandHandler_GoToBase;
 
+    /// <summary>
+    /// If we find a single results quickly enough, we do want to take the user directly to it,
+    /// instead of popping up the FAR window to show it.
+    /// </summary>
+    protected override bool NavigateToSingleResultIfQuick => true;
+
     protected override Task FindActionAsync(IFindUsagesContext context, Document document, IGoToBaseService service, int caretPosition, CancellationToken cancellationToken)
         => service.FindBasesAsync(context, document, caretPosition, ClassificationOptionsProvider, cancellationToken);
 }
diff --git a/src/EditorFeatures/Core/GoToDefinition/AbstractGoOrFindCommandHandler.cs b/src/EditorFeatures/Core/GoToDefinition/AbstractGoOrFindCommandHandler.cs
@@ -8,9 +8,7 @@
 using Microsoft.CodeAnalysis.Classification;
 using Microsoft.CodeAnalysis.Editor.Host;
 using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
-using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
-using Microsoft.CodeAnalysis.Editor.Tagging;
 using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.FindUsages;
 using Microsoft.CodeAnalysis.Host;
@@ -28,7 +26,7 @@
 
 namespace Microsoft.CodeAnalysis.GoToDefinition;
 
-internal abstract class AbstractGoToCommandHandler<TLanguageService, TCommandArgs>(
+internal abstract class AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs>(
     IThreadingContext threadingContext,
     IStreamingFindUsagesPresenter streamingPresenter,
     IAsynchronousOperationListener listener,
@@ -69,8 +67,17 @@ internal abstract class AbstractGoToCommandHandler<TLanguageService, TCommandArg
     private Func<CancellationToken, Task>? _delayHook;
 
     public abstract string DisplayName { get; }
+
     protected abstract FunctionId FunctionId { get; }
 
+    /// <summary>
+    /// If we should try to navigate to the sole item found, if that item was found within 1.5seconds.
+    /// </summary>
+    protected abstract bool NavigateToSingleResultIfQuick { get; }
+
+    protected virtual StreamingFindUsagesPresenterOptions GetStreamingPresenterOptions(Document document)
+        => StreamingFindUsagesPresenterOptions.Default;
+
     protected abstract Task FindActionAsync(IFindUsagesContext context, Document document, TLanguageService service, int caretPosition, CancellationToken cancellationToken);
 
     private static (Document?, TLanguageService?) GetDocumentAndService(ITextSnapshot snapshot)
@@ -166,16 +173,16 @@ private async Task ExecuteCommandWorkerAsync(
         // IStreamingFindUsagesPresenter.
         var findContext = new BufferedFindUsagesContext();
 
-        var delayTask = DelayAsync(cancellationToken);
+        var delayBeforeShowingResultsWindowTask = DelayAsync(cancellationToken);
         var findTask = FindResultsAsync(findContext, document, service, position, cancellationToken);
 
-        var firstFinishedTask = await Task.WhenAny(delayTask, findTask).ConfigureAwait(false);
+        var firstFinishedTask = await Task.WhenAny(delayBeforeShowingResultsWindowTask, findTask).ConfigureAwait(false);
         if (cancellationToken.IsCancellationRequested)
             // we bailed out because another command was issued.  Immediately stop everything we're doing and return
             // back so the next operation can run.
             return;
 
-        if (firstFinishedTask == findTask)
+        if (this.NavigateToSingleResultIfQuick && firstFinishedTask == findTask)
         {
             // We completed the search within 1.5 seconds.  If we had at least one result then Navigate to it directly
             // (if there is just one) or present them all if there are many.
@@ -196,7 +203,7 @@ await _streamingPresenter.TryPresentLocationOrNavigateIfOneAsync(
         // We either got no results, or 1.5 has passed and we didn't figure out the symbols to navigate to or
         // present.  So pop up the presenter to show the user that we're involved in a longer search, without
         // blocking them.
-        await PresentResultsInStreamingPresenterAsync(findContext, findTask, cancellationToken).ConfigureAwait(false);
+        await PresentResultsInStreamingPresenterAsync(document, findContext, findTask, cancellationToken).ConfigureAwait(false);
     }
 
     private Task DelayAsync(CancellationToken cancellationToken)
@@ -206,16 +213,24 @@ private Task DelayAsync(CancellationToken cancellationToken)
             return delayHook(cancellationToken);
         }
 
-        return Task.Delay(TaggerDelay.OnIdle.ComputeTimeDelay(), cancellationToken);
+        // If we want to navigate to a single result if it is found quickly, then delay showing the find-refs winfor
+        // for 1.5 seconds to see if a result comes in by then.  If we're not navigating and are always showing the
+        // far window, then don't have any delay showing the window.
+        var delay = this.NavigateToSingleResultIfQuick
+            ? DelayTimeSpan.Idle
+            : TimeSpan.Zero;
+
+        return Task.Delay(delay, cancellationToken);
     }
 
     private async Task PresentResultsInStreamingPresenterAsync(
+        Document document,
         BufferedFindUsagesContext findContext,
         Task findTask,
         CancellationToken cancellationToken)
     {
         await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
-        var (presenterContext, presenterCancellationToken) = _streamingPresenter.StartSearch(DisplayName, StreamingFindUsagesPresenterOptions.Default);
+        var (presenterContext, presenterCancellationToken) = _streamingPresenter.StartSearch(DisplayName, GetStreamingPresenterOptions(document));
 
         try
         {
@@ -276,9 +291,9 @@ internal TestAccessor GetTestAccessor()
 
     internal readonly struct TestAccessor
     {
-        private readonly AbstractGoToCommandHandler<TLanguageService, TCommandArgs> _instance;
+        private readonly AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs> _instance;
 
-        internal TestAccessor(AbstractGoToCommandHandler<TLanguageService, TCommandArgs> instance)
+        internal TestAccessor(AbstractGoOrFindCommandHandler<TLanguageService, TCommandArgs> instance)
             => _instance = instance;
 
         internal ref Func<CancellationToken, Task>? DelayHook
diff --git a/src/EditorFeatures/Core/GoToImplementation/GoToImplementationCommandHandler.cs b/src/EditorFeatures/Core/GoToImplementation/GoToImplementationCommandHandler.cs
@@ -30,7 +30,7 @@ internal sealed class GoToImplementationCommandHandler(
     IThreadingContext threadingContext,
     IStreamingFindUsagesPresenter streamingPresenter,
     IAsynchronousOperationListenerProvider listenerProvider,
-    IGlobalOptionService globalOptions) : AbstractGoToCommandHandler<IFindUsagesService, GoToImplementationCommandArgs>(
+    IGlobalOptionService globalOptions) : AbstractGoOrFindCommandHandler<IFindUsagesService, GoToImplementationCommandArgs>(
         threadingContext,
         streamingPresenter,
         listenerProvider.GetListener(FeatureAttribute.GoToImplementation),
@@ -40,6 +40,12 @@ internal sealed class GoToImplementationCommandHandler(
 
     protected override FunctionId FunctionId => FunctionId.CommandHandler_GoToImplementation;
 
+    /// <summary>
+    /// If we find a single results quickly enough, we do want to take the user directly to it,
+    /// instead of popping up the FAR window to show it.
+    /// </summary>
+    protected override bool NavigateToSingleResultIfQuick => true;
+
     protected override Task FindActionAsync(IFindUsagesContext context, Document document, IFindUsagesService service, int caretPosition, CancellationToken cancellationToken)
         => service.FindImplementationsAsync(context, document, caretPosition, ClassificationOptionsProvider, cancellationToken);
 }
diff --git a/src/EditorFeatures/Test/FindReferences/FindReferencesCommandHandlerTests.cs b/src/EditorFeatures/Test/FindReferences/FindReferencesCommandHandlerTests.cs
diff --git a/src/EditorFeatures/Test2/FindReferences/FindReferencesCommandHandlerTests.vb b/src/EditorFeatures/Test2/FindReferences/FindReferencesCommandHandlerTests.vb
