From b5e84513662f79d0d13439dff1fa95b1b695090f Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 23 Jun 2021 09:51:58 -0700 Subject: [PATCH] Only require code fix for first iteration Fixes #874 --- .../CodeActionTest`1.cs | 10 ++- .../PublicAPI.Unshipped.txt | 2 +- .../CodeFixTest`1.cs | 10 ++- .../CodeRefactoringTest`1.cs | 5 +- .../CodeFixIterationTests.cs | 61 +++++++++++++++++++ .../TestAnalyzers/LiteralUnderFiveAnalyzer.cs | 6 +- .../TestFixes/IncrementFix.cs | 2 +- 7 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/CodeActionTest`1.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/CodeActionTest`1.cs index 73f93a627..e3542db49 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/CodeActionTest`1.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/CodeActionTest`1.cs @@ -110,16 +110,22 @@ protected static bool HasAnyChange(ProjectState oldState, ProjectState newState, return false; } - protected static CodeAction? TryGetCodeActionToApply(ImmutableArray actions, int? codeActionIndex, string? codeActionEquivalenceKey, Action? codeActionVerifier, IVerifier verifier) + protected static CodeAction? TryGetCodeActionToApply(int iteration, ImmutableArray actions, int? codeActionIndex, string? codeActionEquivalenceKey, Action? codeActionVerifier, IVerifier verifier) { CodeAction? result; if (codeActionIndex.HasValue && codeActionEquivalenceKey != null) { - if (actions.Length <= codeActionIndex) + var expectedAction = actions.FirstOrDefault(action => action.EquivalenceKey == codeActionEquivalenceKey); + if (expectedAction is null && iteration > 0) { + // No matching code action was found. This is acceptable if this is not the first iteration. return null; } + verifier.True( + actions.Length > codeActionIndex, + $"Expected to find a code action at index '{codeActionIndex}', but only '{actions.Length}' code actions were found."); + verifier.Equal( codeActionEquivalenceKey, actions[codeActionIndex.Value].EquivalenceKey, diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt index 42f6732ca..f9d4cbf7d 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt @@ -246,7 +246,7 @@ static Microsoft.CodeAnalysis.Testing.AnalyzerVerifier.VerifyAnalyzerAsync(string source, params Microsoft.CodeAnalysis.Testing.DiagnosticResult[] expected) -> System.Threading.Tasks.Task static Microsoft.CodeAnalysis.Testing.CodeActionTest.CodeActionExpected(Microsoft.CodeAnalysis.Testing.SolutionState state) -> bool static Microsoft.CodeAnalysis.Testing.CodeActionTest.HasAnyChange(Microsoft.CodeAnalysis.Testing.ProjectState oldState, Microsoft.CodeAnalysis.Testing.ProjectState newState, bool recursive) -> bool -static Microsoft.CodeAnalysis.Testing.CodeActionTest.TryGetCodeActionToApply(System.Collections.Immutable.ImmutableArray actions, int? codeActionIndex, string codeActionEquivalenceKey, System.Action codeActionVerifier, Microsoft.CodeAnalysis.Testing.IVerifier verifier) -> Microsoft.CodeAnalysis.CodeActions.CodeAction +static Microsoft.CodeAnalysis.Testing.CodeActionTest.TryGetCodeActionToApply(int iteration, System.Collections.Immutable.ImmutableArray actions, int? codeActionIndex, string codeActionEquivalenceKey, System.Action codeActionVerifier, Microsoft.CodeAnalysis.Testing.IVerifier verifier) -> Microsoft.CodeAnalysis.CodeActions.CodeAction static Microsoft.CodeAnalysis.Testing.DiagnosticResult.CompilerError(string identifier) -> Microsoft.CodeAnalysis.Testing.DiagnosticResult static Microsoft.CodeAnalysis.Testing.DiagnosticResult.CompilerWarning(string identifier) -> Microsoft.CodeAnalysis.Testing.DiagnosticResult static Microsoft.CodeAnalysis.Testing.IVerifierExtensions.EqualOrDiff(this Microsoft.CodeAnalysis.Testing.IVerifier verifier, string expected, string actual, string message = null) -> void diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs index 411b8a0d8..0bc34d839 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs @@ -534,9 +534,12 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV var previousDiagnostics = ImmutableArray.Create<(Project project, Diagnostic diagnostic)>(); + var currentIteration = -1; bool done; do { + currentIteration++; + var analyzerDiagnostics = await GetSortedDiagnosticsAsync(project.Solution, analyzers, additionalDiagnostics: ImmutableArray<(Project project, Diagnostic diagnostic)>.Empty, CompilerDiagnostics, verifier, cancellationToken).ConfigureAwait(false); if (analyzerDiagnostics.Length == 0) { @@ -590,7 +593,7 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV } var filteredActions = FilterCodeActions(actions.ToImmutable()); - var actionToApply = TryGetCodeActionToApply(filteredActions, codeFixIndex, codeFixEquivalenceKey, codeActionVerifier, verifier); + var actionToApply = TryGetCodeActionToApply(currentIteration, filteredActions, codeFixIndex, codeFixEquivalenceKey, codeActionVerifier, verifier); if (actionToApply != null) { anyActions = true; @@ -672,9 +675,12 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV var previousDiagnostics = ImmutableArray.Create<(Project project, Diagnostic diagnostic)>(); + var currentIteration = -1; bool done; do { + currentIteration++; + var analyzerDiagnostics = await GetSortedDiagnosticsAsync(project.Solution, analyzers, additionalDiagnostics: ImmutableArray<(Project project, Diagnostic diagnostic)>.Empty, CompilerDiagnostics, verifier, cancellationToken).ConfigureAwait(false); if (analyzerDiagnostics.Length == 0) { @@ -728,7 +734,7 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV actions.AddRange(FilterCodeActions(actionsBuilder.ToImmutable()).Select(action => (action, codeFixProvider))); } - var actionToApply = TryGetCodeActionToApply(actions.Select(a => a.Item1).ToImmutableArray(), codeFixIndex, codeFixEquivalenceKey, codeActionVerifier, verifier); + var actionToApply = TryGetCodeActionToApply(currentIteration, actions.Select(a => a.Item1).ToImmutableArray(), codeFixIndex, codeFixEquivalenceKey, codeActionVerifier, verifier); if (actionToApply != null) { firstDiagnostic = diagnostic; diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeRefactoring.Testing/CodeRefactoringTest`1.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeRefactoring.Testing/CodeRefactoringTest`1.cs index 80ee3581f..4aa1ca2bb 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeRefactoring.Testing/CodeRefactoringTest`1.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeRefactoring.Testing/CodeRefactoringTest`1.cs @@ -227,9 +227,12 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV numberOfIterations = -numberOfIterations; } + var currentIteration = -1; bool done; do { + currentIteration++; + try { verifier.True(--numberOfIterations >= -1, "The upper limit for the number of code fix iterations was exceeded"); @@ -253,7 +256,7 @@ private async Task VerifyProjectAsync(ProjectState newState, Project project, IV } var filteredActions = FilterCodeActions(actions.ToImmutable()); - var actionToApply = TryGetCodeActionToApply(filteredActions, codeActionIndex, codeActionEquivalenceKey, codeActionVerifier, verifier); + var actionToApply = TryGetCodeActionToApply(currentIteration, filteredActions, codeActionIndex, codeActionEquivalenceKey, codeActionVerifier, verifier); if (actionToApply != null) { anyActions = true; diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing.UnitTests/CodeFixIterationTests.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing.UnitTests/CodeFixIterationTests.cs index 5b38386b7..e50674169 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing.UnitTests/CodeFixIterationTests.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing.UnitTests/CodeFixIterationTests.cs @@ -395,6 +395,67 @@ class TestClass2 { Assert.Equal($"Context: {context}{Environment.NewLine}Expected '2' iterations but found '1' iterations.", exception.Message); } + [Fact] + [WorkItem(874, "https://github.com/dotnet/roslyn-sdk/issues/874")] + public async Task TestTwoIterationsRequiredButOneApplied() + { + var testCode = @" +class TestClass { + int field = [|3|]; +} +"; + var fixedCode = @" +class TestClass { + int field = [|4|]; +} +"; + + await new CSharpTest + { + TestCode = testCode, + FixedState = + { + Sources = { fixedCode }, + MarkupHandling = MarkupMode.Allow, + }, + CodeActionEquivalenceKey = "IncrementFix:4", + CodeActionIndex = 0, + }.RunAsync(); + } + + [Fact] + [WorkItem(874, "https://github.com/dotnet/roslyn-sdk/issues/874")] + public async Task TestTwoIterationsRequiredButNoneApplied() + { + var testCode = @" +class TestClass { + int field = [|3|]; +} +"; + var fixedCode = @" +class TestClass { + int field = [|4|]; +} +"; + + var exception = await Assert.ThrowsAsync(async () => + { + await new CSharpTest + { + TestCode = testCode, + FixedState = + { + Sources = { fixedCode }, + MarkupHandling = MarkupMode.Allow, + }, + CodeActionEquivalenceKey = "IncrementFix:3", + CodeActionIndex = 0, + }.RunAsync(); + }); + + new DefaultVerifier().EqualOrDiff($"Context: Iterative code fix application{Environment.NewLine}The code action equivalence key and index must be consistent when both are specified.", exception.Message); + } + private class CSharpTest : CSharpCodeFixTest { public int DiagnosticIndexToFix { get; set; } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/LiteralUnderFiveAnalyzer.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/LiteralUnderFiveAnalyzer.cs index 281dd7808..e852b0a6a 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/LiteralUnderFiveAnalyzer.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/LiteralUnderFiveAnalyzer.cs @@ -14,6 +14,8 @@ namespace Microsoft.CodeAnalysis.Testing.TestAnalyzers [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] public class LiteralUnderFiveAnalyzer : DiagnosticAnalyzer { + internal const string CurrentValueProperty = nameof(CurrentValueProperty); + internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor("LiteralUnderFive", "title", "message", "category", DiagnosticSeverity.Warning, isEnabledByDefault: true); @@ -34,7 +36,9 @@ private void HandleLiteralOperation(OperationAnalysisContext context) && operation.ConstantValue.Value is int value && value < 5) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, operation.Syntax.GetLocation())); + var properties = ImmutableDictionary.Empty + .Add(CurrentValueProperty, value.ToString()); + context.ReportDiagnostic(Diagnostic.Create(Descriptor, operation.Syntax.GetLocation(), properties)); } } } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestFixes/IncrementFix.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestFixes/IncrementFix.cs index dadff19f9..f7d0765e8 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestFixes/IncrementFix.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestFixes/IncrementFix.cs @@ -31,7 +31,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) CodeAction.Create( "LiteralUnderFive", cancellationToken => CreateChangedDocument(context.Document, diagnostic.Location.SourceSpan, cancellationToken), - nameof(IncrementFix)), + $"{nameof(IncrementFix)}:{int.Parse(diagnostic.Properties[LiteralUnderFiveAnalyzer.CurrentValueProperty]!) + 1}"), diagnostic); }