Fix extract component whitespace handling (#11262)

fixes #11261
dotnet · Nov 27, 2024 · e4d1b9e · e4d1b9e
1 parent 22063f2
commit e4d1b9e
Show file tree

Hide file tree

Showing 2 changed files with 149 additions and 16 deletions.
diff --git a/...t.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs b/...t.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs
@@ -83,7 +83,7 @@ public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeAct
 
     private static (SyntaxNode? Start, SyntaxNode? End) GetStartAndEndElements(RazorCodeActionContext context, RazorSyntaxTree syntaxTree)
     {
-        var owner = syntaxTree.Root.FindInnermostNode(context.StartAbsoluteIndex, includeWhitespace: true);
+        var owner = syntaxTree.Root.FindInnermostNode(context.StartAbsoluteIndex, includeWhitespace: !context.HasSelection);
         if (owner is null)
         {
             return (null, null);
@@ -112,25 +112,13 @@ private static (SyntaxNode? Start, SyntaxNode? End) GetStartAndEndElements(Razor
 
     private static SyntaxNode? GetEndElementNode(RazorCodeActionContext context, RazorSyntaxTree syntaxTree)
     {
-        var endOwner = syntaxTree.Root.FindInnermostNode(context.EndAbsoluteIndex, includeWhitespace: true);
+        var endOwner = syntaxTree.Root.FindInnermostNode(context.EndAbsoluteIndex, includeWhitespace: false);
         if (endOwner is null)
         {
             return null;
         }
 
-        // Correct selection to include the current node if the selection ends immediately after a closing tag.
-        if (endOwner is MarkupTextLiteralSyntax markupTextLiteral
-            && SelectionShouldBePrevious(markupTextLiteral, context.EndAbsoluteIndex)
-            && endOwner.TryGetPreviousSibling(out var previousSibling))
-        {
-            endOwner = previousSibling;
-        }
-
         return GetBlockOrTextNode(endOwner);
-
-        static bool SelectionShouldBePrevious(MarkupTextLiteralSyntax markupTextLiteral, int absoluteIndex)
-            => markupTextLiteral.Span.Start == absoluteIndex
-                || markupTextLiteral.ContainsOnlyWhitespace();
     }
 
     private static SyntaxNode? GetBlockOrTextNode(SyntaxNode node)

diff --git a/...e.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionProviderTest.cs b/...e.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionProviderTest.cs
@@ -22,8 +22,10 @@
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using Moq;
+using Roslyn.Test.Utilities;
 using Xunit;
 using Xunit.Abstractions;
+using WorkItemAttribute = Roslyn.Test.Utilities.WorkItemAttribute;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.Test.CodeActions.Razor;
 
@@ -465,6 +467,144 @@ public Task Handle_MultipointSelection_TextInNested2()
             </div>|}
             """);
 
+    [Fact]
+    [WorkItem("https://github.com/dotnet/razor/issues/11261")]
+    public Task Handle_Inside_ElseBlock()
+        => TestAsync("""
+            @page "/weather"
+
+            <PageTitle>Weather</PageTitle>
+
+            <h1>Weather</h1>
+
+            <p>This component demonstrates showing data.</p>
+
+            @if (forecasts == null)
+            {
+                <p><em>Loading...</em></p>
+            }
+            else
+            {
+            {|selection:    {|result:<table class="table">
+                    <thead>
+                        <tr>
+                            <th>Date</th>
+                            <th aria-label="Temperature in Celsius">Temp. (C)</th>
+                            <th aria-label="Temperature in Farenheit">Temp. (F)</th>
+                            <th>Summary</th>
+                        </tr>
+                    </thead>
+                    <tbody>
+                        @foreach (var forecast in forecasts)
+                        {
+                            <tr>
+                                <td>@forecast.Date.ToShortDateString()</td>
+                                <td>@forecast.TemperatureC</td>
+                                <td>@forecast.TemperatureF</td>
+                                <td>@forecast.Summary</td>
+                            </tr>
+                        }
+                    </tbody>
+                </table>|}|}
+            }
+
+            @code {
+                private WeatherForecast[]? forecasts;
+
+                protected override async Task OnInitializedAsync()
+                {
+                    // Simulate asynchronous loading to demonstrate a loading indicator
+                    await Task.Delay(500);
+
+                    var startDate = DateOnly.FromDateTime(DateTime.Now);
+                    var summaries = new[] { "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching" };
+                    forecasts = Enumerable.Range(1, 5).Select(index => new WeatherForecast
+                    {
+                        Date = startDate.AddDays(index),
+                        TemperatureC = Random.Shared.Next(-20, 55),
+                        Summary = summaries[Random.Shared.Next(summaries.Length)]
+                    }).ToArray();
+                }
+
+                private class WeatherForecast
+                {
+                    public DateOnly Date { get; set; }
+                    public int TemperatureC { get; set; }
+                    public string? Summary { get; set; }
+                    public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
+                }
+            }
+            """);
+
+    [Fact]
+    [WorkItem("https://github.com/dotnet/razor/issues/11261")]
+    public Task Handle_Inside_ElseBlock_NoSelection()
+        => TestAsync("""
+            @page "/weather"
+
+            <PageTitle>Weather</PageTitle>
+
+            <h1>Weather</h1>
+
+            <p>This component demonstrates showing data.</p>
+
+            @if (forecasts == null)
+            {
+                <p><em>Loading...</em></p>
+            }
+            else
+            {
+            {|selection:|}  <table class="table">
+                    <thead>
+                        <tr>
+                            <th>Date</th>
+                            <th aria-label="Temperature in Celsius">Temp. (C)</th>
+                            <th aria-label="Temperature in Farenheit">Temp. (F)</th>
+                            <th>Summary</th>
+                        </tr>
+                    </thead>
+                    <tbody>
+                        @foreach (var forecast in forecasts)
+                        {
+                            <tr>
+                                <td>@forecast.Date.ToShortDateString()</td>
+                                <td>@forecast.TemperatureC</td>
+                                <td>@forecast.TemperatureF</td>
+                                <td>@forecast.Summary</td>
+                            </tr>
+                        }
+                    </tbody>
+                </table>
+            }
+
+            @code {
+                private WeatherForecast[]? forecasts;
+
+                protected override async Task OnInitializedAsync()
+                {
+                    // Simulate asynchronous loading to demonstrate a loading indicator
+                    await Task.Delay(500);
+
+                    var startDate = DateOnly.FromDateTime(DateTime.Now);
+                    var summaries = new[] { "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching" };
+                    forecasts = Enumerable.Range(1, 5).Select(index => new WeatherForecast
+                    {
+                        Date = startDate.AddDays(index),
+                        TemperatureC = Random.Shared.Next(-20, 55),
+                        Summary = summaries[Random.Shared.Next(summaries.Length)]
+                    }).ToArray();
+                }
+
+                private class WeatherForecast
+                {
+                    public DateOnly Date { get; set; }
+                    public int TemperatureC { get; set; }
+                    public string? Summary { get; set; }
+                    public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
+                }
+            }
+            """);
+
     private static RazorCodeActionContext CreateRazorCodeActionContext(
         VSCodeActionParams request,
         TextSpan selectionSpan,
@@ -557,7 +697,12 @@ private async Task TestAsync(string contents)
         Assert.NotNull(razorCodeActionResolutionParams);
         var actionParams = ((JsonElement)razorCodeActionResolutionParams.Data!).Deserialize<ExtractToComponentCodeActionParams>();
         Assert.NotNull(actionParams);
-        Assert.Equal(resultSpan.Start, actionParams.Start);
-        Assert.Equal(resultSpan.End, actionParams.End);
+
+        if (resultSpan.Start != actionParams.Start || resultSpan.End != actionParams.End)
+        {
+            var resultText = context.SourceText.GetSubTextString(resultSpan);
+            var actualText = context.SourceText.GetSubTextString(TextSpan.FromBounds(actionParams.Start, actionParams.End));
+            AssertEx.EqualOrDiff(resultText, actualText, "Code action span does not match expected");
+        }
     }
 }