提交 fa0e0b07 编写于 作者: D Dustin Campbell

Fix VB incorrectly reported redundant casts in compound assignment statements

Consider the following code:

```VB
Dim b As Byte
b += CByte(1)
```

The `CByte` cast can be removed if the Option Strict is Off. However, if Option Strict is On, the compiler
produces an error ("Option Strict On disallows implicit conversions from 'Integer' to 'Byte'").

The issue here is that compound statements are not considered by unnecessary cast detection. Unfortunately,
supporting is not as straight forward as I'd hoped for a couple of reasons:

1. There is no compiler API to get the built-in operator used by a VB assignment statement. If there were, this
would be a straightforward fix since removing the cast results in the compiler choosing `System.Int32.op_Add(Int32, Int32)`
rather than `System.Byte.op_Add(Byte, Byte)`.

2. There is a hidden narrowing-numeric conversion of a constant value in the following code:

```VB
b += 1
```

Using the compiler API to classify conversion from the expression `1` to the type of `b` results in a
widening-numeric conversion, which is misleading since the type of `1` is `System.Int32` and the type of `b`
is `System.Byte`.

Fixing the problem requires two changes:

1. Actually support assignment statements in the speculative analyzer and check to see if replacment would break
the compound assignment.
2. Add a special case in the VB speculative analyzer in the case that Option Strict is not Off and the expression
has a constant value. In that case, we use a different compiler API to classify *the type of the expression* `1` to
the type of `b`, rather than classifying *the expression* `1` to the type of `b`.
上级 7b5bb3d6
......@@ -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)
......
......@@ -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;
}
......
......@@ -744,7 +744,7 @@ private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression,
return true;
}
protected bool ReplacementBreaksCompoundAssignExpression(
protected bool ReplacementBreaksCompoundAssignment(
TExpressionSyntax originalLeft,
TExpressionSyntax originalRight,
TExpressionSyntax newLeft,
......
......@@ -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
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册