Merge pull request #67948 from Cosifne/dev/shech/FixMergeConsecutiveI…

…fStatementCodeRefactoringProvider Fix a wrong text span usage in MergeIfStatementsCodeRefactoringProvider
dotnet · Apr 25, 2023 · 248e851 · 248e851
2 parents 1546e8e + 080c404
commit 248e851
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 8 deletions.
diff --git a/...orFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/...orFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs
@@ -36,6 +36,8 @@
 
 using System.Diagnostics;
 using System.IO;
+using Microsoft.CodeAnalysis.CodeRefactorings;
+using Microsoft.CodeAnalysis.Testing;
 
 #if !CODE_STYLE
 using Microsoft.CodeAnalysis.Editor.UnitTests.Extensions;
@@ -320,12 +322,35 @@ protected async Task TestDiagnosticMissingAsync(
         protected abstract Task<ImmutableArray<Diagnostic>> GetDiagnosticsWorkerAsync(
             TestWorkspace workspace, TestParameters parameters);
 
+        internal abstract Task<CodeRefactoring> GetCodeRefactoringAsync(TestWorkspace workspace, TestParameters parameters);
+
         protected Task TestSmartTagTextAsync(string initialMarkup, string displayText, int index)
             => TestSmartTagTextAsync(initialMarkup, displayText, new TestParameters(index: index));
 
         protected Task TestSmartTagGlyphTagsAsync(string initialMarkup, ImmutableArray<string> glyphTags, int index)
             => TestSmartTagGlyphTagsAsync(initialMarkup, glyphTags, new TestParameters(index: index));
 
+        protected async Task TestCodeRefactoringApplicableTextSpan(
+            string markup,
+            string textSpanMarker,
+            TestParameters parameters = null)
+        {
+            var ps = parameters ?? TestParameters.Default;
+            using var workspace = CreateWorkspaceFromOptions(markup, ps);
+            var refactoring = await GetCodeRefactoringAsync(workspace, ps).ConfigureAwait(false);
+
+            TestFileMarkupParser.GetPositionsAndSpans(markup, out _, out _, out var spans);
+            Assert.True(spans.ContainsKey(textSpanMarker));
+
+            var expectedTextSpans = spans[textSpanMarker].Sort();
+            var actualTextSpans = refactoring.CodeActions.WhereAsArray(action => action.applicableToSpan is not null).SelectAsArray(action => action.applicableToSpan).Sort();
+            Assert.Equal(expectedTextSpans.Length, actualTextSpans.Length);
+            for (var i = 0; i < expectedTextSpans.Length; i++)
+            {
+                Assert.Equal(expectedTextSpans[i], actualTextSpans[i]);
+            }
+        }
+
         protected async Task TestSmartTagTextAsync(
             string initialMarkup,
             string displayText,

diff --git a/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionTest.cs b/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionTest.cs
@@ -92,7 +92,7 @@ private static async Task<CodeAction> GetFixAllFixAsync(
         protected override Task<ImmutableArray<Diagnostic>> GetDiagnosticsWorkerAsync(TestWorkspace workspace, TestParameters parameters)
             => SpecializedTasks.EmptyImmutableArray<Diagnostic>();
 
-        internal async Task<CodeRefactoring> GetCodeRefactoringAsync(
+        internal override async Task<CodeRefactoring> GetCodeRefactoringAsync(
             TestWorkspace workspace, TestParameters parameters)
         {
             GetDocumentAndSelectSpanOrAnnotatedSpan(workspace, out var document, out var span, out _);

diff --git a/src/EditorFeatures/DiagnosticsTestUtilities/Diagnostics/AbstractUserDiagnosticTest.cs b/src/EditorFeatures/DiagnosticsTestUtilities/Diagnostics/AbstractUserDiagnosticTest.cs
@@ -4,27 +4,32 @@
 
 #nullable disable
 
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CodeFixesAndRefactorings;
+using Microsoft.CodeAnalysis.CodeRefactorings;
+using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
+using Microsoft.CodeAnalysis.Remote.Testing;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.Utilities;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.CodeAnalysis.UnitTests.Diagnostics;
 using Roslyn.Test.Utilities;
 using Roslyn.Utilities;
 using Xunit;
-using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.Remote.Testing;
 using Xunit.Abstractions;
-using Microsoft.CodeAnalysis.Shared.Extensions;
-using Microsoft.CodeAnalysis.CodeFixesAndRefactorings;
+using FixAllContext = Microsoft.CodeAnalysis.CodeFixes.FixAllContext;
+using FixAllProvider = Microsoft.CodeAnalysis.CodeFixes.FixAllProvider;
+using FixAllState = Microsoft.CodeAnalysis.CodeFixes.FixAllState;
 
 namespace Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics
 {
@@ -85,6 +90,9 @@ protected override async Task<ImmutableArray<Diagnostic>> GetDiagnosticsWorkerAs
             return dxs;
         }
 
+        internal override Task<CodeRefactoring> GetCodeRefactoringAsync(TestWorkspace workspace, TestParameters parameters)
+            => throw new NotImplementedException("No refactoring provided in diagnostic test");
+
         protected static void AddAnalyzerToWorkspace(Workspace workspace, DiagnosticAnalyzer analyzer, TestParameters parameters)
         {
             AnalyzerReference[] analyzeReferences;

diff --git a/...es/CSharpTest/SplitOrMergeIfStatements/MergeConsecutiveIfStatementsTests_ElseIf_Middle.cs b/...es/CSharpTest/SplitOrMergeIfStatements/MergeConsecutiveIfStatementsTests_ElseIf_Middle.cs
@@ -22,10 +22,10 @@ void M(bool a, bool b, bool c)
                     {
                         if (a)
                             System.Console.WriteLine(null);
-                        [||]else if (b)
+                        [||]else {|applicableSpan:if (b)
                             System.Console.WriteLine();
                         else if (c)
-                            System.Console.WriteLine();
+                            System.Console.WriteLine();|}
                     }
                 }
                 """;
@@ -45,6 +45,7 @@ void M(bool a, bool b, bool c)
 
             await TestActionCountAsync(Initial, 1);
             await TestInRegularAndScriptAsync(Initial, Expected);
+            await TestCodeRefactoringApplicableTextSpan(Initial, "applicableSpan");
         }
 
         [Fact]

diff --git a/...ore/Portable/SplitOrMergeIfStatements/AbstractMergeIfStatementsCodeRefactoringProvider.cs b/...ore/Portable/SplitOrMergeIfStatements/AbstractMergeIfStatementsCodeRefactoringProvider.cs
@@ -57,7 +57,7 @@ void RegisterRefactoring(MergeDirection direction, TextSpan upperIfOrElseIfSpan,
                             c => RefactorAsync(document, upperIfOrElseIfSpan, lowerIfOrElseIfSpan, c),
                             direction,
                             syntaxFacts.GetText(syntaxKinds.IfKeyword)),
-                        new TextSpan(upperIfOrElseIfSpan.Start, lowerIfOrElseIfSpan.End));
+                        TextSpan.FromBounds(upperIfOrElseIfSpan.Start, lowerIfOrElseIfSpan.End));
                 }
             }
         }