Merge pull request #44453 from sharwell/top-extract-method

Update Extract Method to support top-level statements
dotnet · Sep 19, 2020 · 0457da1 · 0457da1
2 parents e91ccb5 + 6f34b47
commit 0457da1
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 18 deletions.
diff --git a/src/EditorFeatures/CSharpTest/CodeActions/ExtractMethod/ExtractLocalFunctionTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/ExtractMethod/ExtractLocalFunctionTests.cs
@@ -4746,14 +4746,61 @@ void NewMethod()
         [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
         public async Task TopLevelStatement_ArgumentInInvocation()
         {
-            // ExtractLocalFunction currently disabled in the presence of top-level statements
-            // https://github.com/dotnet/roslyn/issues/44260
+            var code = @"
+System.Console.WriteLine([|""string""|]);
+";
+            var expected = @"
+System.Console.WriteLine({|Rename:NewMethod|}());
+
+static string NewMethod()
+{
+    return ""string"";
+}";
+
+            await TestAsync(code, expected, TestOptions.Regular.WithLanguageVersion(LanguageVersion.CSharp9), index: CodeActionIndex);
+        }
+
+        [WorkItem(44260, "https://github.com/dotnet/roslyn/issues/44260")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public async Task TopLevelStatement_InBlock_ArgumentInInvocation()
+        {
+            var code = @"
+{
+    System.Console.WriteLine([|""string""|]);
+}
+";
+            var expected = @"
+{
+    System.Console.WriteLine({|Rename:NewMethod|}());
+
+    static string NewMethod()
+    {
+        return ""string"";
+    }
+}
+";
+
+            await TestInRegularAndScriptAsync(code, expected, parseOptions: TestOptions.Regular.WithLanguageVersion(LanguageVersion.CSharp9), index: CodeActionIndex);
+        }
 
+        [WorkItem(44260, "https://github.com/dotnet/roslyn/issues/44260")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public async Task TopLevelStatement_ArgumentInInvocation_InInteractive()
+        {
             var code = @"
 System.Console.WriteLine([|""string""|]);
 ";
-            await TestExactActionSetOfferedAsync(code, new[] { FeaturesResources.Extract_method },
-                new TestParameters(parseOptions: TestOptions.Regular.WithLanguageVersion(LanguageVersion.CSharp8)));
+            var expected =
+@"{
+    System.Console.WriteLine({|Rename:NewMethod|}());
+
+    static string NewMethod()
+    {
+        return ""string"";
+    }
+}";
+
+            await TestAsync(code, expected, TestOptions.Script.WithLanguageVersion(LanguageVersion.CSharp9), index: CodeActionIndex);
         }
     }
 }
diff --git a/src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodBase.cs b/src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodBase.cs
@@ -12,6 +12,7 @@
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
 using Microsoft.CodeAnalysis.ExtractMethod;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ExtractMethod
@@ -102,7 +103,7 @@ protected static async Task TestExtractMethodAsync(
             {
                 if (expected != "")
                 {
-                    Assert.Equal(expected, actual);
+                    AssertEx.EqualOrDiff(expected, actual);
                 }
                 else
                 {

diff --git a/src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodTests.cs b/src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodTests.cs
@@ -11246,12 +11246,14 @@ public async Task TopLevelStatement_ValueInAssignment()
 ";
             var expected = @"
 bool local;
-local = NewMethod();
 
 bool NewMethod()
 {
     return true;
-}";
+}
+
+local = NewMethod();
+";
             await TestExtractMethodAsync(code, expected);
         }
 

diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.cs
@@ -69,6 +69,13 @@ protected override async Task<InsertionPoint> GetInsertionPointAsync(SemanticDoc
                     return await InsertionPoint.CreateAsync(document, globalStatement.Parent, cancellationToken).ConfigureAwait(false);
                 }
 
+                // check whether the global statement is a statement container
+                if (!globalStatement.Statement.IsStatementContainerNode() && !root.SyntaxTree.IsScript())
+                {
+                    // The extracted function will be a new global statement
+                    return await InsertionPoint.CreateAsync(document, globalStatement.Parent, cancellationToken).ConfigureAwait(false);
+                }
+
                 return await InsertionPoint.CreateAsync(document, globalStatement.Statement, cancellationToken).ConfigureAwait(false);
             }
 

diff --git a/src/Features/Core/Portable/ExtractMethod/AbstractExtractMethodService.cs b/src/Features/Core/Portable/ExtractMethod/AbstractExtractMethodService.cs
@@ -4,9 +4,7 @@
 
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.ExtractMethod
@@ -38,15 +36,6 @@ public async Task<ExtractMethodResult> ExtractMethodAsync(
                 return new FailedExtractMethodResult(selectionResult.Status);
             }
 
-            var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
-            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
-            if (localFunction && syntaxFacts.ContainsGlobalStatement(root))
-            {
-                // ExtractLocalFunction doesn't yet support local functions in top-level statements
-                // https://github.com/dotnet/roslyn/issues/44260
-                return new FailedExtractMethodResult(OperationStatus.FailedWithUnknownReason);
-            }
-
             cancellationToken.ThrowIfCancellationRequested();
 
             // extract method

diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationService.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationService.cs
@@ -482,6 +482,21 @@ public override TDeclarationNode AddStatements<TDeclarationNode>(
             {
                 return (accessorDeclaration.Body == null) ? destinationMember : Cast<TDeclarationNode>(accessorDeclaration.AddBodyStatements(StatementGenerator.GenerateStatements(statements).ToArray()));
             }
+            else if (destinationMember is CompilationUnitSyntax compilationUnit && options is null)
+            {
+                // Insert the new global statement(s) at the end of any current global statements
+                var insertionIndex = compilationUnit.Members.LastIndexOf(memberDeclaration => memberDeclaration.IsKind(SyntaxKind.GlobalStatement)) + 1;
+                var wrappedStatements = StatementGenerator.GenerateStatements(statements).Select(generated => SyntaxFactory.GlobalStatement(generated)).ToArray();
+                return Cast<TDeclarationNode>(compilationUnit.WithMembers(compilationUnit.Members.InsertRange(insertionIndex, wrappedStatements)));
+            }
+            else if (destinationMember is StatementSyntax statement && statement.IsParentKind(SyntaxKind.GlobalStatement))
+            {
+                // We are adding a statement to a global statement in script, where the CompilationUnitSyntax is not a
+                // statement container. If the global statement is not already a block, create a block which can hold
+                // both the original statement and any new statements we are adding to it.
+                var block = statement as BlockSyntax ?? SyntaxFactory.Block(statement);
+                return Cast<TDeclarationNode>(block.AddStatements(StatementGenerator.GenerateStatements(statements).ToArray()));
+            }
             else
             {
                 return AddStatementsWorker(destinationMember, statements, options, cancellationToken);