Merge pull request #1915 from mavasani/CastSimplification

Fix cast simplification within ConditionalAccessExpression.

Handle ConditionalAccessExpressionSyntax expressions within speculation analyzer to ensure that semantics of the conditional access expression and the WhenNotNull part don't change on cast simplication.

Fixes #253

src/EditorFeatures/Test2/Simplification/CastSimplificationTests.vb

@@ -4125,9 +4125,10 @@ class Program
 </code>
 
             Test(input, expected)
-        End SUb
+        End Sub
+
         <Fact, Trait(Traits.Feature, Traits.Features.Simplification)>
-    <WorkItem(1067214)>
+        <WorkItem(1067214)>
         Public Sub CSharp_Remove_UnncessaryCastInExpressionBody_Method()
             Dim input =
 <Workspace>
@@ -4149,6 +4150,52 @@ class Program
     public int X() => 0;
 }
 ]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <Fact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(253, "https://github.com/dotnet/roslyn/issues/253")>
+        Public Sub CSharp_DoNotRemove_NecessaryCastInConditionAccess()
+            Dim input =
+<Workspace>
+    <Project Language="C#" CommonReferences="true">
+        <Document><![CDATA[
+public class Class1
+{
+    static void Test(object arg)
+    {
+        var identity = ({|Simplify:(B)arg|})?.A ?? (A)arg;
+    }
+}
+
+class A { }
+class B
+{
+    public A A { get { return null; } }
+}
+]]>
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code><![CDATA[
+public class Class1
+{
+    static void Test(object arg)
+    {
+        var identity = ((B)arg)?.A ?? (A)arg;
+    }
+}
+
+class A { }
+class B
+{
+    public A A { get { return null; } }
+}
+]]>
 </code>
 
             Test(input, expected)
@@ -7115,6 +7162,50 @@ Class Program
     End Sub
 End Class
 ]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <Fact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(253, "https://github.com/dotnet/roslyn/issues/253")>
+        Public Sub VisualBasic_DoNotRemove_NecessaryCastInConditionAccess()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document><![CDATA[
+Option Strict On
+
+Public Class Class1
+    Friend Shared Sub Test(arg As Object)
+        Dim identity = If({|Simplify:TryCast(arg, B)|}?.A, TryCast(arg, A))
+    End Sub
+    Class A
+    End Class
+    Class B
+        Public ReadOnly Property A As A
+    End Class
+End Class
+]]>
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code><![CDATA[
+Option Strict On
+
+Public Class Class1
+    Friend Shared Sub Test(arg As Object)
+        Dim identity = If(TryCast(arg, B)?.A, TryCast(arg, A))
+    End Sub
+    Class A
+    End Class
+    Class B
+        Public ReadOnly Property A As A
+    End Class
+End Class
+]]>
 </code>
 
             Test(input, expected)

src/Workspaces/CSharp/Portable/Utilities/SpeculationAnalyzer.cs

@@ -279,6 +279,10 @@ protected override bool ReplacementChangesSemanticsForNodeLanguageSpecific(Synta
                 // If replacing the node will result in a broken binary expression, we won't remove it.
                 return ReplacementBreaksBinaryExpression((BinaryExpressionSyntax)currentOriginalNode, (BinaryExpressionSyntax)currentReplacedNode);
             }
+            else if (currentOriginalNode.Kind() == SyntaxKind.ConditionalAccessExpression)
+            {
+                return ReplacementBreaksConditionalAccessExpression((ConditionalAccessExpressionSyntax)currentOriginalNode, (ConditionalAccessExpressionSyntax)currentReplacedNode);
+            }
             else if (currentOriginalNode is AssignmentExpressionSyntax)
             {
                 // If replacing the node will result in a broken assignment expression, we won't remove it.
@@ -561,6 +565,14 @@ private bool ReplacementBreaksBinaryExpression(BinaryExpressionSyntax binaryExpr
                 !ImplicitConversionsAreCompatible(binaryExpression, newBinaryExpression);
         }
 
+        private bool ReplacementBreaksConditionalAccessExpression(ConditionalAccessExpressionSyntax conditionalAccessExpression, ConditionalAccessExpressionSyntax newConditionalAccessExpression)
+        {
+            return !SymbolsAreCompatible(conditionalAccessExpression, newConditionalAccessExpression) ||
+                !TypesAreCompatible(conditionalAccessExpression, newConditionalAccessExpression) || 
+                !SymbolsAreCompatible(conditionalAccessExpression.WhenNotNull, newConditionalAccessExpression.WhenNotNull) ||
+                !TypesAreCompatible(conditionalAccessExpression.WhenNotNull, newConditionalAccessExpression.WhenNotNull);
+        }
+
         private bool ReplacementBreaksIsOrAsExpression(BinaryExpressionSyntax originalIsOrAsExpression, BinaryExpressionSyntax newIsOrAsExpression)
         {
             // Special case: Lambda expressions and anonymous delegates cannot appear

src/Workspaces/VisualBasic/Portable/Utilities/SpeculationAnalyzer.vb

@@ -328,6 +328,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
                 End If
 
                 Return Not ImplicitConversionsAreCompatible(originalExpression, newExpression)
+            ElseIf currentOriginalNode.Kind = SyntaxKind.ConditionalAccessExpression
+                Dim originalExpression = DirectCast(currentOriginalNode, ConditionalAccessExpressionSyntax)
+                Dim newExpression = DirectCast(currentReplacedNode, ConditionalAccessExpressionSyntax)
+                Return ReplacementBreaksConditionalAccessExpression(originalExpression, newExpression)
             ElseIf currentOriginalNode.Kind = SyntaxKind.VariableDeclarator Then
                 ' Heuristic: If replacing the node will result in changing the type of a local variable
                 ' that is type-inferred, we won't remove it. It's possible to do this analysis, but it's
@@ -470,6 +474,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
                 Not TypesAreCompatible(binaryExpression, newBinaryExpression)
         End Function
 
+        Private Function ReplacementBreaksConditionalAccessExpression(conditionalAccessExpression As ConditionalAccessExpressionSyntax, newConditionalAccessExpression As ConditionalAccessExpressionSyntax) As Boolean
+            Return Not SymbolsAreCompatible(conditionalAccessExpression, newConditionalAccessExpression) OrElse
+                Not TypesAreCompatible(conditionalAccessExpression, newConditionalAccessExpression) OrElse
+                Not SymbolsAreCompatible(conditionalAccessExpression.WhenNotNull, newConditionalAccessExpression.WhenNotNull) OrElse
+                Not TypesAreCompatible(conditionalAccessExpression.WhenNotNull, newConditionalAccessExpression.WhenNotNull)
+        End Function
+
         Protected Overrides Function GetForEachStatementExpression(forEachStatement As ForEachStatementSyntax) As ExpressionSyntax
             Return forEachStatement.Expression
         End Function