From e4d1b9e72afaea34cf4d644bb3e8a16e1910260f Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Wed, 27 Nov 2024 15:00:05 -0800 Subject: [PATCH] Fix extract component whitespace handling (#11262) fixes #11261 --- .../ExtractToComponentCodeActionProvider.cs | 16 +- ...xtractToComponentCodeActionProviderTest.cs | 149 +++++++++++++++++- 2 files changed, 149 insertions(+), 16 deletions(-) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs index 9a0a25559c7..fefae3c1b7e 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToComponentCodeActionProvider.cs @@ -83,7 +83,7 @@ public Task> 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/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionProviderTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionProviderTest.cs index 18fed216463..105a5614503 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionProviderTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.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() |} """); + [Fact] + [WorkItem("https://github.com/dotnet/razor/issues/11261")] + public Task Handle_Inside_ElseBlock() + => TestAsync(""" + @page "/weather" + + Weather + +

Weather

+ +

This component demonstrates showing data.

+ + @if (forecasts == null) + { +

Loading...

+ } + else + { + {|selection: {|result: + + + + + + + + + + @foreach (var forecast in forecasts) + { + + + + + + + } + +
DateTemp. (C)Temp. (F)Summary
@forecast.Date.ToShortDateString()@forecast.TemperatureC@forecast.TemperatureF@forecast.Summary
|}|} + } + + @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" + + Weather + +

Weather

+ +

This component demonstrates showing data.

+ + @if (forecasts == null) + { +

Loading...

+ } + else + { + {|selection:|} + + + + + + + + + + @foreach (var forecast in forecasts) + { + + + + + + + } + +
DateTemp. (C)Temp. (F)Summary
@forecast.Date.ToShortDateString()@forecast.TemperatureC@forecast.TemperatureF@forecast.Summary
+ } + + @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(); 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"); + } } }