Fix ExtractMethod refactoring on out vars (#13874)

src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs

@@ -5566,5 +5566,82 @@ class C1
             var file = this.ParseFile(text, parseOptions: TestOptions.Regular);
             Assert.Equal(0, file.Errors().Length);
         }
+
+        [Fact]
+        public void ParseOutVar()
+        {
+            var tree = UsingTree(@"
+class C
+{
+    void Foo()
+    {
+        M(out var x);
+    }
+}", options: TestOptions.Regular.WithTuplesFeature());
+            N(SyntaxKind.CompilationUnit);
+            {
+                N(SyntaxKind.ClassDeclaration);
+                {
+                    N(SyntaxKind.ClassKeyword);
+                    N(SyntaxKind.IdentifierToken, "C");
+                    N(SyntaxKind.OpenBraceToken);
+                    N(SyntaxKind.MethodDeclaration);
+                    {
+                        N(SyntaxKind.PredefinedType);
+                        {
+                            N(SyntaxKind.VoidKeyword);
+                        }
+                        N(SyntaxKind.IdentifierToken, "Foo");
+                        N(SyntaxKind.ParameterList);
+                        {
+                            N(SyntaxKind.OpenParenToken);
+                            N(SyntaxKind.CloseParenToken);
+                        }
+                        N(SyntaxKind.Block);
+                        {
+                            N(SyntaxKind.OpenBraceToken);
+                            N(SyntaxKind.ExpressionStatement);
+                            {
+                                N(SyntaxKind.InvocationExpression);
+                                {
+                                    N(SyntaxKind.IdentifierName);
+                                    {
+                                        N(SyntaxKind.IdentifierToken, "M");
+                                    }
+                                    N(SyntaxKind.ArgumentList);
+                                    {
+                                        N(SyntaxKind.OpenParenToken);
+                                        N(SyntaxKind.Argument);
+                                        {
+                                            N(SyntaxKind.OutKeyword);
+                                            N(SyntaxKind.DeclarationExpression);
+                                            {
+                                                N(SyntaxKind.TypedVariableComponent);
+                                                {
+                                                    N(SyntaxKind.IdentifierName);
+                                                    {
+                                                        N(SyntaxKind.IdentifierToken, "var");
+                                                    }
+                                                    N(SyntaxKind.SingleVariableDesignation);
+                                                    {
+                                                        N(SyntaxKind.IdentifierToken, "x");
+                                                    }
+                                                }
+                                            }
+                                        }
+                                        N(SyntaxKind.CloseParenToken);
+                                    }
+                                }
+                                N(SyntaxKind.SemicolonToken);
+                            }
+                            N(SyntaxKind.CloseBraceToken);
+                        }
+                    }
+                    N(SyntaxKind.CloseBraceToken);
+                }
+                N(SyntaxKind.EndOfFileToken);
+            }
+            EOF();
+        }
     }
 }

src/EditorFeatures/CSharpTest/CodeActions/ExtractMethod/ExtractMethodTests.cs

@@ -442,10 +442,7 @@ public async Task TestTuple()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| (int, int) x = (1, 2); |]  System . Console . WriteLine ( x.Item1 ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.Item1); } private static (int, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.Item1); } private static (int, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -454,10 +451,7 @@ public async Task TestTupleDeclarationWithNames()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| (int a, int b) x = (1, 2); |]  System . Console . WriteLine ( x.a ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int a, int b) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int b) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int a, int b) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int b) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -466,10 +460,7 @@ public async Task TestTupleDeclarationWithSomeNames()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| (int a, int) x = (1, 2); |]  System . Console . WriteLine ( x.a ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int a, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int a, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -478,10 +469,7 @@ public async Task TestTupleLiteralWithNames()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| (int, int) x = (a: 1, b: 2); |]  System . Console . WriteLine ( x.Item1 ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.Item1); } private static (int, int) NewMethod() { return (a: 1, b: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int, int) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.Item1); } private static (int, int) NewMethod() { return (a: 1, b: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -490,10 +478,7 @@ public async Task TestTupleDeclarationAndLiteralWithNames()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| (int a, int b) x = (c: 1, d: 2); |]  System . Console . WriteLine ( x.a ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int a, int b) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int b) NewMethod() { return (c: 1, d: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int a, int b) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.a); } private static (int a, int b) NewMethod() { return (c: 1, d: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -502,10 +487,7 @@ public async Task TestTupleIntoVar()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| var x = (c: 1, d: 2); |]  System . Console . WriteLine ( x.c ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int c, int d) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.c); } private static (int c, int d) NewMethod() { return (c: 1, d: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int c, int d) x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.c); } private static (int c, int d) NewMethod() { return (c: 1, d: 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -514,10 +496,7 @@ public async Task RefactorWithoutSystemValueTuple()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| var x = (c: 1, d: 2); |]  System . Console . WriteLine ( x.c ); } } ",
-@"class Program { static void Main ( string [ ] args ) { object x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.c); } private static object NewMethod() { return (c: 1, d: 2); } }",
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { object x = {|Rename:NewMethod|}(); System.Console.WriteLine(x.c); } private static object NewMethod() { return (c: 1, d: 2); } }");
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -527,10 +506,7 @@ public async Task TestTupleWithNestedNamedTuple()
             // This is not the best refactoring, but this is an edge case
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { [| var x = new System.ValueTuple<int, int, int, int, int, int, int, (string a, string b)>(1, 2, 3, 4, 5, 6, 7, (a: ""hello"", b: ""world"")); |]  System . Console . WriteLine ( x.c ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { (int, int, int, int, int, int, int, string, string) x = {|Rename:NewMethod|}(); System . Console . WriteLine ( x.c ); } private static (int, int, int, int, int, int, int, string, string) NewMethod() { return new System.ValueTuple<int, int, int, int, int, int, int, (string a, string b)>(1, 2, 3, 4, 5, 6, 7, (a: ""hello"", b: ""world"")); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { (int, int, int, int, int, int, int, string, string) x = {|Rename:NewMethod|}(); System . Console . WriteLine ( x.c ); } private static (int, int, int, int, int, int, int, string, string) NewMethod() { return new System.ValueTuple<int, int, int, int, int, int, int, (string a, string b)>(1, 2, 3, 4, 5, 6, 7, (a: ""hello"", b: ""world"")); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -538,10 +514,7 @@ public async Task TestDeconstruction()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { var (x, y) = [| (1, 2) |];  System . Console . WriteLine ( x ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { var (x, y) = {|Rename:NewMethod|}(); System . Console . WriteLine ( x ); } private static (int, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { var (x, y) = {|Rename:NewMethod|}(); System . Console . WriteLine ( x ); } private static (int, int) NewMethod() { return (1, 2); } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)]
@@ -549,10 +522,79 @@ public async Task TestDeconstruction2()
         {
             await TestAsync(
 @"class Program { static void Main ( string [ ] args ) { var (x, y) = (1, 2); var z = [| 3; |]  System . Console . WriteLine ( z ); } } " + TestResources.NetFX.ValueTuple.tuplelib_cs,
-@"class Program { static void Main ( string [ ] args ) { var (x, y) = (1, 2); int z = {|Rename:NewMethod|}(); System . Console . WriteLine ( z ); } private static int NewMethod() { return 3; } }" + TestResources.NetFX.ValueTuple.tuplelib_cs,
-index: 0,
-parseOptions: TestOptions.Regular,
-withScriptOption: true);
+@"class Program { static void Main ( string [ ] args ) { var (x, y) = (1, 2); int z = {|Rename:NewMethod|}(); System . Console . WriteLine ( z ); } private static int NewMethod() { return 3; } }" + TestResources.NetFX.ValueTuple.tuplelib_cs);
         }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        [Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.OutVar)]
+        public async Task TestOutVar()
+        {
+            await TestAsync(
+@"class C { static void M(int i) { int r; [|r = M1(out int y, i);|] System.Console.WriteLine(r + y); } } ",
+@"class C { static void M(int i) { int r; int y; {|Rename:NewMethod|}(i, out r, out y); System.Console.WriteLine(r + y); } 
+private static void NewMethod(int i, out int r, out int y) { r = M1(out y, i); } }");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        [Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Patterns)]
+        public async Task TestIsPattern()
+        {
+            await TestAsync(
+@"class C { static void M(int i) { int r; [|r = M1(3 is int y, i);|] System.Console.WriteLine(r + y); } } ",
+@"class C { static void M(int i) { int r; int y; {|Rename:NewMethod|}(i, out r, out y); System.Console.WriteLine(r + y); }
+private static void NewMethod(int i, out int r, out int y) { r = M1(3 is int {|Conflict:y|}, i); } }");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        [Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Patterns)]
+        public async Task TestOutVarAndIsPattern()
+        {
+            await TestAsync(
+@"class C
+{
+    static void M()
+    {
+        int r;
+        [|r = M1(out /*out*/ int /*int*/ y /*y*/) + M2(3 is int z);|]
+        System.Console.WriteLine(r + y + z);
+    }
+} ",
+@"class C
+{
+    static void M()
+    {
+        int r;
+        int y, z;
+        {|Rename:NewMethod|}(out r, out y, out z);
+        System.Console.WriteLine(r + y + z);
+    }
+
+    private static void NewMethod(out int r, out int y, out int z)
+    {
+        r = M1(out /*out*/  /*int*/ y /*y*/) + M2(3 is int {|Conflict:z|});
     }
+} ",
+compareTokens: false);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        [Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Patterns)]
+        public async Task ConflictingOutVarLocals()
+        {
+            await TestAsync(
+@"class C { static void M() { int r; [| r = M1(out int y); { M2(out int y); System.Console.Write(y); }|] System.Console.WriteLine(r + y); } } ",
+@"class C { static void M() { int r; int y; {|Rename:NewMethod|}(out r, out y); System.Console.WriteLine(r + y); }
+private static void NewMethod(out int r, out int y) { r = M1(out y); { M2(out int y); System.Console.Write(y); } } } ");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        [Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Patterns)]
+        public async Task ConflictingPatternLocals()
+        {
+            await TestAsync(
+@"class C { static void M() { int r; [| r = M1(1 is int y); { M2(2 is int y); System.Console.Write(y); }|] System.Console.WriteLine(r + y); } } ",
+@"class C { static void M() { int r; int y; {|Rename:NewMethod|}(out r, out y); System.Console.WriteLine(r + y); }
+private static void NewMethod(out int r, out int y) { r = M1(1 is int {|Conflict:y|}); { M2(2 is int y); System.Console.Write(y); } } } ");
+        }
+     }
 }
\ No newline at end of file

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeGeneration;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -306,6 +309,8 @@ private OperationStatus CheckActiveStatements(IEnumerable<StatementSyntax> state
                 var variableToRemoveMap = CreateVariableDeclarationToRemoveMap(
                     this.AnalyzerResult.GetVariablesToMoveOutToCallSiteOrDelete(cancellationToken), cancellationToken);
 
+                statements = statements.Select(s => FixDeclarationExpressionsAndDeclarationPatterns(s, variableToRemoveMap));
+
                 foreach (var statement in statements)
                 {
                     var declarationStatement = statement as LocalDeclarationStatementSyntax;
@@ -394,6 +399,76 @@ yield return
                 }
             }
 
+            /// <summary>
+            /// If the statement has an `out var` declaration expression for a variable which
+            /// needs to be removed, we need to turn it into a plain `out` parameter, so that
+            /// it doesn't declare a duplicate variable.
+            /// If the statement has a pattern declaration (such as `3 is int i`) for a variable
+            /// which needs to be removed, we will annotate it as a conflict, since we don't have
+            /// a better refactoring.
+            /// </summary>
+            private StatementSyntax FixDeclarationExpressionsAndDeclarationPatterns(StatementSyntax statement,
+                HashSet<SyntaxAnnotation> variablesToRemove)
+            {
+                var replacements = new Dictionary<SyntaxNode, SyntaxNode>();
+
+                var declarations = statement.DescendantNodes()
+                    .Where(n => n.IsKind(SyntaxKind.DeclarationExpression, SyntaxKind.DeclarationPattern));
+
+                foreach (var node in declarations)
+                {
+                    switch (node.Kind())
+                    {
+                        case SyntaxKind.DeclarationExpression:
+                            var declaration = (DeclarationExpressionSyntax)node;
+                            if (declaration.VariableComponent.Kind() != SyntaxKind.TypedVariableComponent)
+                            {
+                                break;
+                            }
+
+                            var variableComponent = (TypedVariableComponentSyntax)declaration.VariableComponent;
+                            if (variableComponent.Designation.Kind() != SyntaxKind.SingleVariableDesignation)
+                            {
+                                break;
+                            }
+
+                            var designation = (SingleVariableDesignationSyntax)variableComponent.Designation;
+                            var name = designation.Identifier.ValueText;
+                            if (variablesToRemove.HasSyntaxAnnotation(designation))
+                            {
+                                var newLeadingTrivia = new SyntaxTriviaList();
+                                newLeadingTrivia = newLeadingTrivia.AddRange(variableComponent.Type.GetLeadingTrivia());
+                                newLeadingTrivia = newLeadingTrivia.AddRange(variableComponent.Type.GetTrailingTrivia());
+                                newLeadingTrivia = newLeadingTrivia.AddRange(designation.GetLeadingTrivia());
+
+                                replacements.Add(declaration, SyntaxFactory.IdentifierName(designation.Identifier)
+                                    .WithLeadingTrivia(newLeadingTrivia));
+                            }
+
+                            break;
+
+                        case SyntaxKind.DeclarationPattern:
+                            var pattern = (DeclarationPatternSyntax)node;
+                            if (!variablesToRemove.HasSyntaxAnnotation(pattern))
+                            {
+                                break;
+                            }
+
+                            // We don't have a good refactoring for this, so we just annotate the conflict
+                            // For instance, when a local declared by a pattern declaration (`3 is int i`) is
+                            // used outside the block we're trying to extract.
+                            var identifier = pattern.Identifier;
+                            var annotation = ConflictAnnotation.Create(CSharpFeaturesResources.Conflict_s_detected);
+                            var newIdentifier = identifier.WithAdditionalAnnotations(annotation);
+                            replacements.Add(pattern, pattern.WithIdentifier(newIdentifier));
+
+                            break;
+                    }
+                }
+
+                return statement.ReplaceNodes(replacements.Keys, (orig, partiallyReplaced) => replacements[orig]);
+            }
+
             private static SyntaxToken ApplyTriviaFromDeclarationToAssignmentIdentifier(LocalDeclarationStatementSyntax declarationStatement, bool firstVariableToAttachTrivia, VariableDeclaratorSyntax variable)
             {
                 var identifier = variable.Identifier;