Merge pull request #6361 from DustinCampbell/issue-2560

Fix VB incorrectly reported redundant casts in compound assignment statements

src/EditorFeatures/Test2/Simplification/CastSimplificationTests.vb

@@ -7687,7 +7687,7 @@ End Class
 
         <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
         <WorkItem(995855)>
-        Public Sub VisualBasic_Remove_UnnecessaryCastInTernaryExpression()
+        Public Sub VisualBasic_DontRemove_NecessaryCastInTernaryExpression1()
             Dim input =
 <Workspace>
     <Project Language="Visual Basic" CommonReferences="true">
@@ -7696,7 +7696,109 @@ Class C
     Private Shared Sub Main(args As String())
         Dim s As Byte = 0
         Dim i As Integer = 0
-        s += If(i = 0, {|Simplify:CByte(0)|}, {|Simplify:CByte(0)|})
+        s += If(i = 0, CByte(0), {|Simplify:CByte(0)|})
+    End Sub
+End Class
+]]>
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, CByte(0), CByte(0))
+    End Sub
+End Class
+]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(995855)>
+        Public Sub VisualBasic_DontRemove_NecessaryCastInTernaryExpression2()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, {|Simplify:CByte(0)|}, CByte(0))
+    End Sub
+End Class
+]]>
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, CByte(0), CByte(0))
+    End Sub
+End Class
+]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(995855)>
+        Public Sub VisualBasic_Remove_UnnecessaryCastInTernaryExpression1()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, 0, {|Simplify:CByte(0)|})
+    End Sub
+End Class
+]]>
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, 0, 0)
+    End Sub
+End Class
+]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(995855)>
+        Public Sub VisualBasic_Remove_UnnecessaryCastInTernaryExpression2()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document><![CDATA[
+Class C
+    Private Shared Sub Main(args As String())
+        Dim s As Byte = 0
+        Dim i As Integer = 0
+        s += If(i = 0, {|Simplify:CByte(0)|}, 0)
     End Sub
 End Class
 ]]>
@@ -8201,6 +8303,165 @@ Class C
     End Sub
 End Class
 ]]>
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        Public Sub VisualBasic_Remove_IntegerToByte_OptionStrictOff()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document>
+Option Strict Off
+Class C
+    Sub M()
+        Dim b As Byte
+        b += {|Simplify:CByte(1)|}
+    End Sub
+End Class
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+Option Strict Off
+Class C
+    Sub M()
+        Dim b As Byte
+        b += 1
+    End Sub
+End Class
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        Public Sub VisualBasic_DontRemove_IntegerToByte_OptionStrictOn1()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        b += {|Simplify:CByte(1)|}
+    End Sub
+End Class
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        b += CByte(1)
+    End Sub
+End Class
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        Public Sub VisualBasic_DontRemove_IntegerToByte_OptionStrictOn2()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        Const x = 1
+        b += {|Simplify:CByte(x)|}
+    End Sub
+End Class
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        Const x = 1
+        b += CByte(x)
+    End Sub
+End Class
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        Public Sub VisualBasic_DontRemove_IntegerToByte_OptionStrictOn3()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        Dim x As Integer = 1
+        b += {|Simplify:CByte(x)|}
+    End Sub
+End Class
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        Dim x As Integer = 1
+        b += CByte(x)
+    End Sub
+End Class
+</code>
+
+            Test(input, expected)
+        End Sub
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        Public Sub VisualBasic_DontRemove_IntegerToByte_OptionStrictOn4()
+            Dim input =
+<Workspace>
+    <Project Language="Visual Basic" CommonReferences="true">
+        <Document>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        b = b + {|Simplify:CByte(1)|}
+    End Sub
+End Class
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+Option Strict On
+Class C
+    Sub M()
+        Dim b As Byte
+        b = b + CByte(1)
+    End Sub
+End Class
 </code>
 
             Test(input, expected)

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

@@ -618,7 +618,7 @@ private bool ReplacementBreaksAssignmentExpression(AssignmentExpressionSyntax as
             if (assignmentExpression.IsCompoundAssignExpression() &&
                 assignmentExpression.Kind() != SyntaxKind.LeftShiftAssignmentExpression &&
                 assignmentExpression.Kind() != SyntaxKind.RightShiftAssignmentExpression &&
-                ReplacementBreaksCompoundAssignExpression(assignmentExpression.Left, assignmentExpression.Right, newAssignmentExpression.Left, newAssignmentExpression.Right))
+                ReplacementBreaksCompoundAssignment(assignmentExpression.Left, assignmentExpression.Right, newAssignmentExpression.Left, newAssignmentExpression.Right))
             {
                 return true;
             }

src/Workspaces/Core/Portable/Utilities/AbstractSpeculationAnalyzer.cs

@@ -744,7 +744,7 @@ private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression,
             return true;
         }
 
-        protected bool ReplacementBreaksCompoundAssignExpression(
+        protected bool ReplacementBreaksCompoundAssignment(
             TExpressionSyntax originalLeft,
             TExpressionSyntax originalRight,
             TExpressionSyntax newLeft,

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

@@ -112,7 +112,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
                 semanticModel = semanticModel.ParentModel
             End If
 
-            Dim speculativeModel As semanticModel = Nothing
+            Dim speculativeModel As SemanticModel = Nothing
             Dim statementNode = TryCast(nodeToSpeculate, ExecutableStatementSyntax)
             If statementNode IsNot Nothing Then
                 semanticModel.TryGetSpeculativeSemanticModel(position, statementNode, speculativeModel)
@@ -328,7 +328,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
                 End If
 
                 Return Not ImplicitConversionsAreCompatible(originalExpression, newExpression)
-            ElseIf currentOriginalNode.Kind = SyntaxKind.ConditionalAccessExpression
+            ElseIf TypeOf currentOriginalNode Is AssignmentStatementSyntax Then
+                Dim originalAssignmentStatement = DirectCast(currentOriginalNode, AssignmentStatementSyntax)
+
+                If SyntaxFacts.IsAssignmentStatementOperatorToken(originalAssignmentStatement.OperatorToken.Kind()) Then
+                    Dim newAssignmentStatement = DirectCast(currentReplacedNode, AssignmentStatementSyntax)
+
+                    If ReplacementBreaksCompoundAssignment(originalAssignmentStatement.Left, originalAssignmentStatement.Right, newAssignmentStatement.Left, newAssignmentStatement.Right) Then
+                        Return True
+                    End If
+                End If
+            ElseIf currentOriginalNode.Kind = SyntaxKind.ConditionalAccessExpression Then
                 Dim originalExpression = DirectCast(currentOriginalNode, ConditionalAccessExpressionSyntax)
                 Dim newExpression = DirectCast(currentReplacedNode, ConditionalAccessExpressionSyntax)
                 Return ReplacementBreaksConditionalAccessExpression(originalExpression, newExpression)
@@ -472,7 +482,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
             If SyntaxFacts.IsAssignmentStatementOperatorToken(operatorTokenKind) AndAlso
                 operatorTokenKind <> SyntaxKind.LessThanLessThanEqualsToken AndAlso
                 operatorTokenKind <> SyntaxKind.GreaterThanGreaterThanEqualsToken AndAlso
-                ReplacementBreaksCompoundAssignExpression(binaryExpression.Left, binaryExpression.Right, newBinaryExpression.Left, newBinaryExpression.Right) Then
+                ReplacementBreaksCompoundAssignment(binaryExpression.Left, binaryExpression.Right, newBinaryExpression.Left, newBinaryExpression.Right) Then
                 Return True
             End If
 
@@ -515,6 +525,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
         Protected Overrides Function ConversionsAreCompatible(originalExpression As ExpressionSyntax, originalTargetType As ITypeSymbol, newExpression As ExpressionSyntax, newTargetType As ITypeSymbol) As Boolean
             Dim originalConversion = Me.OriginalSemanticModel.ClassifyConversion(originalExpression, originalTargetType)
             Dim newConversion = Me.SpeculativeSemanticModel.ClassifyConversion(newExpression, newTargetType)
+
+            ' When Option Strict is not Off and the new expression has a constant value, it's possible that
+            ' there Is a hidden narrowing conversion that will be missed. In that case, use the
+            ' conversion between the type of the new expression and the new target type.
+
+            If Me.OriginalSemanticModel.OptionStrict() <> OptionStrict.Off AndAlso
+               Me.SpeculativeSemanticModel.GetConstantValue(newExpression).HasValue Then
+
+                Dim newExpressionType = Me.SpeculativeSemanticModel.GetTypeInfo(newExpression).Type
+                newConversion = Me.OriginalSemanticModel.Compilation.ClassifyConversion(newExpressionType, newTargetType)
+            End If
+
             Return ConversionsAreCompatible(originalConversion, newConversion)
         End Function