Fix some things for extract component

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<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeAct
             return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
         }
 
-        var actionParams = CreateActionParams(startNode, endNode, @namespace);
+        var span = TryGetSpanFromNodes(startNode, endNode, context);
+        if (span is null)
+        {
+            return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
+        }
+
+        var actionParams = new ExtractToComponentCodeActionParams
+        {
+            Start = span.Value.Start,
+            End = span.Value.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<SyntaxNode>(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 <p>hello$ there</p>
-            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,50 +118,88 @@ 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<SyntaxNode>(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<SyntaxNode>(IsBlockNode);
+        if (blockNode is not null)
+        {
+            return blockNode;
+        }
 
-    private static ExtractToComponentCodeActionParams CreateActionParams(
-        SyntaxNode startNode,
-        SyntaxNode endNode,
-        string @namespace)
+        // Account for cases where a text literal is not contained
+        // within a block node. For example:
+        // <h1> Example </h1>
+        // [|This is not in a block but is a valid selection|]
+        if (node is MarkupTextLiteralSyntax markupTextLiteral)
+        {
+            return markupTextLiteral;
+        }
+
+        return null;
+    }
+
+    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 null)
         {
-            Start = selectionSpan.Start,
-            End = selectionSpan.End,
-            Namespace = @namespace
-        };
+            return null;
+        }
+
+        var selectionSpan = initialSpan.Value;
+
+        // 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.
         var commonAncestor = endNode.Span.Contains(startNode.Span)
             ? 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<SyntaxNode>(IsBlockOrMarkupBlockNode);
+        while (commonAncestor is not null && IsBlockOrMarkupBlockNode(commonAncestor))
         {
             if (commonAncestor.Span.Contains(startNode.Span) &&
                 commonAncestor.Span.Contains(endNode.Span))
@@ -168,6 +210,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 +232,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 +249,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 +282,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;
 }

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;
+}

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 "/"
+
+            <PageTitle>Home</PageTitle>
+
+            {|result:{|selection:<h1>Hello</h1>|}|}
+
+            Welcome to your new app.
+            """);
+
+    [Fact]
+    public Task Handle_MultipointSelection_TextOnly()
+        => TestAsync("""
+            @page "/"
+
+            <PageTitle>Home</PageTitle>
+            <h1>Hello</h1>
+
+            {|result:{|selection:Welcome to your new app|}|}
+            """);
+
+    [Fact]
+    public Task Handle_MultipointSelection_TextOnly2()
+        => TestAsync("""
+            @page "/"
+
+            <PageTitle>Home</PageTitle>
+            <h1>Hello</h1>
+
+            Welcome to {|result:{|selection:your new app|}|}
+            """);
+
+    [Fact]
+    public Task Handle_MultipointSelection_TextInNested()
+        => TestAsync("""
+            {|result:<div>
+                <div>
+                    <p>
+                        Hello {|selection:there!
+                    </p>
+                </div>
+            </div>
+
+            Welcome to your|}|} new app
+            """);
+
+    [Fact]
+    public Task Handle_MultipointSelection_TextInNested2()
+        => TestAsync("""
+            Welcome to your{|result:{|selection: new app
+
+            <div>
+                <div>
+                    <p>
+                        Hello |}there!
+                    </p>
+                </div>
+            </div>|}
+            """);
+
     private static RazorCodeActionContext CreateRazorCodeActionContext(
         VSCodeActionParams request,
         TextSpan selectionSpan,

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 = """
+            <h1> Hello </h1>
+
+            Welcome to [|your new app|]
+            """;
+
+        var expectedRazorComponent = """
+            your new app
+            """;
+
+        var expectedOriginalDocument = """
+            <h1> Hello </h1>
+
+            Welcome to <Component />
+            """;
+
+        await TestAsync(
+            input,
+            expectedOriginalDocument,
+            expectedRazorComponent);
+    }
+
     private async Task TestAsync(
         string input,
         string expectedOriginalDocument,