diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/AsyncSuggestedActionsSource.cs b/src/EditorFeatures/Core.Wpf/Suggestions/AsyncSuggestedActionsSource.cs index cb22c62204420..d95cb66d77b25 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/AsyncSuggestedActionsSource.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/AsyncSuggestedActionsSource.cs @@ -102,13 +102,7 @@ private async Task GetSuggestedActionsWorkerAsync( // Collectors are in priority order. So just walk them from highest to lowest. foreach (var collector in collectors) { - var priority = collector.Priority switch - { - VisualStudio.Utilities.DefaultOrderings.Highest => CodeActionRequestPriority.High, - VisualStudio.Utilities.DefaultOrderings.Default => CodeActionRequestPriority.Normal, - VisualStudio.Utilities.DefaultOrderings.Lowest => CodeActionRequestPriority.Lowest, - _ => (CodeActionRequestPriority?)null, - }; + var priority = TryGetPriority(collector.Priority); if (priority != null) { diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs index 332cf7daf6362..1b5d86630fdce 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs @@ -417,27 +417,42 @@ await InvokeBelowInputPriorityAsync(() => CodeActionOptions options, CancellationToken cancellationToken) { - if (state.Target.Owner._codeFixService != null && - state.Target.SubjectBuffer.SupportsCodeFixes()) + foreach (var order in Orderings) { - var result = await state.Target.Owner._codeFixService.GetMostSevereFixableDiagnosticAsync( - document, range.Span.ToTextSpan(), options, cancellationToken).ConfigureAwait(false); + var priority = TryGetPriority(order); + Contract.ThrowIfNull(priority); - if (result.HasFix) - { - Logger.Log(FunctionId.SuggestedActions_HasSuggestedActionsAsync); - return GetFixCategory(result.Diagnostic.Severity); - } + var result = await GetFixLevelAsync(priority.Value).ConfigureAwait(false); + if (result != null) + return result; + } - if (result.PartialResult) + return null; + + async Task GetFixLevelAsync(CodeActionRequestPriority priority) + { + if (state.Target.Owner._codeFixService != null && + state.Target.SubjectBuffer.SupportsCodeFixes()) { - // reset solution version number so that we can raise suggested action changed event - Volatile.Write(ref state.Target.LastSolutionVersionReported, InvalidSolutionVersion); - return null; + var result = await state.Target.Owner._codeFixService.GetMostSevereFixAsync( + document, range.Span.ToTextSpan(), priority, options, cancellationToken).ConfigureAwait(false); + + if (result.HasFix) + { + Logger.Log(FunctionId.SuggestedActions_HasSuggestedActionsAsync); + return GetFixCategory(result.CodeFixCollection.FirstDiagnostic.Severity); + } + + if (!result.UpToDate) + { + // reset solution version number so that we can raise suggested action changed event + Volatile.Write(ref state.Target.LastSolutionVersionReported, InvalidSolutionVersion); + return null; + } } - } - return null; + return null; + } } private async Task TryGetRefactoringSuggestedActionCategoryAsync( diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSourceProvider.cs b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSourceProvider.cs index a99c015036d7e..25e25897204d1 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSourceProvider.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSourceProvider.cs @@ -6,18 +6,18 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.ComponentModel.Composition; +using System.Linq; +using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Editor.Tags; -using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Shared.Utilities; -using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Language.Intellisense; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; @@ -37,6 +37,11 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.Suggestions [SuggestedActionPriority(DefaultOrderings.Lowest)] internal partial class SuggestedActionsSourceProvider : ISuggestedActionsSourceProvider { + public static readonly ImmutableArray Orderings = ImmutableArray.Create( + DefaultOrderings.Highest, + DefaultOrderings.Default, + DefaultOrderings.Lowest); + private static readonly Guid s_CSharpSourceGuid = new Guid("b967fea8-e2c3-4984-87d4-71a38f49e16a"); private static readonly Guid s_visualBasicSourceGuid = new Guid("4de30e93-3e0c-40c2-a4ba-1124da4539f6"); private static readonly Guid s_xamlSourceGuid = new Guid("a0572245-2eab-4c39-9f61-06a6d8c5ddda"); @@ -100,5 +105,14 @@ public SuggestedActionsSourceProvider( ? new AsyncSuggestedActionsSource(_threadingContext, _globalOptions, this, textView, textBuffer, _suggestedActionCategoryRegistry) : new SyncSuggestedActionsSource(_threadingContext, _globalOptions, this, textView, textBuffer, _suggestedActionCategoryRegistry); } + + private static CodeActionRequestPriority? TryGetPriority(string priority) + => priority switch + { + DefaultOrderings.Highest => CodeActionRequestPriority.High, + DefaultOrderings.Default => CodeActionRequestPriority.Normal, + DefaultOrderings.Lowest => CodeActionRequestPriority.Lowest, + _ => (CodeActionRequestPriority?)null, + }; } } diff --git a/src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs b/src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs index d29746f7892a1..ba421e1ecddd6 100644 --- a/src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs +++ b/src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs @@ -60,7 +60,8 @@ public async Task TestGetFirstDiagnosticWithFixAsync() var reference = new MockAnalyzerReference(); var project = workspace.CurrentSolution.Projects.Single().AddAnalyzerReference(reference); var document = project.Documents.Single(); - var unused = await fixService.GetMostSevereFixableDiagnosticAsync(document, TextSpan.FromBounds(0, 0), CodeActionOptions.Default, CancellationToken.None); + var unused = await fixService.GetMostSevereFixAsync( + document, TextSpan.FromBounds(0, 0), CodeActionRequestPriority.None, CodeActionOptions.Default, CancellationToken.None); var fixer1 = (MockFixer)fixers.Single().Value; var fixer2 = (MockFixer)reference.Fixer!; @@ -297,7 +298,8 @@ private static async Task GetFirstDiagnosticWithFixWithExceptionValidationAsync( errorReportingService.OnError = message => errorReported = true; GetDocumentAndExtensionManager(tuple.analyzerService, workspace, out var document, out var extensionManager); - var unused = await tuple.codeFixService.GetMostSevereFixableDiagnosticAsync(document, TextSpan.FromBounds(0, 0), CodeActionOptions.Default, CancellationToken.None); + var unused = await tuple.codeFixService.GetMostSevereFixAsync( + document, TextSpan.FromBounds(0, 0), CodeActionRequestPriority.None, CodeActionOptions.Default, CancellationToken.None); Assert.True(extensionManager.IsDisabled(codefix)); Assert.False(extensionManager.IsIgnored(codefix)); Assert.True(errorReported); diff --git a/src/EditorFeatures/Test/EditAndContinue/Helpers/MockDiagnosticAnalyzerService.cs b/src/EditorFeatures/Test/EditAndContinue/Helpers/MockDiagnosticAnalyzerService.cs index 5926e8873d0ca..c652fbf59aa6d 100644 --- a/src/EditorFeatures/Test/EditAndContinue/Helpers/MockDiagnosticAnalyzerService.cs +++ b/src/EditorFeatures/Test/EditAndContinue/Helpers/MockDiagnosticAnalyzerService.cs @@ -50,7 +50,7 @@ public Task> GetProjectDiagnosticsForIdsAsync(Sol public Task> GetSpecificCachedDiagnosticsAsync(Workspace workspace, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, ArrayBuilder diagnostics, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + public Task<(ImmutableArray diagnostics, bool upToDate)> TryGetDiagnosticsForSpanAsync(Document document, TextSpan range, Func? shouldIncludeDiagnostic, bool includeSuppressedDiagnostics = false, CodeActionRequestPriority priority = CodeActionRequestPriority.None, CancellationToken cancellationToken = default) => throw new NotImplementedException(); } } diff --git a/src/EditorFeatures/Test/SuggestedActions/SuggestedActionSourceProviderTests.cs b/src/EditorFeatures/Test/SuggestedActions/SuggestedActionSourceProviderTests.cs new file mode 100644 index 0000000000000..84806cc7953e2 --- /dev/null +++ b/src/EditorFeatures/Test/SuggestedActions/SuggestedActionSourceProviderTests.cs @@ -0,0 +1,26 @@ +// 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.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis.Editor.Implementation.Suggestions; +using Microsoft.VisualStudio.Language.Intellisense; +using Roslyn.Test.Utilities; +using Xunit; + +namespace Microsoft.CodeAnalysis.Editor.UnitTests.SuggestedActions +{ + public class SuggestedActionSourceProviderTests + { + [Fact] + public void EnsureAttributesMatchData() + { + // Ensure that the list of orderings on this type matches the set we expose in SuggestedActionsSourceProvider.Orderings + var attributes = typeof(SuggestedActionsSourceProvider).GetCustomAttributes(inherit: false) + .OfType() + .ToImmutableArray(); + AssertEx.SetEqual(attributes.Select(a => a.Priority), SuggestedActionsSourceProvider.Orderings); + } + } +} diff --git a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb index e565227170f1e..8bcb00f82195c 100644 --- a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb +++ b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb @@ -2223,16 +2223,14 @@ class C Dim incrementalAnalyzer = diagnosticService.CreateIncrementalAnalyzer(workspace) ' Verify diagnostics for span - Dim diagnostics As New PooledObjects.ArrayBuilder(Of DiagnosticData) - Await diagnosticService.TryAppendDiagnosticsForSpanAsync(document, span, diagnostics) - Dim diagnostic = Assert.Single(diagnostics) + Dim t = Await diagnosticService.TryGetDiagnosticsForSpanAsync(document, span, shouldIncludeDiagnostic:=Nothing) + Dim diagnostic = Assert.Single(t.diagnostics) Assert.Equal("CS0219", diagnostic.Id) ' Verify no diagnostics outside the local decl span span = localDecl.GetLastToken().GetNextToken().GetNextToken().Span - diagnostics.Clear() - Await diagnosticService.TryAppendDiagnosticsForSpanAsync(document, span, diagnostics) - Assert.Empty(diagnostics) + t = Await diagnosticService.TryGetDiagnosticsForSpanAsync(document, span, shouldIncludeDiagnostic:=Nothing) + Assert.Empty(t.diagnostics) End Using End Function @@ -2322,8 +2320,7 @@ class MyClass Assert.Equal(analyzer.Descriptor.Id, descriptors.Single().Id) ' Try get diagnostics for span - Dim diagnostics As New PooledObjects.ArrayBuilder(Of DiagnosticData) - Await diagnosticService.TryAppendDiagnosticsForSpanAsync(document, span, diagnostics) + Await diagnosticService.TryGetDiagnosticsForSpanAsync(document, span, shouldIncludeDiagnostic:=Nothing) ' Verify only span-based analyzer is invoked with TryAppendDiagnosticsForSpanAsync Assert.Equal(isSpanBasedAnalyzer, analyzer.ReceivedOperationCallback) diff --git a/src/Features/Core/Portable/CodeFixes/CodeFixService.cs b/src/Features/Core/Portable/CodeFixes/CodeFixService.cs index 401517ac5ebe0..12ce0be23aaa1 100644 --- a/src/Features/Core/Portable/CodeFixes/CodeFixService.cs +++ b/src/Features/Core/Portable/CodeFixes/CodeFixService.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; +using System.Configuration; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -34,9 +35,6 @@ namespace Microsoft.CodeAnalysis.CodeFixes [Export(typeof(ICodeFixService)), Shared] internal partial class CodeFixService : ICodeFixService { - private static readonly Comparison s_diagnosticDataComparisonById = - new((d1, d2) => DiagnosticId.CompareOrdinal(d1.Id, d2.Id)); - private readonly IDiagnosticAnalyzerService _diagnosticService; private readonly ImmutableArray> _fixers; private readonly ImmutableDictionary>> _fixersPerLanguageMap; @@ -73,73 +71,83 @@ public CodeFixService( _configurationProvidersMap = GetConfigurationProvidersPerLanguageMap(configurationProviders); } - public async Task GetMostSevereFixableDiagnosticAsync( - Document document, TextSpan range, CodeActionOptions options, CancellationToken cancellationToken) + private Func? GetShouldIncludeDiagnosticPredicate( + Document document, + CodeActionRequestPriority priority) { - cancellationToken.ThrowIfCancellationRequested(); + // For Normal or Low priority, we only need to execute analyzers which can report at least one fixable + // diagnostic that can have a non-suppression/configuration fix. + // + // For CodeActionPriorityRequest.High, we only run compiler analyzer, which always has fixable diagnostics, + // so we can return a null predicate here to include all diagnostics. + + if (!(priority is CodeActionRequestPriority.Normal or CodeActionRequestPriority.Low)) + return null; + + var hasWorkspaceFixers = TryGetWorkspaceFixersMap(document, out var workspaceFixersMap); + var projectFixersMap = GetProjectFixers(document.Project); - if (!document.IsOpen()) + return id => { - return default; - } + if (hasWorkspaceFixers && workspaceFixersMap!.Value.ContainsKey(id)) + return true; - using var _ = ArrayBuilder.GetInstance(out var diagnostics); - using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + return projectFixersMap.ContainsKey(id); + }; + } + + public async Task GetMostSevereFixAsync( + Document document, TextSpan range, CodeActionRequestPriority priority, CodeActionOptions options, CancellationToken cancellationToken) + { + var (allDiagnostics, upToDate) = await _diagnosticService.TryGetDiagnosticsForSpanAsync( + document, range, GetShouldIncludeDiagnosticPredicate(document, priority), + includeSuppressedDiagnostics: false, priority, cancellationToken).ConfigureAwait(false); + + var spanToDiagnostics = ConvertToMap(allDiagnostics); + using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); var linkedToken = linkedTokenSource.Token; - // This flag is used by SuggestedActionsSource to track what solution is was - // last able to get "full results" for. - var isFullResult = await _diagnosticService.TryAppendDiagnosticsForSpanAsync( - document, range, diagnostics, cancellationToken: linkedToken).ConfigureAwait(false); + var spanToErrorDiagnostics = new SortedDictionary>(); + var spanToOtherDiagnostics = new SortedDictionary>(); - var errorDiagnostics = diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error); - var otherDiagnostics = diagnostics.Where(d => d.Severity != DiagnosticSeverity.Error); + foreach (var (span, diagnostics) in spanToDiagnostics) + { + foreach (var diagnostic in diagnostics) + { + var preferredMap = diagnostic.Severity == DiagnosticSeverity.Error + ? spanToErrorDiagnostics + : spanToOtherDiagnostics; - // Kick off a task that will determine there's an Error Diagnostic with a fixer - var errorDiagnosticsTask = Task.Run( - () => GetFirstDiagnosticWithFixAsync(document, errorDiagnostics, range, options, linkedToken), - linkedToken); + preferredMap.MultiAdd(span, diagnostic); + } + } - // Kick off a task that will determine if any non-Error Diagnostic has a fixer - var otherDiagnosticsTask = Task.Run( - () => GetFirstDiagnosticWithFixAsync(document, otherDiagnostics, range, options, linkedToken), - linkedToken); + var errorFixTask = Task.Run(() => GetFirstFixAsync(spanToErrorDiagnostics, cancellationToken), cancellationToken); + var otherFixTask = Task.Run(() => GetFirstFixAsync(spanToOtherDiagnostics, linkedToken), linkedToken); // If the error diagnostics task happens to complete with a non-null result before // the other diagnostics task, we can cancel the other task. - var diagnostic = await errorDiagnosticsTask.ConfigureAwait(false) - ?? await otherDiagnosticsTask.ConfigureAwait(false); + var collection = await errorFixTask.ConfigureAwait(false) ?? + await otherFixTask.ConfigureAwait(false); linkedTokenSource.Cancel(); - return new FirstDiagnosticResult(partialResult: !isFullResult, - hasFix: diagnostic != null, - diagnostic: diagnostic); - } - - private async Task GetFirstDiagnosticWithFixAsync( - Document document, - IEnumerable severityGroup, - TextSpan range, - CodeActionOptions options, - CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); + return new FirstFixResult(upToDate, collection); - foreach (var diagnostic in severityGroup) + async Task GetFirstFixAsync( + SortedDictionary> spanToDiagnostics, + CancellationToken cancellationToken) { - if (!range.IntersectsWith(diagnostic.GetTextSpan())) + await foreach (var collection in StreamFixesAsync( + document, spanToDiagnostics, fixAllForInSpan: false, + priority, options, _ => null, cancellationToken).ConfigureAwait(false)) { - continue; + // Stop at the result error we see. + return collection; } - if (await ContainsAnyFixAsync(document, diagnostic, options, cancellationToken).ConfigureAwait(false)) - { - return diagnostic; - } + return null; } - - return null; } public async IAsyncEnumerable StreamFixesAsync( @@ -150,57 +158,32 @@ public async IAsyncEnumerable StreamFixesAsync( Func addOperationScope, [EnumeratorCancellation] CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - - // 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 - // (if it is up-to-date) or synchronously do the work at the spot. - // - // this design's weakness is that each side don't have enough information to narrow down works to do. it will most likely always do more works than needed. - // sometimes way more than it is needed. (compilation) - - // group diagnostics by their diagnostics span - // 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. - var aggregatedDiagnostics = new SortedDictionary>(); - - // For 'CodeActionPriorityRequest.Normal' or 'CodeActionPriorityRequest.Low', we do not compute suppression/configuration fixes, - // those fixes have a dedicated request priority 'CodeActionPriorityRequest.Lowest'. - // Hence, for Normal or Low priority, we only need to execute analyzers which can report at least one fixable diagnostic - // that can have a non-suppression/configuration fix. - // Note that for 'CodeActionPriorityRequest.High', we only run compiler analyzer, which always has fixable diagnostics, - // so we can pass in null. - var shouldIncludeDiagnostic = priority is CodeActionRequestPriority.Normal or CodeActionRequestPriority.Low - ? GetFixableDiagnosticFilter(document) - : null; - // 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; - var diagnostics = await _diagnosticService.GetDiagnosticsForSpanAsync( - document, range, shouldIncludeDiagnostic, includeSuppressionFixes, priority, addOperationScope, cancellationToken).ConfigureAwait(false); - foreach (var diagnostic in diagnostics) - { - if (diagnostic.IsSuppressed) - continue; + // 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 (if it is up-to-date) or + // synchronously do the work at the spot. + // + // this design's weakness is that each side don't have enough information to narrow down works to do. it + // will most likely always do more works than needed. sometimes way more than it is needed. (compilation) - var list = aggregatedDiagnostics.GetOrAdd(diagnostic.GetTextSpan(), static _ => new List()); - list.Add(diagnostic); - } + var diagnostics = await _diagnosticService.GetDiagnosticsForSpanAsync( + document, range, GetShouldIncludeDiagnosticPredicate(document, priority), + includeSuppressionFixes, priority, addOperationScope, cancellationToken).ConfigureAwait(false); - if (aggregatedDiagnostics.Count == 0) + if (diagnostics.IsEmpty) yield break; - // Order diagnostics by DiagnosticId so the fixes are in a deterministic order. - foreach (var (_, diagnosticList) in aggregatedDiagnostics) - diagnosticList.Sort(s_diagnosticDataComparisonById); + var spanToDiagnostics = ConvertToMap(diagnostics); // 'CodeActionRequestPriority.Lowest' is used when the client only wants suppression/configuration fixes. if (priority != CodeActionRequestPriority.Lowest) { await foreach (var collection in StreamFixesAsync( - document, aggregatedDiagnostics, fixAllForInSpan: false, + document, spanToDiagnostics, fixAllForInSpan: false, priority, options, addOperationScope, cancellationToken).ConfigureAwait(false)) { yield return collection; @@ -212,7 +195,7 @@ public async IAsyncEnumerable StreamFixesAsync( { // Ensure that we do not register duplicate configuration fixes. using var _2 = PooledHashSet.GetInstance(out var registeredConfigurationFixTitles); - foreach (var (span, diagnosticList) in aggregatedDiagnostics) + foreach (var (span, diagnosticList) in spanToDiagnostics) { await foreach (var codeFixCollection in StreamConfigurationFixesAsync( document, span, diagnosticList, registeredConfigurationFixTitles, options, cancellationToken).ConfigureAwait(false)) @@ -221,23 +204,29 @@ public async IAsyncEnumerable StreamFixesAsync( } } } + } - yield break; - - // Local functions - Func GetFixableDiagnosticFilter(Document document) + private static SortedDictionary> ConvertToMap( + ImmutableArray diagnostics) + { + // group diagnostics by their diagnostics span + // + // 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. + var spanToDiagnostics = new SortedDictionary>(); + foreach (var diagnostic in diagnostics) { - var hasWorkspaceFixers = TryGetWorkspaceFixersMap(document, out var workspaceFixersMap); - var projectFixersMap = GetProjectFixers(document.Project); - - return id => - { - if (hasWorkspaceFixers && workspaceFixersMap!.Value.ContainsKey(id)) - return true; + if (diagnostic.IsSuppressed) + continue; - return projectFixersMap.ContainsKey(id); - }; + spanToDiagnostics.MultiAdd(diagnostic.GetTextSpan(), diagnostic); } + + // Order diagnostics by DiagnosticId so the fixes are in a deterministic order. + foreach (var (_, diagnosticList) in spanToDiagnostics) + diagnosticList.Sort(static (d1, d2) => DiagnosticId.CompareOrdinal(d1.Id, d2.Id)); + + return spanToDiagnostics; } public async Task GetDocumentFixAllForIdInSpanAsync( @@ -494,7 +483,7 @@ void AddAllFixers( continue; allFixers.Add(fixer); - fixerToRangesAndDiagnostics.GetOrAdd(fixer, static _ => new()).Add((range, diagnostics)); + fixerToRangesAndDiagnostics.MultiAdd(fixer, (range, diagnostics)); } } } @@ -779,83 +768,6 @@ private async Task> GetProjectDiagnosticsAsync(Project p } } - private async Task ContainsAnyFixAsync( - Document document, DiagnosticData diagnosticData, CodeActionOptions options, CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); - - var workspaceFixers = ImmutableArray.Empty; - var hasAnySharedFixer = TryGetWorkspaceFixersMap(document, out var fixerMap) && fixerMap.Value.TryGetValue(diagnosticData.Id, out workspaceFixers); - var hasAnyProjectFixer = GetProjectFixers(document.Project).TryGetValue(diagnosticData.Id, out var projectFixers); - - // TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict CodeFixes in Interactive - if (hasAnySharedFixer && document.Project.Solution.Workspace.Kind == WorkspaceKind.Interactive) - { - workspaceFixers = workspaceFixers.WhereAsArray(IsInteractiveCodeFixProvider); - hasAnySharedFixer = workspaceFixers.Any(); - } - - var hasConfigurationFixer = - _configurationProvidersMap.TryGetValue(document.Project.Language, out var lazyConfigurationProviders) && - !lazyConfigurationProviders.Value.IsDefaultOrEmpty; - - if (!hasAnySharedFixer && !hasAnyProjectFixer && !hasConfigurationFixer) - { - return false; - } - - var allFixers = ImmutableArray.Empty; - if (hasAnySharedFixer) - { - allFixers = workspaceFixers; - } - - if (hasAnyProjectFixer) - { - allFixers = allFixers.AddRange(projectFixers!); - } - - var diagnostic = await diagnosticData.ToDiagnosticAsync(document.Project, cancellationToken).ConfigureAwait(false); - - if (hasConfigurationFixer) - { - foreach (var lazyConfigurationProvider in lazyConfigurationProviders!.Value) - { - if (lazyConfigurationProvider.IsFixableDiagnostic(diagnostic)) - { - return true; - } - } - } - - var fixes = new List(); - var context = new CodeFixContext(document, diagnostic.Location.SourceSpan, ImmutableArray.Create(diagnostic), - - // TODO: Can we share code between similar lambdas that we pass to this API in BatchFixAllProvider.cs, CodeFixService.cs and CodeRefactoringService.cs? - (action, applicableDiagnostics) => - { - // Serialize access for thread safety - we don't know what thread the fix provider will call this delegate from. - lock (fixes) - { - fixes.Add(new CodeFix(document.Project, action, applicableDiagnostics)); - } - }, - options, - cancellationToken); - - var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService(); - - // we do have fixer. now let's see whether it actually can fix it - foreach (var fixer in allFixers) - { - await extensionManager.PerformActionAsync(fixer, () => fixer.RegisterCodeFixesAsync(context) ?? Task.CompletedTask).ConfigureAwait(false); - if (fixes.Count > 0) - return true; - } - - return false; - } - private bool IsInteractiveCodeFixProvider(CodeFixProvider provider) { // TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict CodeFixes in Interactive diff --git a/src/Features/Core/Portable/CodeFixes/FirstDiagnosticResult.cs b/src/Features/Core/Portable/CodeFixes/FirstDiagnosticResult.cs deleted file mode 100644 index eab1bc7048bc8..0000000000000 --- a/src/Features/Core/Portable/CodeFixes/FirstDiagnosticResult.cs +++ /dev/null @@ -1,24 +0,0 @@ -// 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. - -#nullable disable - -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Microsoft.CodeAnalysis.CodeFixes -{ - internal readonly struct FirstDiagnosticResult - { - public readonly bool PartialResult; - public readonly bool HasFix; - public readonly DiagnosticData Diagnostic; - - public FirstDiagnosticResult(bool partialResult, bool hasFix, DiagnosticData diagnostic) - { - PartialResult = partialResult; - HasFix = hasFix; - Diagnostic = diagnostic; - } - } -} diff --git a/src/Features/Core/Portable/CodeFixes/FirstFixResult.cs b/src/Features/Core/Portable/CodeFixes/FirstFixResult.cs new file mode 100644 index 0000000000000..83674aa0355f8 --- /dev/null +++ b/src/Features/Core/Portable/CodeFixes/FirstFixResult.cs @@ -0,0 +1,23 @@ +// 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.Diagnostics.CodeAnalysis; + +namespace Microsoft.CodeAnalysis.CodeFixes +{ + internal readonly struct FirstFixResult + { + public readonly bool UpToDate; + public readonly CodeFixCollection? CodeFixCollection; + + [MemberNotNullWhen(true, nameof(CodeFixCollection))] + public bool HasFix => CodeFixCollection != null; + + public FirstFixResult(bool upToDate, CodeFixCollection? codeFixCollection) + { + UpToDate = upToDate; + CodeFixCollection = codeFixCollection; + } + } +} diff --git a/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs b/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs index 1f2109bc9fe6c..a23d3675febdd 100644 --- a/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs +++ b/src/Features/Core/Portable/CodeFixes/ICodeFixService.cs @@ -17,10 +17,17 @@ namespace Microsoft.CodeAnalysis.CodeFixes internal interface ICodeFixService { IAsyncEnumerable StreamFixesAsync(Document document, TextSpan textSpan, CodeActionRequestPriority priority, CodeActionOptions options, Func addOperationScope, CancellationToken cancellationToken); + + /// + /// Similar to except that instead of streaming all results, this ends with the + /// first. This will also attempt to return a fix for an error first, but will fall back to any fix if that + /// does not succeed. + /// + Task GetMostSevereFixAsync(Document document, TextSpan range, CodeActionRequestPriority priority, CodeActionOptions options, 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); - Task GetMostSevereFixableDiagnosticAsync(Document document, TextSpan range, CodeActionOptions options, CancellationToken cancellationToken); } internal static class ICodeFixServiceExtensions diff --git a/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService.cs b/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService.cs index 0167fa8aa2ab9..6cd64ed6fb3d1 100644 --- a/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService.cs +++ b/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService.cs @@ -64,16 +64,29 @@ public void Reanalyze(Workspace workspace, IEnumerable? projectIds = } } - public Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, ArrayBuilder diagnostics, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + public Task<(ImmutableArray diagnostics, bool upToDate)> TryGetDiagnosticsForSpanAsync( + Document document, + TextSpan range, + Func? shouldIncludeDiagnostic, + bool includeSuppressedDiagnostics = false, + CodeActionRequestPriority priority = CodeActionRequestPriority.None, + CancellationToken cancellationToken = default) { if (_map.TryGetValue(document.Project.Solution.Workspace, out var analyzer)) { // always make sure that analyzer is called on background thread. - return Task.Run(() => analyzer.TryAppendDiagnosticsForSpanAsync( - document, range, diagnostics, shouldIncludeDiagnostic: null, includeSuppressedDiagnostics, CodeActionRequestPriority.None, blockForData: false, addOperationScope: null, cancellationToken), cancellationToken); + return Task.Run(async () => + { + using var _ = ArrayBuilder.GetInstance(out var diagnostics); + var upToDate = await analyzer.TryAppendDiagnosticsForSpanAsync( + document, range, diagnostics, shouldIncludeDiagnostic, + includeSuppressedDiagnostics, priority, blockForData: false, + addOperationScope: null, cancellationToken).ConfigureAwait(false); + return (diagnostics.ToImmutable(), upToDate); + }, cancellationToken); } - return SpecializedTasks.False; + return Task.FromResult((ImmutableArray.Empty, upToDate: false)); } public Task> GetDiagnosticsForSpanAsync( diff --git a/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs b/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs index ae6dff7a009ee..b25c464875796 100644 --- a/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs +++ b/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs @@ -8,7 +8,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Diagnostics @@ -75,7 +74,7 @@ internal interface IDiagnosticAnalyzerService /// Use /// if you want to force complete all analyzers and get up-to-date diagnostics for all analyzers for the given span. /// - Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, ArrayBuilder diagnostics, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default); + Task<(ImmutableArray diagnostics, bool upToDate)> TryGetDiagnosticsForSpanAsync(Document document, TextSpan range, Func? shouldIncludeDiagnostic, bool includeSuppressedDiagnostics = false, CodeActionRequestPriority priority = CodeActionRequestPriority.None, CancellationToken cancellationToken = default); /// /// Return up to date diagnostics for the given span for the document diff --git a/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb index 240b7ac2dea93..bcd7d41bf5e83 100644 --- a/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb +++ b/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb @@ -523,10 +523,6 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Return SpecializedTasks.EmptyImmutableArray(Of DiagnosticData) End Function - Public Function TryAppendDiagnosticsForSpanAsync(document As Document, range As TextSpan, diagnostics As ArrayBuilder(Of DiagnosticData), Optional includeSuppressedDiagnostics As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As Task(Of Boolean) Implements IDiagnosticAnalyzerService.TryAppendDiagnosticsForSpanAsync - Return Task.FromResult(False) - End Function - Public Function GetSpecificCachedDiagnosticsAsync(workspace As Workspace, id As Object, Optional includeSuppressedDiagnostics As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As Task(Of ImmutableArray(Of DiagnosticData)) Implements IDiagnosticAnalyzerService.GetSpecificCachedDiagnosticsAsync Return SpecializedTasks.EmptyImmutableArray(Of DiagnosticData)() End Function @@ -554,6 +550,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Public Function ForceAnalyzeAsync(solution As Solution, onProjectAnalyzed As Action(Of Project), Optional projectId As ProjectId = Nothing, Optional cancellationToken As CancellationToken = Nothing) As Task Implements IDiagnosticAnalyzerService.ForceAnalyzeAsync Throw New NotImplementedException() End Function + + Public Function TryGetDiagnosticsForSpanAsync(document As Document, range As TextSpan, shouldIncludeDiagnostic As Func(Of String, Boolean), Optional includeSuppressedDiagnostics As Boolean = False, Optional priority As CodeActionRequestPriority = CodeActionRequestPriority.None, Optional cancellationToken As CancellationToken = Nothing) As Task(Of (diagnostics As ImmutableArray(Of DiagnosticData), upToDate As Boolean)) Implements IDiagnosticAnalyzerService.TryGetDiagnosticsForSpanAsync + Return Task.FromResult((ImmutableArray(Of DiagnosticData).Empty, False)) + End Function End Class End Class End Namespace