Skip to content

Commit

Permalink
Merge pull request #59599 from CyrusNajmabadi/firstFix
Browse files Browse the repository at this point in the history
Use the same codepath for computing light-bulb actions to determine if hte lightbulb shoudl appear at all.
  • Loading branch information
CyrusNajmabadi authored Mar 4, 2022
2 parents ec27179 + 8f4c7f0 commit 372547b
Show file tree
Hide file tree
Showing 14 changed files with 227 additions and 249 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
45 changes: 30 additions & 15 deletions src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string?> 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<string?> TryGetRefactoringSuggestedActionCategoryAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,6 +37,11 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.Suggestions
[SuggestedActionPriority(DefaultOrderings.Lowest)]
internal partial class SuggestedActionsSourceProvider : ISuggestedActionsSourceProvider
{
public static readonly ImmutableArray<string> 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");
Expand Down Expand Up @@ -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,
};
}
}
6 changes: 4 additions & 2 deletions src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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!;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(Sol
public Task<ImmutableArray<DiagnosticData>> GetSpecificCachedDiagnosticsAsync(Workspace workspace, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
=> throw new NotImplementedException();

public Task<bool> TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, ArrayBuilder<DiagnosticData> diagnostics, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
public Task<(ImmutableArray<DiagnosticData> diagnostics, bool upToDate)> TryGetDiagnosticsForSpanAsync(Document document, TextSpan range, Func<string, bool>? shouldIncludeDiagnostic, bool includeSuppressedDiagnostics = false, CodeActionRequestPriority priority = CodeActionRequestPriority.None, CancellationToken cancellationToken = default)
=> throw new NotImplementedException();
}
}
Original file line number Diff line number Diff line change
@@ -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<SuggestedActionPriorityAttribute>()
.ToImmutableArray();
AssertEx.SetEqual(attributes.Select(a => a.Priority), SuggestedActionsSourceProvider.Orderings);
}
}
}
13 changes: 5 additions & 8 deletions src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 372547b

Please sign in to comment.