Merge pull request #1149 from balajikris/ExtractMethod

Fix Extract Method and Generate Variables in Conditional Access Expressions.

Fixes #751.

Properly handle ConditionalAccessExpressions in ExpressionSyntaxExtensions::CanReplaceWithLValue. When conditional access expressions begin with some form of member binding expression (starts with the dot operator), they cannot be replaced with an LValue. @jmarolf contributed commit cf3dd545

This issue does not affect VB as VB language parses this differently than C#.

src/EditorFeatures/CSharpTest/ExtractMethod/SelectionValidatorTests.cs

@@ -1547,5 +1547,123 @@ static void Main()
 }";
             TestSelection(code, expectedFail: true);
         }
+
+        [WorkItem(751, "https://github.com/dotnet/roslyn/issues/751")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public void SimpleConditionalAccessExpressionSelectFirstExpression()
+        {
+            var code = @"using System;
+class Program
+{
+    static void Main(string[] args)
+    {
+        A a = new A();
+        var l = {|r:{|b:a|}?.Length |}?? 0;
+    }
+}
+class A
+{
+    public int Length { get; internal set; }
+}";
+            TestSelection(code);
+        }
+
+        [WorkItem(751, "https://github.com/dotnet/roslyn/issues/751")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public void SimpleConditionalAccessExpressionSelectSecondExpression()
+        {
+            var code = @"using System;
+class Program
+{
+    static void Main(string[] args)
+    {
+        A a = new A();
+        var l = {|r:a?.{|b:Length|}|} ?? 0;
+    }
+}
+class A
+{
+    public int Length { get; internal set; }
+}";
+            TestSelection(code);
+        }
+
+        [WorkItem(751, "https://github.com/dotnet/roslyn/issues/751")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public void NestedConditionalAccessExpressionWithMemberBindingExpression()
+        {
+            var code = @"using System;
+ 
+class Program
+{
+    static void Main(string[] args)
+    {
+        A a = new A();
+        var l = {|r:a?.{|b:Prop|}?.Length |}?? 0;
+    }
+}
+class A
+{
+    public B Prop { get; set; }
+}
+class B
+{
+    public int Length { get; set; }
+}";
+            TestSelection(code);
+        }
+
+        [WorkItem(751, "https://github.com/dotnet/roslyn/issues/751")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public void NestedConditionalAccessExpressionWithMemberBindingExpressionSelectSecondExpression()
+        {
+            var code = @"using System;
+ 
+class Program
+{
+    static void Main(string[] args)
+    {
+        A a = new A();
+        var l = {|r:a?.Prop?.{|b:Length|}|} ?? 0;
+    }
+}
+class A
+{
+    public B Prop { get; set; }
+}
+class B
+{
+    public int Length { get; set; }
+}";
+            TestSelection(code);
+        }
+
+        [WorkItem(751, "https://github.com/dotnet/roslyn/issues/751")]
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public void NestedConditionalAccessExpressionWithInvocationExpression()
+        {
+            var code = @"using System;
+ 
+class Program
+{
+    static void Main(string[] args)
+    {
+        A a = new A();
+        var l = {|r:a?.{|b:Method()|}?.Length |}?? 0;
+    }
+}
+class A
+{
+    public B Method()
+    {
+        return new B();
+    }
+}
+class B
+{
+    public int Length { get; set; }
+}";
+            TestSelection(code);
+        }
     }
 }

src/Features/CSharp/GenerateMember/GenerateVariable/CSharpGenerateVariableService.cs

@@ -121,6 +121,11 @@ protected override bool ContainingTypesOrSelfHasUnsafeKeyword(INamedTypeSymbol c
                 return false;
             }
 
+            if (expression.IsParentKind(SyntaxKind.ConditionalAccessExpression))
+            {
+                return true;
+            }
+
             return expression.CanReplaceWithLValue(document.SemanticModel, cancellationToken);
         }
 

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

@@ -474,6 +474,18 @@ private static bool CanReplace(ISymbol symbol)
                 }
             }
 
+            // If we are a conditional access expression:
+            // case (1) : obj?.Method(), obj1.obj2?.Property
+            // case (2) : obj?.GetAnotherObj()?.Length, obj?.AnotherObj?.Length
+            // in case (1), the entire expression forms the conditional access expression, which can be replaced with an LValue.
+            // in case (2), the nested conditional access expression is ".GetAnotherObj()?.Length" or ".AnotherObj()?.Length"
+            // essentially, the first expression (before the operator) in a nested conditional access expression 
+            // is some form of member binding expression and they cannot be replaced with an LValue.
+            if (expression.IsKind(SyntaxKind.ConditionalAccessExpression))
+            {
+                return expression.Parent.Kind() != SyntaxKind.ConditionalAccessExpression;
+            }
+
             switch (expression.Parent.Kind())
             {
                 case SyntaxKind.InvocationExpression:
@@ -503,7 +515,6 @@ private static bool CanReplace(ISymbol symbol)
                 case SyntaxKind.ExpressionStatement:
                 case SyntaxKind.ArrayInitializerExpression:
                 case SyntaxKind.CollectionInitializerExpression:
-                case SyntaxKind.ConditionalAccessExpression:
                 case SyntaxKind.Argument:
                 case SyntaxKind.AttributeArgument:
                 case SyntaxKind.AnonymousObjectMemberDeclarator: