From da64131ad60ce3d39c0e4d6b0bf6065d035825df Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 16 Feb 2022 12:14:21 -0800 Subject: [PATCH] Allow codefix service to produce codefixes in a streaming fashion --- .../Core/Portable/CodeFixes/CodeFixService.cs | 95 ++++++++++--------- .../Portable/CodeFixes/ICodeFixService.cs | 11 ++- .../Extensions/IAsyncEnumerableExtensions.cs | 24 +++++ 3 files changed, 84 insertions(+), 46 deletions(-) create mode 100644 src/Workspaces/Core/Portable/Shared/Extensions/IAsyncEnumerableExtensions.cs diff --git a/src/Features/Core/Portable/CodeFixes/CodeFixService.cs b/src/Features/Core/Portable/CodeFixes/CodeFixService.cs index 24b5fa1096df0..2024a876c3cb6 100644 --- a/src/Features/Core/Portable/CodeFixes/CodeFixService.cs +++ b/src/Features/Core/Portable/CodeFixes/CodeFixService.cs @@ -142,13 +142,13 @@ public async Task GetMostSevereFixableDiagnosticAsync( return null; } - public async Task> GetFixesAsync( + public async IAsyncEnumerable StreamFixesAsync( Document document, TextSpan range, CodeActionRequestPriority priority, CodeActionOptions options, Func addOperationScope, - CancellationToken cancellationToken) + [EnumeratorCancellation] CancellationToken cancellationToken) { // REVIEW: this is the first and simplest design. basically, when ctrl+. is pressed, it asks diagnostic service to give back // current diagnostics for the given span, and it will use that to get fixes. internally diagnostic service will either return cached information @@ -172,7 +172,7 @@ public async Task> GetFixesAsync( ? GetFixableDiagnosticFilter(document) : null; - // We only need to compute suppression/configurationofixes when request priority is + // We only need to compute suppression/configuration fixes when request priority is // 'CodeActionPriorityRequest.Lowest' or 'CodeActionPriorityRequest.None'. var includeSuppressionFixes = priority is CodeActionRequestPriority.Lowest or CodeActionRequestPriority.None; @@ -188,21 +188,21 @@ public async Task> GetFixesAsync( } if (aggregatedDiagnostics.Count == 0) - return ImmutableArray.Empty; + yield break; // Order diagnostics by DiagnosticId so the fixes are in a deterministic order. foreach (var (_, diagnosticList) in aggregatedDiagnostics) diagnosticList.Sort(s_diagnosticDataComparisonById); - // append fixes for all diagnostics with the same diagnostics span - using var _1 = ArrayBuilder.GetInstance(out var result); - // 'CodeActionRequestPriority.Lowest' is used when the client only wants suppression/configuration fixes. if (priority != CodeActionRequestPriority.Lowest) { - await AppendFixesAsync( + await foreach (var collection in StreamFixesAsync( document, aggregatedDiagnostics, fixAllForInSpan: false, - priority, options, result, addOperationScope, cancellationToken).ConfigureAwait(false); + priority, options, addOperationScope, cancellationToken).ConfigureAwait(false)) + { + yield return collection; + } } // TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict CodeFixes in Interactive @@ -212,13 +212,15 @@ await AppendFixesAsync( using var _2 = PooledHashSet.GetInstance(out var registeredConfigurationFixTitles); foreach (var (span, diagnosticList) in aggregatedDiagnostics) { - await AppendConfigurationsAsync( - document, span, diagnosticList, - result, registeredConfigurationFixTitles, cancellationToken).ConfigureAwait(false); + await foreach (var codeFixCollection in StreamConfigurationFixesAsync( + document, span, diagnosticList, registeredConfigurationFixTitles, cancellationToken).ConfigureAwait(false)) + { + yield return codeFixCollection; + } } } - return result.ToImmutable(); + yield break; // Local functions Func GetFixableDiagnosticFilter(Document document) @@ -249,13 +251,16 @@ Func GetFixableDiagnosticFilter(Document document) {range, diagnostics }, }; - await AppendFixesAsync( + await foreach (var collection in StreamFixesAsync( document, spanToDiagnostics, fixAllForInSpan: true, CodeActionRequestPriority.None, - options, result, addOperationScope: static _ => null, cancellationToken).ConfigureAwait(false); + options, addOperationScope: static _ => null, cancellationToken).ConfigureAwait(false)) + { + // TODO: Just get the first fix for now until we have a way to config user's preferred fix + // https://github.com/dotnet/roslyn/issues/27066 + return collection; + } - // TODO: Just get the first fix for now until we have a way to config user's preferred fix - // https://github.com/dotnet/roslyn/issues/27066 - return result.ToImmutable().FirstOrDefault(); + return null; } public async Task ApplyCodeFixesForSpecificDiagnosticIdAsync(Document document, string diagnosticId, IProgressTracker progressTracker, CodeActionOptions options, CancellationToken cancellationToken) @@ -342,15 +347,14 @@ private bool TryGetWorkspaceFixer( } } - private async Task AppendFixesAsync( + private async IAsyncEnumerable StreamFixesAsync( Document document, SortedDictionary> spanToDiagnostics, bool fixAllForInSpan, CodeActionRequestPriority priority, CodeActionOptions options, - ArrayBuilder result, Func addOperationScope, - CancellationToken cancellationToken) + [EnumeratorCancellation] CancellationToken cancellationToken) { var hasAnySharedFixer = TryGetWorkspaceFixersMap(document, out var fixerMap); @@ -358,7 +362,7 @@ private async Task AppendFixesAsync( var hasAnyProjectFixer = projectFixersMap.Any(); if (!hasAnySharedFixer && !hasAnyProjectFixer) - return; + yield break; // TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict CodeFixes in Interactive var isInteractive = document.Project.Solution.Workspace.Kind == WorkspaceKind.Interactive; @@ -424,8 +428,8 @@ private async Task AppendFixesAsync( foreach (var (span, diagnostics) in fixerToRangesAndDiagnostics[fixer]) { - await AppendFixesOrConfigurationsAsync( - document, span, diagnostics, fixAllForInSpan, result, fixer, + var codeFixCollection = await TryGetFixesOrConfigurationsAsync( + document, span, diagnostics, fixAllForInSpan, fixer, hasFix: d => this.GetFixableDiagnosticIds(fixer, extensionManager).Contains(d.Id), getFixes: dxs => { @@ -451,9 +455,14 @@ await AppendFixesOrConfigurationsAsync( }, cancellationToken).ConfigureAwait(false); - // Just need the first result if we are doing fix all in span - if (fixAllForInSpan && result.Any()) - return; + if (codeFixCollection != null) + { + yield return codeFixCollection; + + // Just need the first result if we are doing fix all in span + if (fixAllForInSpan) + yield break; + } } } } @@ -465,7 +474,7 @@ await AppendFixesOrConfigurationsAsync( } } - return; + yield break; void AddAllFixers( ImmutableArray fixers, @@ -557,17 +566,17 @@ static ImmutableArray FilterApplicableDiagnostics( } } - private async Task AppendConfigurationsAsync( + private async IAsyncEnumerable StreamConfigurationFixesAsync( Document document, TextSpan diagnosticsSpan, IEnumerable diagnostics, - ArrayBuilder result, PooledHashSet registeredConfigurationFixTitles, - CancellationToken cancellationToken) + [EnumeratorCancellation] CancellationToken cancellationToken) { - if (!_configurationProvidersMap.TryGetValue(document.Project.Language, out var lazyConfigurationProviders) || lazyConfigurationProviders.Value == null) + if (!_configurationProvidersMap.TryGetValue(document.Project.Language, out var lazyConfigurationProviders) || + lazyConfigurationProviders.Value == null) { - return; + yield break; } // append CodeFixCollection for each CodeFixProvider @@ -575,8 +584,8 @@ private async Task AppendConfigurationsAsync( { using (RoslynEventSource.LogInformationalBlock(FunctionId.CodeFixes_GetCodeFixesAsync, provider, cancellationToken)) { - await AppendFixesOrConfigurationsAsync( - document, diagnosticsSpan, diagnostics, fixAllForInSpan: false, result, provider, + var codeFixCollection = await TryGetFixesOrConfigurationsAsync( + document, diagnosticsSpan, diagnostics, fixAllForInSpan: false, provider, hasFix: d => provider.IsFixableDiagnostic(d), getFixes: async dxs => { @@ -584,16 +593,17 @@ await AppendFixesOrConfigurationsAsync( return fixes.WhereAsArray(f => registeredConfigurationFixTitles.Add(f.Action.Title)); }, cancellationToken).ConfigureAwait(false); + if (codeFixCollection != null) + yield return codeFixCollection; } } } - private async Task AppendFixesOrConfigurationsAsync( + private async Task TryGetFixesOrConfigurationsAsync( Document document, TextSpan fixesSpan, IEnumerable diagnosticsWithSameSpan, bool fixAllForInSpan, - ArrayBuilder result, TCodeFixProvider fixer, Func hasFix, Func, Task>> getFixes, @@ -607,7 +617,7 @@ await diagnosticsWithSameSpan.OrderByDescending(d => d.Severity) if (diagnostics.Length <= 0) { // this can happen for suppression case where all diagnostics can't be suppressed - return; + return null; } var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService(); @@ -616,9 +626,7 @@ await diagnosticsWithSameSpan.OrderByDescending(d => d.Severity) defaultValue: ImmutableArray.Empty).ConfigureAwait(false); if (fixes.IsDefaultOrEmpty) - { - return; - } + return null; // If the fix provider supports fix all occurrences, then get the corresponding FixAllProviderInfo and fix all context. var fixAllProviderInfo = extensionManager.PerformFunction( @@ -667,10 +675,9 @@ await diagnosticsWithSameSpan.OrderByDescending(d => d.Severity) supportedScopes = fixAllProviderInfo.SupportedScopes; } - var codeFix = new CodeFixCollection( + return new CodeFixCollection( fixer, fixesSpan, fixes, fixAllState, supportedScopes, diagnostics.First()); - result.Add(codeFix); } /// Looks explicitly for an . @@ -992,7 +999,7 @@ public FixerComparer(ImmutableDictionary priorityMap) => _priorityMap = priorityMap; public int Compare([AllowNull] CodeFixProvider x, [AllowNull] CodeFixProvider y) - => GetValue(x).CompareTo(GetValue(y)); + => GetValue(x!).CompareTo(GetValue(y!)); private int GetValue(CodeFixProvider provider) => _priorityMap.TryGetValue(provider, out var value) ? value : int.MaxValue; diff --git a/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs b/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs index c001b53f37052..fc04f0104fd90 100644 --- a/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs +++ b/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; @@ -15,7 +16,7 @@ namespace Microsoft.CodeAnalysis.CodeFixes { internal interface ICodeFixService { - Task> GetFixesAsync(Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func addOperationScope, CancellationToken cancellationToken); + IAsyncEnumerable StreamFixesAsync(Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func addOperationScope, CancellationToken cancellationToken); Task GetDocumentFixAllForIdInSpanAsync(Document document, TextSpan textSpan, string diagnosticId, CodeActionOptions options, CancellationToken cancellationToken); Task ApplyCodeFixesForSpecificDiagnosticIdAsync(Document document, string diagnosticId, IProgressTracker progressTracker, CodeActionOptions options, CancellationToken cancellationToken); CodeFixProvider? GetSuppressionFixer(string language, IEnumerable diagnosticIds); @@ -24,7 +25,13 @@ internal interface ICodeFixService internal static class ICodeFixServiceExtensions { + public static IAsyncEnumerable StreamFixesAsync(this ICodeFixService service, Document document, TextSpan range, CodeActionOptions options, CancellationToken cancellationToken) + => service.StreamFixesAsync(document, range, CodeActionRequestPriority.None, options, addOperationScope: _ => null, cancellationToken); + public static Task> GetFixesAsync(this ICodeFixService service, Document document, TextSpan range, CodeActionOptions options, CancellationToken cancellationToken) - => service.GetFixesAsync(document, range, CodeActionRequestPriority.None, options, addOperationScope: _ => null, cancellationToken); + => service.StreamFixesAsync(document, range, options, cancellationToken).ToImmutableArrayAsync(); + + public static Task> GetFixesAsync(this ICodeFixService service, Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func addOperationScope, CancellationToken cancellationToken) + => service.StreamFixesAsync(document, textSpan, priority, options, addOperationScope, cancellationToken).ToImmutableArrayAsync(); } } diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/IAsyncEnumerableExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/IAsyncEnumerableExtensions.cs new file mode 100644 index 0000000000000..c246588d4acef --- /dev/null +++ b/src/Workspaces/Core/Portable/Shared/Extensions/IAsyncEnumerableExtensions.cs @@ -0,0 +1,24 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.PooledObjects; + +namespace Microsoft.CodeAnalysis.Shared.Extensions +{ + internal static class IAsyncEnumerableExtensions + { + public static async Task> ToImmutableArrayAsync(this IAsyncEnumerable values) + { + using var _ = ArrayBuilder.GetInstance(out var result); + await foreach (var value in values.ConfigureAwait(false)) + result.Add(value); + + return result.ToImmutable(); + } + } +}