Merge pull request #2297 from balajikris/ExtractMethod-528

Extract method in Expression bodied members. Fixes #528

src/EditorFeatures/CSharpTest/CodeActions/IntroduceVariable/IntroduceVariableTests.cs

@@ -2142,7 +2142,8 @@ class T
         }
 
         [WorkItem(528, "http://github.com/dotnet/roslyn/issues/528")]
-        [Fact(Skip = "http://github.com/dotnet/roslyn/issues/971"), Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
+        [WorkItem(971, "http://github.com/dotnet/roslyn/issues/971")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
         public void TestIntroduceLocalInExpressionBodiedMethodWithBlockBodiedAnonymousMethodExpression()
         {
             var code =
@@ -2170,7 +2171,8 @@ class TestClass
         }
 
         [WorkItem(528, "http://github.com/dotnet/roslyn/issues/528")]
-        [Fact(Skip = "http://github.com/dotnet/roslyn/issues/971"), Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
+        [WorkItem(971, "http://github.com/dotnet/roslyn/issues/971")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
         public void TestIntroduceLocalInExpressionBodiedMethodWithSingleLineBlockBodiedAnonymousMethodExpression()
         {
             var code =
@@ -2191,7 +2193,8 @@ class TestClass
         }
 
         [WorkItem(528, "http://github.com/dotnet/roslyn/issues/528")]
-        [Fact(Skip = "http://github.com/dotnet/roslyn/issues/971"), Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
+        [WorkItem(971, "http://github.com/dotnet/roslyn/issues/971")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
         public void TestIntroduceLocalInExpressionBodiedMethodWithBlockBodiedSimpleLambdaExpression()
         {
             var code =
@@ -2244,7 +2247,8 @@ class TestClass
         }
 
         [WorkItem(528, "http://github.com/dotnet/roslyn/issues/528")]
-        [Fact(Skip = "http://github.com/dotnet/roslyn/issues/971"), Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
+        [WorkItem(971, "http://github.com/dotnet/roslyn/issues/971")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
         public void TestIntroduceLocalInExpressionBodiedMethodWithBlockBodiedParenthesizedLambdaExpression()
         {
             var code =
@@ -2297,7 +2301,8 @@ class TestClass
         }
 
         [WorkItem(528, "http://github.com/dotnet/roslyn/issues/528")]
-        [Fact(Skip = "http://github.com/dotnet/roslyn/issues/971"), Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
+        [WorkItem(971, "http://github.com/dotnet/roslyn/issues/971")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)]
         public void TestIntroduceLocalInExpressionBodiedMethodWithBlockBodiedAnonymousMethodExpressionInMethodArgs()
         {
             var code =

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

@@ -431,6 +431,390 @@ private static int NewMethod(int i)
 
             #endregion
 
+            #region ExpressionBodiedMembers
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethod()
+            {
+                var code = @"using System;
+class T
+{
+    int m;
+    int M1() => [|1|] + 2 + 3 + m;
+}";
+                var expected = @"using System;
+class T
+{
+    int m;
+    int M1() => NewMethod() + 2 + 3 + m;
+
+    private static int NewMethod()
+    {
+        return 1;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedOperator()
+            {
+                var code = @"using System;
+class Complex
+{
+    int real; int imaginary;
+    public static Complex operator +(Complex a, Complex b) => a.Add([|b.real + 1|]);
+
+    private Complex Add(int b)
+    {
+        throw new NotImplementedException();
+    }
+}";
+                var expected = @"using System;
+class Complex
+{
+    int real; int imaginary;
+    public static Complex operator +(Complex a, Complex b) => a.Add(NewMethod(b));
+
+    private static int NewMethod(Complex b)
+    {
+        return b.real + 1;
+    }
+
+    private Complex Add(int b)
+    {
+        throw new NotImplementedException();
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedConversionOperator()
+            {
+                var code = @"using System;
+public struct DBBool
+{
+    public static readonly DBBool dbFalse = new DBBool(-1);
+    int value;
+
+    DBBool(int value)
+    {
+        this.value = value;
+    }
+
+    public static implicit operator DBBool(bool x) => x ? new DBBool([|1|]) : dbFalse;
+}";
+                var expected = @"using System;
+public struct DBBool
+{
+    public static readonly DBBool dbFalse = new DBBool(-1);
+    int value;
+
+    DBBool(int value)
+    {
+        this.value = value;
+    }
+
+    public static implicit operator DBBool(bool x) => x ? new DBBool(NewMethod()) : dbFalse;
+
+    private static int NewMethod()
+    {
+        return 1;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedProperty()
+            {
+                var code = @"using System;
+class T
+{
+    int M1 => [|1|] + 2;
+}";
+                var expected = @"using System;
+class T
+{
+    int M1 => NewMethod() + 2;
+
+    private static int NewMethod()
+    {
+        return 1;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedIndexer()
+            {
+                var code = @"using System;
+class SampleCollection<T>
+{
+    private T[] arr = new T[100];
+    public T this[int i] => i > 0 ? arr[[|i + 1|]] : arr[i + 2];
+}";
+                var expected = @"using System;
+class SampleCollection<T>
+{
+    private T[] arr = new T[100];
+    public T this[int i] => i > 0 ? arr[NewMethod(i)] : arr[i + 2];
+
+    private static int NewMethod(int i)
+    {
+        return i + 1;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedIndexer2()
+            {
+                var code = @"using System;
+class SampleCollection<T>
+{
+    private T[] arr = new T[100];
+    public T this[int i] => [|i > 0 ? arr[i + 1]|] : arr[i + 2];
+}";
+                var expected = @"using System;
+class SampleCollection<T>
+{
+    private T[] arr = new T[100];
+    public T this[int i] => NewMethod(i);
+
+    private T NewMethod(int i)
+    {
+        return i > 0 ? arr[i + 1] : arr[i + 2];
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithBlockBodiedAnonymousMethodExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => delegate (int x)
+    {
+        return [|9|];
+    };
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => delegate (int x)
+    {
+        return NewMethod();
+    };
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithSingleLineBlockBodiedAnonymousMethodExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => delegate (int x) { return [|9|]; };
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => delegate (int x) { return NewMethod(); };
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithBlockBodiedSimpleLambdaExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => f =>
+    {
+        return f * [|9|];
+    };
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => f =>
+    {
+        return f * NewMethod();
+    };
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithExpressionBodiedSimpleLambdaExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => f => f * [|9|];
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => f => f * NewMethod();
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithBlockBodiedParenthesizedLambdaExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => (f) =>
+    {
+        return f * [|9|];
+    };
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => (f) =>
+    {
+        return f * NewMethod();
+    };
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithExpressionBodiedParenthesizedLambdaExpression()
+            {
+                var code = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => (f) => f * [|9|];
+}";
+                var expected = @"using System;
+class TestClass
+{
+    Func<int, int> Y() => (f) => f * NewMethod();
+
+    private static int NewMethod()
+    {
+        return 9;
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void ExpressionBodiedMethodWithBlockBodiedAnonymousMethodExpressionInMethodArgs()
+            {
+                var code = @"using System;
+class TestClass
+{
+    public int Prop => Method1(delegate()
+    {
+        return [|8|];
+    });
+
+    private int Method1(Func<int> p)
+    {
+        throw new NotImplementedException();
+    }
+}";
+                var expected = @"using System;
+class TestClass
+{
+    public int Prop => Method1(delegate()
+    {
+        return NewMethod();
+    });
+
+    private static int NewMethod()
+    {
+        return 8;
+    }
+
+    private int Method1(Func<int> p)
+    {
+        throw new NotImplementedException();
+    }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            [WorkItem(528, "https://github.com/dotnet/roslyn/issues/528")]
+            [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+            public void LeadingAndTrailingTriviaOnExpressionBodiedMethod()
+            {
+                var code = @"using System;
+class TestClass
+{
+    int M1() => 1 + 2 + /*not moved*/ [|3|] /*not moved*/;
+
+    void Cat() { }
+}";
+                var expected = @"using System;
+class TestClass
+{
+    int M1() => 1 + 2 + /*not moved*/ NewMethod() /*not moved*/;
+
+    private static int NewMethod()
+    {
+        return 3;
+    }
+
+    void Cat() { }
+}";
+                TestExtractMethod(code, expected);
+            }
+
+            #endregion
+
             [WorkItem(11155, "DevDiv_Projects/Roslyn")]
             [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
             public void AnonymousTypeMember1()

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

@@ -172,6 +172,12 @@ protected override SyntaxNode GetFirstStatementOrInitializerSelectedAtCallSite()
                         scope = this.CSharpSelectionResult.GetContainingScopeOf<ConstructorInitializerSyntax>();
                     }
 
+                    if (scope == null)
+                    {
+                        // This is similar to FieldDeclaration case but we only want to do this if the member has an expression body.
+                        scope = this.CSharpSelectionResult.GetContainingScopeOf<ArrowExpressionClauseSyntax>().Parent;
+                    }
+
                     return scope;
                 }
 

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

@@ -128,9 +128,10 @@ private async Task<IEnumerable<SyntaxNode>> CreateStatementsOrInitializerToInser
             {
                 var selectedNode = this.GetFirstStatementOrInitializerSelectedAtCallSite();
 
-                // field initializer and constructor initializer case
+                // field initializer, constructor initializer, expression bodied member case
                 if (selectedNode is ConstructorInitializerSyntax ||
-                    selectedNode is FieldDeclarationSyntax)
+                    selectedNode is FieldDeclarationSyntax ||
+                    IsExpressionBodiedMember(selectedNode))
                 {
                     var statement = await GetStatementOrInitializerContainingInvocationToExtractedMethodAsync(this.CallSiteAnnotation, cancellationToken).ConfigureAwait(false);
                     return SpecializedCollections.SingletonEnumerable(statement);
@@ -151,6 +152,11 @@ private async Task<IEnumerable<SyntaxNode>> CreateStatementsOrInitializerToInser
                 return statements;
             }
 
+            private bool IsExpressionBodiedMember(SyntaxNode node)
+            {
+                return node is MemberDeclarationSyntax && ((MemberDeclarationSyntax)node).GetExpressionBody() != null;
+            }
+
             private SimpleNameSyntax CreateMethodNameForInvocation()
             {
                 return this.AnalyzerResult.MethodTypeParametersInDeclaration.Count == 0

src/Features/CSharp/ExtractMethod/CSharpSelectionResult.cs

@@ -147,6 +147,14 @@ public SyntaxNode GetInnermostStatementContainer()
                 last = statement;
             }
 
+            // expression bodied member case
+            var expressionBodiedMember = this.GetContainingScopeOf<ArrowExpressionClauseSyntax>();
+            if (expressionBodiedMember != null)
+            {
+                // the class declaration is the innermost statement container, since the method does not have a block body
+                return expressionBodiedMember.Parent.Parent;
+            }
+
             // constructor initializer case
             var constructorInitializer = this.GetContainingScopeOf<ConstructorInitializerSyntax>();
             if (constructorInitializer != null)

src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs

@@ -1059,6 +1059,12 @@ public static bool CheckTopLevel(this SyntaxNode node, TextSpan span)
                 return block.ContainsInBlockBody(span);
             }
 
+            var expressionBodiedMember = node as ArrowExpressionClauseSyntax;
+            if (expressionBodiedMember != null)
+            {
+                return expressionBodiedMember.ContainsInExpressionBodiedMemberBody(span);
+            }
+
             var field = node as FieldDeclarationSyntax;
             if (field != null)
             {
@@ -1107,6 +1113,17 @@ public static bool ContainsInBlockBody(this BlockSyntax block, TextSpan textSpan
             return blockSpan.Contains(textSpan);
         }
 
+        public static bool ContainsInExpressionBodiedMemberBody(this ArrowExpressionClauseSyntax expressionBodiedMember, TextSpan textSpan)
+        {
+            if (expressionBodiedMember == null)
+            {
+                return false;
+            }
+
+            var expressionBodiedMemberBody = TextSpan.FromBounds(expressionBodiedMember.Expression.SpanStart, expressionBodiedMember.Expression.Span.End);
+            return expressionBodiedMemberBody.Contains(textSpan);
+        }
+
         public static IEnumerable<MemberDeclarationSyntax> GetMembers(this SyntaxNode node)
         {
             var compilation = node as CompilationUnitSyntax;