[Async lightbulb] Move suppression fixes to 'Low' priority bucket

Addresses second item in dotnet#56843

Prior to this change, all the analyzers that are not high-pri are added to the normal priority bucket and executed at once. This PR further breaks down the normal priority bucket into two buckets as follow:

1. `Normal`: Analyzers that produce at least one fixable diagnostic, i.e. have a corresponding code fixer.
2. `Low`: Analyzers without any fixable diagnostics, i.e. have no corresponding code fixer. These diagnostics will only have Suppression/Configuration quick actions, which are lower priority and always show up at the bottom of the light bulb.

This new bucketing allows us to reduce the number of analyzers that are executed in the Normal priority bucket and hence generate all the non-suppression/configuration code fixes faster in async light bulb.

src/EditorFeatures/Core.Wpf/Suggestions/AsyncSuggestedActionsSource.cs

@@ -104,6 +104,7 @@ private async Task GetSuggestedActionsWorkerAsync(
                         {
                             VisualStudio.Utilities.DefaultOrderings.Highest => CodeActionRequestPriority.High,
                             VisualStudio.Utilities.DefaultOrderings.Default => CodeActionRequestPriority.Normal,
+                            VisualStudio.Utilities.DefaultOrderings.Lowest => CodeActionRequestPriority.Low,
                             _ => (CodeActionRequestPriority?)null,
                         };
 
@@ -116,7 +117,7 @@ private async Task GetSuggestedActionsWorkerAsync(
                                 state, requestedActionCategories, document,
                                 range, selection,
                                 addOperationScope: _ => null,
-                                includeSuppressionFixes: priority.Value == CodeActionRequestPriority.Normal,
+                                includeSuppressionFixes: priority.Value == CodeActionRequestPriority.Low,
                                 priority.Value,
                                 currentActionCount, cancellationToken).WithCancellation(cancellationToken).ConfigureAwait(false);
 

src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs

@@ -329,6 +329,13 @@ protected static Task<ImmutableArray<UnifiedSuggestedActionSet>> GetRefactorings
                     return SpecializedTasks.EmptyImmutableArray<UnifiedSuggestedActionSet>();
                 }
 
+                // 'CodeActionRequestPriority.Low' is reserved for suppression/configuration code fixes.
+                // No code refactoring should have this request priority.
+                if (priority == CodeActionRequestPriority.Low)
+                {
+                    return SpecializedTasks.EmptyImmutableArray<UnifiedSuggestedActionSet>();
+                }
+
                 // If we are computing refactorings outside the 'Refactoring' context, i.e. for example, from the lightbulb under a squiggle or selection,
                 // then we want to filter out refactorings outside the selection span.
                 var filterOutsideSelection = !requestedActionCategories.Contains(PredefinedSuggestedActionCategoryNames.Refactoring);

src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSourceProvider.cs

@@ -33,6 +33,7 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.Suggestions
     [Order]
     [SuggestedActionPriority(DefaultOrderings.Highest)]
     [SuggestedActionPriority(DefaultOrderings.Default)]
+    [SuggestedActionPriority(DefaultOrderings.Lowest)]
     internal partial class SuggestedActionsSourceProvider : ISuggestedActionsSourceProvider
     {
         private static readonly Guid s_CSharpSourceGuid = new Guid("b967fea8-e2c3-4984-87d4-71a38f49e16a");

src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs

@@ -124,6 +124,40 @@ public async Task TestGetFixesAsyncHasNoDuplicateConfigurationActions()
             }
         }
 
+        [Fact, WorkItem(56843, "https://github.com/dotnet/roslyn/issues/56843")]
+        public async Task TestGetFixesAsyncForFixableAndNonFixableAnalyzersAsync()
+        {
+            var codeFix = new MockFixer();
+            var analyzerWithFix = new MockAnalyzerReference.MockDiagnosticAnalyzer();
+            Assert.Equal(codeFix.FixableDiagnosticIds.Single(), analyzerWithFix.SupportedDiagnostics.Single().Id);
+
+            var analyzerWithoutFix = new MockAnalyzerReference.MockDiagnosticAnalyzer("AnalyzerWithoutFixId", "Category");
+            var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(analyzerWithFix, analyzerWithoutFix);
+            var analyzerReference = new MockAnalyzerReference(codeFix, analyzers);
+
+            // Verify no callbacks received at initialization.
+            Assert.False(analyzerWithFix.ReceivedCallback);
+            Assert.False(analyzerWithoutFix.ReceivedCallback);
+
+            var tuple = ServiceSetup(codeFix, includeConfigurationFixProviders: true);
+            using var workspace = tuple.workspace;
+            GetDocumentAndExtensionManager(tuple.analyzerService, workspace, out var document, out var extensionManager, analyzerReference);
+
+            // Verify only analyzerWithFix is executed when GetFixesAsync is invoked with 'CodeActionRequestPriority.Normal'.
+            _ = await tuple.codeFixService.GetFixesAsync(document, TextSpan.FromBounds(0, 0),
+                includeConfigurationFixes: false, priority: CodeActionRequestPriority.Normal, isBlocking: false,
+                addOperationScope: _ => null, cancellationToken: CancellationToken.None);
+            Assert.True(analyzerWithFix.ReceivedCallback);
+            Assert.False(analyzerWithoutFix.ReceivedCallback);
+
+            // Verify both analyzerWithFix and analyzerWithoutFix are executed when GetFixesAsync is invoked with 'CodeActionRequestPriority.Low'.
+            _ = await tuple.codeFixService.GetFixesAsync(document, TextSpan.FromBounds(0, 0),
+                includeConfigurationFixes: true, priority: CodeActionRequestPriority.Low, isBlocking: false,
+                addOperationScope: _ => null, cancellationToken: CancellationToken.None);
+            Assert.True(analyzerWithFix.ReceivedCallback);
+            Assert.True(analyzerWithoutFix.ReceivedCallback);
+        }
+
         [Fact]
         public async Task TestGetCodeFixWithExceptionInRegisterMethod_Diagnostic()
         {
@@ -387,6 +421,8 @@ public MockDiagnosticAnalyzer()
                 {
                 }
 
+                public bool ReceivedCallback { get; private set; }
+
                 private static ImmutableArray<DiagnosticDescriptor> CreateSupportedDiagnostics(ImmutableArray<(string id, string category)> reportedDiagnosticIdsWithCategories)
                 {
                     var builder = ArrayBuilder<DiagnosticDescriptor>.GetInstance();
@@ -405,6 +441,8 @@ public override void Initialize(AnalysisContext context)
                 {
                     context.RegisterSyntaxTreeAction(c =>
                     {
+                        this.ReceivedCallback = true;
+
                         foreach (var descriptor in SupportedDiagnostics)
                         {
                             c.ReportDiagnostic(Diagnostic.Create(descriptor, c.Tree.GetLocation(TextSpan.FromBounds(0, 0))));

src/EditorFeatures/Test/EditAndContinue/Helpers/MockDiagnosticAnalyzerService.cs

@@ -44,6 +44,9 @@ public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution s
         public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(Document document, TextSpan? range, string? diagnosticId = null, bool includeSuppressedDiagnostics = false, CodeActionRequestPriority priority = CodeActionRequestPriority.None, Func<string, IDisposable?>? addOperationScope = null, CancellationToken cancellationToken = default)
             => throw new NotImplementedException();
 
+        public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(Document document, TextSpan? range, Func<string, bool>? shouldIncludeDiagnostic, bool includeSuppressedDiagnostics = false, CodeActionRequestPriority priority = CodeActionRequestPriority.None, Func<string, IDisposable?>? addOperationScope = null, CancellationToken cancellationToken = default)
+            => throw new NotImplementedException();
+
         public Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId = null, ImmutableHashSet<string>? diagnosticIds = null, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
             => throw new NotImplementedException();
 

src/Features/Core/Portable/CodeFixes/CodeFixService.cs

@@ -172,8 +172,12 @@ public async Task<ImmutableArray<CodeFixCollection>> GetFixesAsync(
             // invariant: later code gathers & runs CodeFixProviders for diagnostics with one identical diagnostics span (that gets set later as CodeFixCollection's TextSpan)
             // order diagnostics by span.
             SortedDictionary<TextSpan, List<DiagnosticData>>? aggregatedDiagnostics = null;
+
+            // Only execute analyzers with fixable diagnostics for 'CodeActionPriorityRequest.Normal'
+            var shouldIncludeDiagnostic = priority == CodeActionRequestPriority.Normal ? GetFixableDiagnosticFilter(document) : null;
+
             var diagnostics = await _diagnosticService.GetDiagnosticsForSpanAsync(
-                document, range, diagnosticId: null, includeConfigurationFixes, priority, addOperationScope, cancellationToken).ConfigureAwait(false);
+                document, range, shouldIncludeDiagnostic, includeConfigurationFixes, priority, addOperationScope, cancellationToken).ConfigureAwait(false);
             foreach (var diagnostic in diagnostics)
             {
                 if (diagnostic.IsSuppressed)
@@ -196,11 +200,16 @@ public async Task<ImmutableArray<CodeFixCollection>> GetFixesAsync(
 
             // append fixes for all diagnostics with the same diagnostics span
             using var resultDisposer = ArrayBuilder<CodeFixCollection>.GetInstance(out var result);
-            foreach (var spanAndDiagnostic in aggregatedDiagnostics)
+
+            // 'CodeActionRequestPriority.Low' is used when the client only wants suppression/configuration fixes.
+            if (priority != CodeActionRequestPriority.Low)
             {
-                await AppendFixesAsync(
-                    document, spanAndDiagnostic.Key, spanAndDiagnostic.Value, fixAllForInSpan: false,
-                    priority, isBlocking, result, addOperationScope, cancellationToken).ConfigureAwait(false);
+                foreach (var spanAndDiagnostic in aggregatedDiagnostics)
+                {
+                    await AppendFixesAsync(
+                        document, spanAndDiagnostic.Key, spanAndDiagnostic.Value, fixAllForInSpan: false,
+                        priority, isBlocking, result, addOperationScope, cancellationToken).ConfigureAwait(false);
+                }
             }
 
             if (result.Count > 0 && TryGetWorkspaceFixersPriorityMap(document, out var fixersForLanguage))
@@ -229,6 +238,15 @@ await AppendConfigurationsAsync(
             }
 
             return result.ToImmutable();
+
+            // Local functions
+            Func<string, bool> GetFixableDiagnosticFilter(Document document)
+            {
+                var hasWorkspaceFixers = TryGetWorkspaceFixersMap(document, out var fixerMap);
+                var projectFixersMap = GetProjectFixers(document.Project);
+
+                return id => hasWorkspaceFixers && fixerMap!.Value.ContainsKey(id) || projectFixersMap.ContainsKey(id);
+            }
         }
 
         public async Task<CodeFixCollection?> GetDocumentFixAllForIdInSpanAsync(Document document, TextSpan range, string diagnosticId, CancellationToken cancellationToken)

src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService.cs

@@ -71,7 +71,7 @@ public Task<bool> TryAppendDiagnosticsForSpanAsync(Document document, TextSpan r
             {
                 // always make sure that analyzer is called on background thread.
                 return Task.Run(() => analyzer.TryAppendDiagnosticsForSpanAsync(
-                    document, range, diagnostics, diagnosticId: null, includeSuppressedDiagnostics, CodeActionRequestPriority.None, blockForData: false, addOperationScope: null, cancellationToken), cancellationToken);
+                    document, range, diagnostics, shouldIncludeDiagnostic: null, includeSuppressedDiagnostics, CodeActionRequestPriority.None, blockForData: false, addOperationScope: null, cancellationToken), cancellationToken);
             }
 
             return SpecializedTasks.False;
@@ -85,12 +85,26 @@ public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
             CodeActionRequestPriority priority,
             Func<string, IDisposable?>? addOperationScope,
             CancellationToken cancellationToken)
+        {
+            Func<string, bool>? shouldIncludeDiagnostic = diagnosticId != null ? id => id == diagnosticId : null;
+            return GetDiagnosticsForSpanAsync(document, range, shouldIncludeDiagnostic,
+                includeSuppressedDiagnostics, priority, addOperationScope, cancellationToken);
+        }
+
+        public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
+            Document document,
+            TextSpan? range,
+            Func<string, bool>? shouldIncludeDiagnostic,
+            bool includeSuppressedDiagnostics,
+            CodeActionRequestPriority priority,
+            Func<string, IDisposable?>? addOperationScope,
+            CancellationToken cancellationToken)
         {
             if (_map.TryGetValue(document.Project.Solution.Workspace, out var analyzer))
             {
                 // always make sure that analyzer is called on background thread.
                 return Task.Run(() => analyzer.GetDiagnosticsForSpanAsync(
-                    document, range, diagnosticId, includeSuppressedDiagnostics, priority,
+                    document, range, shouldIncludeDiagnostic, includeSuppressedDiagnostics, priority,
                     blockForData: true, addOperationScope, cancellationToken), cancellationToken);
             }
 

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs

@@ -21,20 +21,20 @@ namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2
     internal partial class DiagnosticIncrementalAnalyzer
     {
         public async Task<bool> TryAppendDiagnosticsForSpanAsync(
-            Document document, TextSpan? range, ArrayBuilder<DiagnosticData> result, string? diagnosticId,
+            Document document, TextSpan? range, ArrayBuilder<DiagnosticData> result, Func<string, bool>? shouldIncludeDiagnostic,
             bool includeSuppressedDiagnostics, CodeActionRequestPriority priority, bool blockForData,
             Func<string, IDisposable?>? addOperationScope, CancellationToken cancellationToken)
         {
             var getter = await LatestDiagnosticsForSpanGetter.CreateAsync(
                 this, document, range, blockForData, addOperationScope, includeSuppressedDiagnostics,
-                priority, diagnosticId, cancellationToken).ConfigureAwait(false);
+                priority, shouldIncludeDiagnostic, cancellationToken).ConfigureAwait(false);
             return await getter.TryGetAsync(result, cancellationToken).ConfigureAwait(false);
         }
 
         public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
             Document document,
             TextSpan? range,
-            string? diagnosticId,
+            Func<string, bool>? shouldIncludeDiagnostic,
             bool includeSuppressedDiagnostics,
             CodeActionRequestPriority priority,
             bool blockForData,
@@ -43,7 +43,7 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
         {
             using var _ = ArrayBuilder<DiagnosticData>.GetInstance(out var list);
             var result = await TryAppendDiagnosticsForSpanAsync(
-                document, range, list, diagnosticId, includeSuppressedDiagnostics,
+                document, range, list, shouldIncludeDiagnostic, includeSuppressedDiagnostics,
                 priority, blockForData, addOperationScope, cancellationToken).ConfigureAwait(false);
             Debug.Assert(result);
             return list.ToImmutable();
@@ -64,7 +64,7 @@ private sealed class LatestDiagnosticsForSpanGetter
             private readonly bool _blockForData;
             private readonly bool _includeSuppressedDiagnostics;
             private readonly CodeActionRequestPriority _priority;
-            private readonly string? _diagnosticId;
+            private readonly Func<string, bool>? _shouldIncludeDiagnostic;
             private readonly Func<string, IDisposable?>? _addOperationScope;
 
             private delegate Task<IEnumerable<DiagnosticData>> DiagnosticsGetterAsync(DiagnosticAnalyzer analyzer, DocumentAnalysisExecutor executor, CancellationToken cancellationToken);
@@ -77,22 +77,22 @@ public static async Task<LatestDiagnosticsForSpanGetter> CreateAsync(
                  Func<string, IDisposable?>? addOperationScope,
                  bool includeSuppressedDiagnostics,
                  CodeActionRequestPriority priority,
-                 string? diagnosticId,
+                 Func<string, bool>? shouldIncludeDiagnostic,
                  CancellationToken cancellationToken)
             {
                 var stateSets = owner._stateManager
                                      .GetOrCreateStateSets(document.Project).Where(s => !owner.DiagnosticAnalyzerInfoCache.IsAnalyzerSuppressed(s.Analyzer, document.Project));
 
                 // filter to specific diagnostic it is looking for
-                if (diagnosticId != null)
+                if (shouldIncludeDiagnostic != null)
                 {
-                    stateSets = stateSets.Where(s => owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(s.Analyzer).Any(d => d.Id == diagnosticId)).ToList();
+                    stateSets = stateSets.Where(s => owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(s.Analyzer).Any(d => shouldIncludeDiagnostic(d.Id))).ToList();
                 }
 
                 var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(document.Project, stateSets, includeSuppressedDiagnostics, cancellationToken).ConfigureAwait(false);
 
                 return new LatestDiagnosticsForSpanGetter(
-                    owner, compilationWithAnalyzers, document, stateSets, diagnosticId, range,
+                    owner, compilationWithAnalyzers, document, stateSets, shouldIncludeDiagnostic, range,
                     blockForData, addOperationScope, includeSuppressedDiagnostics, priority);
             }
 
@@ -101,7 +101,7 @@ private LatestDiagnosticsForSpanGetter(
                 CompilationWithAnalyzers? compilationWithAnalyzers,
                 Document document,
                 IEnumerable<StateSet> stateSets,
-                string? diagnosticId,
+                Func<string, bool>? shouldIncludeDiagnostic,
                 TextSpan? range,
                 bool blockForData,
                 Func<string, IDisposable?>? addOperationScope,
@@ -112,7 +112,7 @@ private LatestDiagnosticsForSpanGetter(
                 _compilationWithAnalyzers = compilationWithAnalyzers;
                 _document = document;
                 _stateSets = stateSets;
-                _diagnosticId = diagnosticId;
+                _shouldIncludeDiagnostic = shouldIncludeDiagnostic;
                 _range = range;
                 _blockForData = blockForData;
                 _addOperationScope = addOperationScope;
@@ -243,6 +243,11 @@ private bool MatchesPriority(DiagnosticAnalyzer analyzer)
                 if (_priority == CodeActionRequestPriority.None)
                     return true;
 
+                // 'CodeActionRequestPriority.Low' is used for suppression/configuration fixes,
+                // which requires all analyzer diagnostics.
+                if (_priority == CodeActionRequestPriority.Low)
+                    return true;
+
                 // The compiler analyzer always counts for any priority.  It's diagnostics may be fixed
                 // by high pri or normal pri fixers.
                 if (analyzer.IsCompilerAnalyzer())
@@ -259,7 +264,7 @@ private bool ShouldInclude(DiagnosticData diagnostic)
                 return diagnostic.DocumentId == _document.Id &&
                     (_range == null || _range.Value.IntersectsWith(diagnostic.GetTextSpan()))
                     && (_includeSuppressedDiagnostics || !diagnostic.IsSuppressed)
-                    && (_diagnosticId == null || _diagnosticId == diagnostic.Id);
+                    && (_shouldIncludeDiagnostic == null || _shouldIncludeDiagnostic(diagnostic.Id));
             }
         }
     }