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 695e3a5f1fa..a91f95755d9 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 @@ -3,7 +3,6 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor.Language; @@ -56,7 +55,18 @@ public Task> ProvideAsync(RazorCodeAct return SpecializedTasks.EmptyImmutableArray(); } - var actionParams = CreateActionParams(startNode, endNode, @namespace); + var possibleSpan = TryGetSpanFromNodes(startNode, endNode, context); + if (possibleSpan is not { } span) + { + return SpecializedTasks.EmptyImmutableArray(); + } + + var actionParams = new ExtractToComponentCodeActionParams + { + Start = span.Start, + End = span.End, + Namespace = @namespace + }; var resolutionParams = new RazorCodeActionResolutionParams() { @@ -78,31 +88,25 @@ private static (SyntaxNode? Start, SyntaxNode? End) GetStartAndEndElements(Razor return (null, null); } - var startElementNode = owner.FirstAncestorOrSelf(IsBlockNode); + // In cases where the start element is just a text literal and there + // is no user selection avoid extracting the whole text literal.or + // the parent element. + if (owner is MarkupTextLiteralSyntax && !context.HasSelection) + { + return (null, null); + } - if (startElementNode is null || LocationInvalid(context.StartAbsoluteIndex, startElementNode)) + var startElementNode = GetBlockOrTextNode(owner); + if (startElementNode is null) { return (null, null); } - var endElementNode = context.StartAbsoluteIndex == context.EndAbsoluteIndex - ? startElementNode - : GetEndElementNode(context, syntaxTree); + var endElementNode = context.HasSelection + ? GetEndElementNode(context, syntaxTree) + : startElementNode; return (startElementNode, endElementNode); - - static bool LocationInvalid(int location, SyntaxNode node) - { - // Make sure to test for cases where selection - // is inside of a markup tag such as

hello$ there

- if (node is MarkupElementSyntax markupElement) - { - return location > markupElement.StartTag.Span.End && - location < markupElement.EndTag.SpanStart; - } - - return !node.Span.Contains(location); - } } private static SyntaxNode? GetEndElementNode(RazorCodeActionContext context, RazorSyntaxTree syntaxTree) @@ -114,40 +118,75 @@ static bool LocationInvalid(int location, SyntaxNode node) } // Correct selection to include the current node if the selection ends immediately after a closing tag. - if (endOwner is MarkupTextLiteralSyntax - && endOwner.ContainsOnlyWhitespace() + if (endOwner is MarkupTextLiteralSyntax markupTextLiteral + && SelectionShouldBePrevious(markupTextLiteral, context.EndAbsoluteIndex) && endOwner.TryGetPreviousSibling(out var previousSibling)) { endOwner = previousSibling; } - return endOwner.FirstAncestorOrSelf(IsBlockNode); + return GetBlockOrTextNode(endOwner); + + static bool SelectionShouldBePrevious(MarkupTextLiteralSyntax markupTextLiteral, int absoluteIndex) + => markupTextLiteral.Span.Start == absoluteIndex + || markupTextLiteral.ContainsOnlyWhitespace(); } - private static bool IsBlockNode(SyntaxNode node) - => node.Kind is - SyntaxKind.MarkupElement or - SyntaxKind.MarkupTagHelperElement or - SyntaxKind.CSharpCodeBlock; + private static SyntaxNode? GetBlockOrTextNode(SyntaxNode node) + { + var blockNode = node.FirstAncestorOrSelf(IsBlockNode); + if (blockNode is not null) + { + return blockNode; + } + + // Account for cases where a text literal is not contained + // within a block node. For example: + //

Example

+ // [|This is not in a block but is a valid selection|] + if (node is MarkupTextLiteralSyntax markupTextLiteral) + { + return markupTextLiteral; + } + + return null; + } - private static ExtractToComponentCodeActionParams CreateActionParams( - SyntaxNode startNode, - SyntaxNode endNode, - string @namespace) + private TextSpan? TryGetSpanFromNodes(SyntaxNode startNode, SyntaxNode endNode, RazorCodeActionContext context) { - var selectionSpan = AreSiblings(startNode, endNode) + // First get a decent span to work with. If the two nodes chosen + // are siblings (even with elements in between) then their start/end + // work fine. However, if the two nodes are not siblings then + // some work has to be done. See GetEncompassingTextSpan for the + // information on the heuristic for choosing a span in that case. + var initialSpan = AreSiblings(startNode, endNode) ? TextSpan.FromBounds(startNode.Span.Start, endNode.Span.End) : GetEncompassingTextSpan(startNode, endNode); - return new ExtractToComponentCodeActionParams + if (initialSpan is not { } selectionSpan) { - Start = selectionSpan.Start, - End = selectionSpan.End, - Namespace = @namespace - }; + return null; + } + + // Now that a span is chosen there is still a chance the user intended only + // part of text to be chosen. If the start or end node are text AND the selection span + // is inside those nodes modify the selection to be the users initial point. That makes sure + // all the text isn't included and only the user selected text is. + // NOTE: Intersects with is important because we want to include the end position when comparing. + if (startNode is MarkupTextLiteralSyntax && startNode.Span.IntersectsWith(selectionSpan.Start)) + { + selectionSpan = TextSpan.FromBounds(context.StartAbsoluteIndex, selectionSpan.End); + } + + if (endNode is MarkupTextLiteralSyntax && endNode.Span.IntersectsWith(selectionSpan.End)) + { + selectionSpan = TextSpan.FromBounds(selectionSpan.Start, context.EndAbsoluteIndex); + } + + return selectionSpan; } - private static TextSpan GetEncompassingTextSpan(SyntaxNode startNode, SyntaxNode endNode) + private static TextSpan? GetEncompassingTextSpan(SyntaxNode startNode, SyntaxNode endNode) { // Find a valid node that encompasses both the start and the end to // become the selection. @@ -155,9 +194,10 @@ private static TextSpan GetEncompassingTextSpan(SyntaxNode startNode, SyntaxNode ? endNode : startNode; - while (commonAncestor is MarkupElementSyntax or - MarkupTagHelperAttributeSyntax or - MarkupBlockSyntax) + // IsBlockOrMarkupBlockNode because the common ancestor could be a MarkupBlock + // even if that's an invalid start/end node. + commonAncestor = commonAncestor.FirstAncestorOrSelf(IsBlockOrMarkupBlockNode); + while (commonAncestor is not null && IsBlockOrMarkupBlockNode(commonAncestor)) { if (commonAncestor.Span.Contains(startNode.Span) && commonAncestor.Span.Contains(endNode.Span)) @@ -168,6 +208,11 @@ MarkupTagHelperAttributeSyntax or commonAncestor = commonAncestor.Parent; } + if (commonAncestor is null) + { + return null; + } + // If walking up the tree was required then make sure to reduce // selection back down to minimal nodes needed. // For example: @@ -185,7 +230,7 @@ MarkupTagHelperAttributeSyntax or commonAncestor != endNode) { SyntaxNode? modifiedStart = null, modifiedEnd = null; - foreach (var child in commonAncestor.ChildNodes().Where(static node => node.Kind == SyntaxKind.MarkupElement)) + foreach (var child in commonAncestor.ChildNodes()) { if (child.Span.Contains(startNode.Span)) { @@ -202,6 +247,8 @@ MarkupTagHelperAttributeSyntax or } } + // There's a start and end node that are siblings and will work for start/end + // of extraction into the new component. if (modifiedStart is not null && modifiedEnd is not null) { return TextSpan.FromBounds(modifiedStart.Span.Start, modifiedEnd.Span.End); @@ -233,4 +280,14 @@ private static bool TryGetNamespace(RazorCodeDocument codeDocument, [NotNullWhen // and causing compiler errors. Avoid offering this refactoring if we can't accurately get a // good namespace to extract to => codeDocument.TryComputeNamespace(fallbackToRootNamespace: true, out @namespace); + + private static bool IsBlockNode(SyntaxNode node) + => node.Kind is + SyntaxKind.MarkupElement or + SyntaxKind.MarkupTagHelperElement or + SyntaxKind.CSharpCodeBlock; + + private static bool IsBlockOrMarkupBlockNode(SyntaxNode node) + => IsBlockNode(node) + || node.Kind == SyntaxKind.MarkupBlock; } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/RazorCodeActionContext.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/RazorCodeActionContext.cs index 053b54c51b8..f9f0b4eb401 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/RazorCodeActionContext.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/RazorCodeActionContext.cs @@ -16,4 +16,7 @@ internal sealed record class RazorCodeActionContext( int EndAbsoluteIndex, SourceText SourceText, bool SupportsFileCreation, - bool SupportsCodeActionResolve); + bool SupportsCodeActionResolve) +{ + public bool HasSelection => StartAbsoluteIndex != EndAbsoluteIndex; +} 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 0ca6ed37b0c..72a448c6bdb 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 @@ -402,6 +402,68 @@ public Task Handle_MultipointSelection_CSharpBlock() }|} """); + [Fact] + public Task Handle_MultipointSelection_SurroundingH1() + => TestAsync(""" + @page "/" + + Home + + {|result:{|selection:

Hello

|}|} + + Welcome to your new app. + """); + + [Fact] + public Task Handle_MultipointSelection_TextOnly() + => TestAsync(""" + @page "/" + + Home +

Hello

+ + {|result:{|selection:Welcome to your new app|}|} + """); + + [Fact] + public Task Handle_MultipointSelection_TextOnly2() + => TestAsync(""" + @page "/" + + Home +

Hello

+ + Welcome to {|result:{|selection:your new app|}|} + """); + + [Fact] + public Task Handle_MultipointSelection_TextInNested() + => TestAsync(""" + {|result:
+
+

+ Hello {|selection:there! +

+
+
+ + Welcome to your|}|} new app + """); + + [Fact] + public Task Handle_MultipointSelection_TextInNested2() + => TestAsync(""" + Welcome to your{|result:{|selection: new app + +
+
+

+ Hello |}there! +

+
+
|} + """); + private static RazorCodeActionContext CreateRazorCodeActionContext( VSCodeActionParams request, TextSpan selectionSpan, diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs index 5358dbc1ebf..a185b24aab7 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs @@ -259,6 +259,31 @@ await TestAsync( expectedRazorComponent); } + [Fact] + public async Task Handle_TextOnlySelection() + { + var input = """ +

Hello

+ + Welcome to [|your new app|] + """; + + var expectedRazorComponent = """ + your new app + """; + + var expectedOriginalDocument = """ +

Hello

+ + Welcome to + """; + + await TestAsync( + input, + expectedOriginalDocument, + expectedRazorComponent); + } + private async Task TestAsync( string input, string expectedOriginalDocument,