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

Allow codefixservice to stream codefixes as it is computed, instead of waiting for all fixes to be ready. #59585

Merged
merged 5 commits into from
Feb 17, 2022
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
93 changes: 50 additions & 43 deletions src/Features/Core/Portable/CodeFixes/CodeFixService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@ public async Task<FirstDiagnosticResult> GetMostSevereFixableDiagnosticAsync(
return null;
}

public async Task<ImmutableArray<CodeFixCollection>> GetFixesAsync(
public async IAsyncEnumerable<CodeFixCollection> StreamFixesAsync(
Document document,
TextSpan range,
CodeActionRequestPriority priority,
CodeActionOptions options,
Func<string, IDisposable?> addOperationScope,
CancellationToken cancellationToken)
[EnumeratorCancellation] CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand All @@ -173,7 +173,7 @@ public async Task<ImmutableArray<CodeFixCollection>> 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;

Expand All @@ -189,21 +189,21 @@ public async Task<ImmutableArray<CodeFixCollection>> GetFixesAsync(
}

if (aggregatedDiagnostics.Count == 0)
return ImmutableArray<CodeFixCollection>.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<CodeFixCollection>.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
Expand All @@ -213,13 +213,15 @@ await AppendFixesAsync(
using var _2 = PooledHashSet<string>.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<string, bool> GetFixableDiagnosticFilter(Document document)
Expand Down Expand Up @@ -252,13 +254,16 @@ Func<string, bool> 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<Document> ApplyCodeFixesForSpecificDiagnosticIdAsync(Document document, string diagnosticId, IProgressTracker progressTracker, CodeActionOptions options, CancellationToken cancellationToken)
Expand Down Expand Up @@ -347,15 +352,14 @@ private bool TryGetWorkspaceFixer(
}
}

private async Task AppendFixesAsync(
private async IAsyncEnumerable<CodeFixCollection> StreamFixesAsync(
Document document,
SortedDictionary<TextSpan, List<DiagnosticData>> spanToDiagnostics,
bool fixAllForInSpan,
CodeActionRequestPriority priority,
CodeActionOptions options,
ArrayBuilder<CodeFixCollection> result,
Func<string, IDisposable?> addOperationScope,
CancellationToken cancellationToken)
[EnumeratorCancellation] CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand All @@ -365,7 +369,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;
Expand Down Expand Up @@ -429,8 +433,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 =>
{
Expand All @@ -456,9 +460,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;
}
}
}
}
Expand All @@ -470,7 +479,7 @@ await AppendFixesOrConfigurationsAsync(
}
}

return;
yield break;

void AddAllFixers(
ImmutableArray<CodeFixProvider> fixers,
Expand Down Expand Up @@ -590,45 +599,46 @@ static ImmutableArray<Diagnostic> FilterApplicableDiagnostics(
}
}

private async Task AppendConfigurationsAsync(
private async IAsyncEnumerable<CodeFixCollection> StreamConfigurationFixesAsync(
Document document,
TextSpan diagnosticsSpan,
IEnumerable<DiagnosticData> diagnostics,
ArrayBuilder<CodeFixCollection> result,
PooledHashSet<string> registeredConfigurationFixTitles,
CancellationToken cancellationToken)
[EnumeratorCancellation] CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

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
foreach (var provider in lazyConfigurationProviders.Value)
{
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 =>
{
var fixes = await provider.GetFixesAsync(document, diagnosticsSpan, dxs, cancellationToken).ConfigureAwait(false);
return fixes.WhereAsArray(f => registeredConfigurationFixTitles.Add(f.Action.Title));
},
cancellationToken).ConfigureAwait(false);
if (codeFixCollection != null)
yield return codeFixCollection;
}
}
}

private async Task AppendFixesOrConfigurationsAsync<TCodeFixProvider>(
private async Task<CodeFixCollection?> TryGetFixesOrConfigurationsAsync<TCodeFixProvider>(
Document document,
TextSpan fixesSpan,
IEnumerable<DiagnosticData> diagnosticsWithSameSpan,
bool fixAllForInSpan,
ArrayBuilder<CodeFixCollection> result,
TCodeFixProvider fixer,
Func<Diagnostic, bool> hasFix,
Func<ImmutableArray<Diagnostic>, Task<ImmutableArray<CodeFix>>> getFixes,
Expand All @@ -644,7 +654,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<IExtensionManager>();
Expand All @@ -653,9 +663,7 @@ await diagnosticsWithSameSpan.OrderByDescending(d => d.Severity)
defaultValue: ImmutableArray<CodeFix>.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(
Expand Down Expand Up @@ -704,10 +712,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);
}

/// <summary> Looks explicitly for an <see cref="AbstractSuppressionCodeFixProvider"/>.</summary>
Expand Down
11 changes: 9 additions & 2 deletions src/Features/Core/Portable/CodeFixes/ICodeFixService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
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;

namespace Microsoft.CodeAnalysis.CodeFixes
{
internal interface ICodeFixService
{
Task<ImmutableArray<CodeFixCollection>> GetFixesAsync(Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func<string, IDisposable?> addOperationScope, CancellationToken cancellationToken);
IAsyncEnumerable<CodeFixCollection> StreamFixesAsync(Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func<string, IDisposable?> addOperationScope, CancellationToken cancellationToken);
Task<CodeFixCollection?> GetDocumentFixAllForIdInSpanAsync(Document document, TextSpan textSpan, string diagnosticId, CodeActionOptions options, CancellationToken cancellationToken);
Task<Document> ApplyCodeFixesForSpecificDiagnosticIdAsync(Document document, string diagnosticId, IProgressTracker progressTracker, CodeActionOptions options, CancellationToken cancellationToken);
CodeFixProvider? GetSuppressionFixer(string language, IEnumerable<string> diagnosticIds);
Expand All @@ -24,7 +25,13 @@ internal interface ICodeFixService

internal static class ICodeFixServiceExtensions
{
public static IAsyncEnumerable<CodeFixCollection> 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<ImmutableArray<CodeFixCollection>> 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<ImmutableArray<CodeFixCollection>> GetFixesAsync(this ICodeFixService service, Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func<string, IDisposable?> addOperationScope, CancellationToken cancellationToken)
=> service.StreamFixesAsync(document, textSpan, priority, options, addOperationScope, cancellationToken).ToImmutableArrayAsync();
}
}
Original file line number Diff line number Diff line change
@@ -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<ImmutableArray<T>> ToImmutableArrayAsync<T>(this IAsyncEnumerable<T> values)
{
using var _ = ArrayBuilder<T>.GetInstance(out var result);
await foreach (var value in values.ConfigureAwait(false))
result.Add(value);

return result.ToImmutable();
}
}
}