Fix extract method in top level

src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodBase.cs

@@ -86,8 +86,7 @@ protected static async Task TestExtractMethodAsync(
         var testDocument = workspace.Documents.Single();
         var subjectBuffer = testDocument.GetTextBuffer();
 
-        var tree = await ExtractMethodAsync(
-            workspace, testDocument, localFunction: localFunction);
+        var tree = await ExtractMethodAsync(workspace, testDocument, localFunction: localFunction);
 
         using (var edit = subjectBuffer.CreateEdit())
         {
@@ -130,7 +129,7 @@ protected static async Task<SyntaxNode> ExtractMethodAsync(
         };
 
         var semanticDocument = await SemanticDocument.CreateAsync(document, CancellationToken.None);
-        var validator = new CSharpSelectionValidator(semanticDocument, testDocument.SelectedSpans.Single(), localFunction: false);
+        var validator = new CSharpSelectionValidator(semanticDocument, testDocument.SelectedSpans.Single(), localFunction);
 
         var (selectedCode, status) = await validator.GetValidSelectionAsync(CancellationToken.None);
         if (!succeed && status.Failed)

src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodMiscellaneousTests.cs

@@ -126,16 +126,23 @@ class A
             }
             """;
 
+        await TestCommandHandler(markupCode, result: null, expectNotification: true);
+    }
+
+    private static async Task TestCommandHandler(string markupCode, string? result, bool expectNotification)
+    {
         using var workspace = EditorTestWorkspace.CreateCSharp(markupCode, composition: EditorTestCompositions.EditorFeaturesWpf);
         var testDocument = workspace.Documents.Single();
 
         var view = testDocument.GetTextView();
         view.Selection.Select(new SnapshotSpan(
             view.TextBuffer.CurrentSnapshot, testDocument.SelectedSpans[0].Start, testDocument.SelectedSpans[0].Length), isReversed: false);
 
+        result ??= view.TextBuffer.CurrentSnapshot.GetText();
+
         var callBackService = (INotificationServiceCallback)workspace.Services.GetRequiredService<INotificationService>();
-        var called = false;
-        callBackService.NotificationCallback = (_, _, _) => called = true;
+        var gotNotification = false;
+        callBackService.NotificationCallback = (_, _, _) => gotNotification = true;
 
         var handler = workspace.ExportProvider.GetCommandHandler<ExtractMethodCommandHandler>(PredefinedCommandHandlerNames.ExtractMethod, ContentTypeNames.CSharpContentType);
 
@@ -144,6 +151,24 @@ class A
         var waiter = workspace.ExportProvider.GetExportedValue<IAsynchronousOperationListenerProvider>().GetWaiter(FeatureAttribute.ExtractMethod);
         await waiter.ExpeditedWaitAsync();
 
-        Assert.True(called);
+        Assert.Equal(expectNotification, gotNotification);
+        Assert.Equal(result, view.TextBuffer.CurrentSnapshot.GetText());
+    }
+
+    [WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/65465")]
+    public async Task TestExtractLocalFunctionInTopLevelFromCommandHandler()
+    {
+        var markupCode = """
+            System.Console.WriteLine([|"string"|]);
+            """;
+
+        await TestCommandHandler(markupCode, """
+            System.Console.WriteLine(NewMethod());
+
+            static string NewMethod()
+            {
+                return "string";
+            }
+            """, expectNotification: false);
     }
 }

src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodTests.cs

@@ -12208,15 +12208,14 @@ public async Task TopLevelStatement_ValueInAssignment()
             """;
         var expected = """
             bool local;
+            local = NewMethod();
 
             static bool NewMethod()
             {
                 return true;
             }
-
-            local = NewMethod();
             """;
-        await TestExtractMethodAsync(code, expected);
+        await TestExtractMethodAsync(code, expected, localFunction: true);
     }
 
     [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/44260")]
@@ -12236,7 +12235,7 @@ static string NewMethod()
                 return "string";
             }
             """;
-        await TestExtractMethodAsync(code, expected);
+        await TestExtractMethodAsync(code, expected, localFunction: true);
     }
 
     [Theory]

src/EditorFeatures/Core/ExtractMethod/ExtractMethodCommandHandler.cs

@@ -14,7 +14,6 @@
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Notification;
-using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.CodeAnalysis.Text.Shared.Extensions;
@@ -39,7 +38,6 @@ internal sealed class ExtractMethodCommandHandler : ICommandHandler<ExtractMetho
     private readonly IThreadingContext _threadingContext;
     private readonly ITextBufferUndoManagerProvider _undoManager;
     private readonly IInlineRenameService _renameService;
-    private readonly IGlobalOptionService _globalOptions;
     private readonly IAsynchronousOperationListener _asyncListener;
 
     [ImportingConstructor]
@@ -48,7 +46,6 @@ public ExtractMethodCommandHandler(
         IThreadingContext threadingContext,
         ITextBufferUndoManagerProvider undoManager,
         IInlineRenameService renameService,
-        IGlobalOptionService globalOptions,
         IAsynchronousOperationListenerProvider asyncListenerProvider)
     {
         Contract.ThrowIfNull(threadingContext);
@@ -58,7 +55,6 @@ public ExtractMethodCommandHandler(
         _threadingContext = threadingContext;
         _undoManager = undoManager;
         _renameService = renameService;
-        _globalOptions = globalOptions;
         _asyncListener = asyncListenerProvider.GetListener(FeatureAttribute.ExtractMethod);
     }
 
@@ -140,12 +136,35 @@ private async Task ExecuteWorkerAsync(
             return;
 
         var options = await document.GetExtractMethodGenerationOptionsAsync(cancellationToken).ConfigureAwait(false);
-        var result = await ExtractMethodService.ExtractMethodAsync(
-            document, span, localFunction: false, options, cancellationToken).ConfigureAwait(false);
+
+        var result = await ExtractMethodService.ExtractMethodAsync(document, span, localFunction: false, options, cancellationToken).ConfigureAwait(false);
+
+        if (!Succeeded(result))
+        {
+            // Extract method didn't succeed.  Or succeeded, but had some reasons to notify the user about.  See if
+            // extracting a local function would be better..
+
+            var localFunctionResult = await ExtractMethodService.ExtractMethodAsync(document, span, localFunction: true, options, cancellationToken).ConfigureAwait(false);
+            if (Succeeded(localFunctionResult))
+            {
+                // Extract local function completely succeeded.  Use that instead.
+                result = localFunctionResult;
+            }
+            else if (!result.Succeeded && localFunctionResult.Succeeded)
+            {
+                // Extract method entirely failed.  But extract local function was able to proceed, albeit with reasons
+                // to notify the user about.  Continue one with extract local function instead.
+                result = localFunctionResult;
+            }
+            else
+            {
+                // Extract local function was just as bad as extract method.  Just report the extract method issues below.
+            }
+        }
+
         Contract.ThrowIfNull(result);
 
-        result = await NotifyUserIfNecessaryAsync(
-            document, result, cancellationToken).ConfigureAwait(false);
+        result = await NotifyUserIfNecessaryAsync(document, result, cancellationToken).ConfigureAwait(false);
         if (result is null)
             return;
 
@@ -185,11 +204,14 @@ private void ApplyChange_OnUIThread(
         undoTransaction.Complete();
     }
 
+    private static bool Succeeded(ExtractMethodResult result)
+        => result is { Succeeded: true, Reasons.Length: 0 };
+
     private async Task<ExtractMethodResult?> NotifyUserIfNecessaryAsync(
         Document document, ExtractMethodResult result, CancellationToken cancellationToken)
     {
         // If we succeeded without any problems, just proceed without notifying the user.
-        if (result is { Succeeded: true, Reasons.Length: 0 })
+        if (Succeeded(result))
             return result;
 
         // We have some sort of issue.  See what the user wants to do.  If we have no way to inform the user bail

src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs

@@ -81,17 +81,16 @@ public override SyntaxNode GetNodeForDataFlowAnalysis()
     }
 
     protected override bool UnderAnonymousOrLocalMethod(SyntaxToken token, SyntaxToken firstToken, SyntaxToken lastToken)
+        => IsUnderAnonymousOrLocalMethod(token, firstToken, lastToken);
+
+    public static bool IsUnderAnonymousOrLocalMethod(SyntaxToken token, SyntaxToken firstToken, SyntaxToken lastToken)
     {
         for (var current = token.Parent; current != null; current = current.Parent)
         {
             if (current is MemberDeclarationSyntax)
                 return false;
 
-            if (current is
-                    SimpleLambdaExpressionSyntax or
-                    ParenthesizedLambdaExpressionSyntax or
-                    AnonymousMethodExpressionSyntax or
-                    LocalFunctionStatementSyntax)
+            if (current is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax)
             {
                 // make sure the selection contains the lambda
                 return firstToken.SpanStart <= current.GetFirstToken().SpanStart &&

src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionValidator.cs

@@ -88,56 +88,62 @@ private SelectionInfo ApplySpecialCases(SelectionInfo selectionInfo, SourceText
         if (selectionInfo.Status.Failed)
             return selectionInfo;
 
-        if (selectionInfo.CommonRootFromOriginalSpan.IsKind(SyntaxKind.CompilationUnit)
-            || selectionInfo.CommonRootFromOriginalSpan.IsParentKind(SyntaxKind.GlobalStatement))
+        // If we're under a global statement (and not inside an inner lambda/local-function) then there are restrictions
+        // on if we can extract a method vs a local function.
+        if (IsCodeInGlobalLevel())
         {
-            // Cannot extract a local function from a global statement in script code
-            if (localFunction && options is { Kind: SourceCodeKind.Script })
-            {
-                return selectionInfo.WithStatus(s => s.With(succeeded: false, CSharpFeaturesResources.Selection_cannot_include_global_statements));
-            }
-
             // Cannot extract a method from a top-level statement in normal code
             if (!localFunction && options is { Kind: SourceCodeKind.Regular })
-            {
                 return selectionInfo.WithStatus(s => s.With(succeeded: false, CSharpFeaturesResources.Selection_cannot_include_top_level_statements));
-            }
+
+            // Cannot extract a local function from a global statement in script code
+            if (localFunction && options is { Kind: SourceCodeKind.Script })
+                return selectionInfo.WithStatus(s => s.With(succeeded: false, CSharpFeaturesResources.Selection_cannot_include_global_statements));
         }
 
         if (_localFunction)
         {
             foreach (var ancestor in selectionInfo.CommonRootFromOriginalSpan.AncestorsAndSelf())
             {
                 if (ancestor.Kind() is SyntaxKind.BaseConstructorInitializer or SyntaxKind.ThisConstructorInitializer)
-                {
                     return selectionInfo.WithStatus(s => s.With(succeeded: false, CSharpFeaturesResources.Selection_cannot_be_in_constructor_initializer));
-                }
 
                 if (ancestor is AnonymousFunctionExpressionSyntax)
-                {
                     break;
-                }
             }
         }
 
         if (!selectionInfo.SelectionInExpression)
-        {
             return selectionInfo;
-        }
 
         var expressionNode = selectionInfo.FirstTokenInFinalSpan.GetCommonRoot(selectionInfo.LastTokenInFinalSpan);
         if (expressionNode is not AssignmentExpressionSyntax assign)
             return selectionInfo;
 
         // make sure there is a visible token at right side expression
         if (assign.Right.GetLastToken().Kind() == SyntaxKind.None)
-        {
             return selectionInfo;
-        }
 
-        return AssignFinalSpan(selectionInfo.With(s => s.FirstTokenInFinalSpan = assign.Right.GetFirstToken(includeZeroWidth: true))
-                                            .With(s => s.LastTokenInFinalSpan = assign.Right.GetLastToken(includeZeroWidth: true)),
-                               text);
+        return AssignFinalSpan(selectionInfo
+            .With(s => s.FirstTokenInFinalSpan = assign.Right.GetFirstToken(includeZeroWidth: true))
+            .With(s => s.LastTokenInFinalSpan = assign.Right.GetLastToken(includeZeroWidth: true)), text);
+
+        bool IsCodeInGlobalLevel()
+        {
+            for (var current = selectionInfo.CommonRootFromOriginalSpan; current != null; current = current.Parent)
+            {
+                if (current is CompilationUnitSyntax)
+                    return true;
+
+                if (current is GlobalStatementSyntax)
+                    return true;
+
+                if (current is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax or MemberDeclarationSyntax)
+                    return false;
+            }
+
+            throw ExceptionUtilities.Unreachable();
+        }
     }
 
     private static TextSpan GetControlFlowSpan(SelectionInfo selectionInfo)

src/Features/CSharpTest/ExtractMethod/ExtractLocalFunctionTests.cs

@@ -5366,8 +5366,6 @@ static string NewMethod()
             },
             FixedCode = expected,
             LanguageVersion = LanguageVersion.CSharp9,
-            CodeActionIndex = 1,
-            CodeActionEquivalenceKey = nameof(FeaturesResources.Extract_local_function),
         }.RunAsync();
     }
 
@@ -5400,7 +5398,6 @@ static string NewMethod()
             },
             FixedCode = expected,
             LanguageVersion = LanguageVersion.CSharp9,
-            CodeActionIndex = 1,
             CodeActionEquivalenceKey = nameof(FeaturesResources.Extract_local_function),
         }.RunAsync();
     }
@@ -5414,17 +5411,15 @@ public async Task TopLevelStatement_ArgumentInInvocation_InInteractive()
             """;
         var expected =
             """
-            {
-                System.Console.WriteLine({|Rename:NewMethod|}());
+            System.Console.WriteLine({|Rename:NewMethod|}());
 
-                static string NewMethod()
-                {
-                    return "string";
-                }
+            static string NewMethod()
+            {
+                return "string";
             }
             """;
 
-        await TestAsync(code, expected, TestOptions.Script.WithLanguageVersion(LanguageVersion.CSharp9), index: 1);
+        await TestAsync(code, expected, TestOptions.Script.WithLanguageVersion(LanguageVersion.CSharp9));
     }
 
     [Fact]