Skip to content

Commit

Permalink
Merge pull request #59585 from CyrusNajmabadi/asyncFixes
Browse files Browse the repository at this point in the history 
Allow codefixservice to stream codefixes as it is computed, instead of waiting for all fixes to be ready.
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Feb 17, 2022
2 parents dcc6313 + 5fffe74 commit 48bf2d8
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 45 deletions.
93 changes: 50 additions & 43 deletions src/Features/Core/Portable/CodeFixes/CodeFixService.cs
View file
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
View file
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();
}
}
24 changes: 24 additions & 0 deletions src/Workspaces/Core/Portable/Shared/Extensions/IAsyncEnumerableExtensions.cs
View file
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();
}
}
}

0 comments on commit 48bf2d8

Please sign in to comment.