Skip to content

Commit

Permalink
Convert nullable bool into bool where necessary - fixes #712
Browse files Browse the repository at this point in the history 
The slightly surprising case here is that "Not Nothing" is False
  • Loading branch information
@GrahamTheCoder
GrahamTheCoder committed Sep 26, 2021
1 parent 694011d commit 28d1bef
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
### VB -> C#

* Convert VB exclamation mark into C# indexer [#765](https://github.com/icsharpcode/CodeConverter/issues/765)
* Deal with nullable bools in binary expressions [#712](https://github.com/icsharpcode/CodeConverter/issues/712)

### C# -> VB

Expand Down
58 changes: 35 additions & 23 deletions CodeConverter/CSharp/ExpressionNodeVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,33 +709,31 @@ public override async Task<CSharpSyntaxNode> VisitUnaryExpression(VBasic.Syntax.
}
var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
SyntaxKind csTokenKind = CSharpUtil.GetExpressionOperatorTokenKind(kind);
if (kind == SyntaxKind.LogicalNotExpression && await NegateAndSimplifyOrNullAsync(node, expr) is { } simpleNegation) {
return AsBool(node, simpleNegation);
}

if (kind == SyntaxKind.LogicalNotExpression &&
_semanticModel.GetTypeInfo(node.Operand).ConvertedType is {} t &&
(t.IsNumericType() || t.IsEnumType())) {
csTokenKind = SyntaxKind.TildeToken;

if (kind == SyntaxKind.LogicalNotExpression && _semanticModel.GetTypeInfo(node.Operand).ConvertedType is { } t) {
if (t.IsNumericType() || t.IsEnumType()) {
csTokenKind = SyntaxKind.TildeToken;
} else if (await NegateAndSimplifyOrNullAsync(node, expr, t) is { } simpleNegation) {
return simpleNegation;
}
}

return SyntaxFactory.PrefixUnaryExpression(
kind,
SyntaxFactory.Token(csTokenKind),
expr.AddParens()
);
}

private ExpressionSyntax AsBool(VBSyntax.UnaryExpressionSyntax node, ExpressionSyntax expr)
{
return CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Operand, expr, forceTargetType: _vbBooleanTypeSymbol);
}

private async Task<ExpressionSyntax> NegateAndSimplifyOrNullAsync(VBSyntax.UnaryExpressionSyntax node, ExpressionSyntax expr)
private async Task<ExpressionSyntax> NegateAndSimplifyOrNullAsync(VBSyntax.UnaryExpressionSyntax node, ExpressionSyntax expr, ITypeSymbol operandConvertedType)
{
if (await _operatorConverter.ConvertReferenceOrNothingComparisonOrNullAsync(node.Operand.SkipIntoParens(), true) is { } nothingComparison) {
return nothingComparison;
} else if (expr is BinaryExpressionSyntax bes && bes.OperatorToken.IsKind(SyntaxKind.EqualsToken)) {
return bes.WithOperatorToken(SyntaxFactory.Token(SyntaxKind.ExclamationEqualsToken));
} else if (operandConvertedType.GetNullableUnderlyingType()?.SpecialType == SpecialType.System_Boolean) {
return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, expr, LiteralConversions.GetLiteralExpression(false));
} else if (expr is BinaryExpressionSyntax eq && eq.OperatorToken.IsKind(SyntaxKind.EqualsEqualsToken, SyntaxKind.ExclamationEqualsToken)){
return eq.WithOperatorToken(SyntaxFactory.Token(eq.OperatorToken.IsKind(SyntaxKind.ExclamationEqualsToken) ? SyntaxKind.EqualsEqualsToken : SyntaxKind.ExclamationEqualsToken));
}

return null;
Expand Down Expand Up @@ -764,14 +762,27 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
var rhs = await node.Right.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);

ITypeSymbol forceLhsTargetType = null;
ITypeSymbol forceRhsTargetType = null;
bool omitRightConversion = false;
bool omitConversion = false;
if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression) && !lhsTypeInfo.Type.IsEnumType() && !rhsTypeInfo.Type.IsEnumType()) {
omitRightConversion = true;
omitConversion = lhsTypeInfo.Type.SpecialType == SpecialType.System_String ||
rhsTypeInfo.Type.SpecialType == SpecialType.System_String;
if (lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String) {
forceLhsTargetType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
if (lhsTypeInfo.Type != null && rhsTypeInfo.Type != null)
{
if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression) &&
!lhsTypeInfo.Type.IsEnumType() && !rhsTypeInfo.Type.IsEnumType())
{
omitRightConversion = true;
omitConversion = lhsTypeInfo.Type.SpecialType == SpecialType.System_String ||
rhsTypeInfo.Type.SpecialType == SpecialType.System_String;
if (lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String) {
forceLhsTargetType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
}
}
else if (node.AlwaysHasBooleanTypeInCSharp()) {
if (!node.Left.AlwaysHasBooleanTypeInCSharp() && lhsTypeInfo.Type.GetNullableUnderlyingType()?.SpecialType == SpecialType.System_Boolean) {
forceLhsTargetType = _vbBooleanTypeSymbol;
} else if (!node.Right.AlwaysHasBooleanTypeInCSharp() && rhsTypeInfo.Type.GetNullableUnderlyingType()?.SpecialType == SpecialType.System_Boolean) {
forceRhsTargetType = _vbBooleanTypeSymbol;
}
}
}

Expand All @@ -792,9 +803,10 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax

omitConversion |= lhsTypeInfo.Type != null && rhsTypeInfo.Type != null &&
lhsTypeInfo.Type.IsEnumType() && Equals(lhsTypeInfo.Type, rhsTypeInfo.Type)
&& !node.IsKind(VBasic.SyntaxKind.AddExpression, VBasic.SyntaxKind.SubtractExpression, VBasic.SyntaxKind.MultiplyExpression, VBasic.SyntaxKind.DivideExpression, VBasic.SyntaxKind.IntegerDivideExpression, VBasic.SyntaxKind.ModuloExpression);
&& !node.IsKind(VBasic.SyntaxKind.AddExpression, VBasic.SyntaxKind.SubtractExpression, VBasic.SyntaxKind.MultiplyExpression, VBasic.SyntaxKind.DivideExpression, VBasic.SyntaxKind.IntegerDivideExpression, VBasic.SyntaxKind.ModuloExpression)
&& forceLhsTargetType == null && forceRhsTargetType == null;
lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs);
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs, forceTargetType: forceRhsTargetType);


var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
Expand Down
3 changes: 2 additions & 1 deletion CodeConverter/CSharp/TypeConversionAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using SyntaxKind = Microsoft.CodeAnalysis.CSharp.SyntaxKind;
using TypeSyntax = Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax;
using CSS = Microsoft.CodeAnalysis.CSharp.Syntax;
using TypeInfo = Microsoft.CodeAnalysis.TypeInfo;

namespace ICSharpCode.CodeConverter.CSharp
{
Expand Down Expand Up @@ -304,7 +305,7 @@ private bool TryAnalyzeCsConversion(VBSyntax.ExpressionSyntax vbNode, ITypeSymbo
typeConversionKind = TypeConversionKind.Conversion;
return true;
} else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean) {
typeConversionKind = TypeConversionKind.NullableBool;
typeConversionKind = vbNode.AlwaysHasBooleanTypeInCSharp() ? TypeConversionKind.Identity : TypeConversionKind.NullableBool;
return true;
} else if (csConversion.IsExplicit) {
typeConversionKind = TypeConversionKind.DestructiveCast;
Expand Down
6 changes: 5 additions & 1 deletion CodeConverter/CSharp/VbSyntaxNodeExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using CSSyntax = Microsoft.CodeAnalysis.CSharp.Syntax;
using ICSharpCode.CodeConverter.Util;
using CSSyntax = Microsoft.CodeAnalysis.CSharp.Syntax;
using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
using VBasic = Microsoft.CodeAnalysis.VisualBasic;
using SyntaxFactory = Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
Expand All @@ -22,5 +23,8 @@ public static bool PrecedenceCouldChange(this VBasic.VisualBasicSyntaxNode node)

return parentIsMemberAccessExpression || parentIsNonArgumentExpression && !parentIsBinaryExpression && !parentIsLambda && !parentIsParenthesis;
}

public static bool AlwaysHasBooleanTypeInCSharp(this VBSyntax.ExpressionSyntax node) =>
node.SkipIntoParens().IsKind(VBasic.SyntaxKind.AndAlsoExpression, VBasic.SyntaxKind.AndExpression, VBasic.SyntaxKind.OrElseExpression, VBasic.SyntaxKind.OrExpression, VBasic.SyntaxKind.NotExpression);
}
}
45 changes: 45 additions & 0 deletions Tests/CSharp/ExpressionTests/BinaryExpressionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,51 @@ private void TestMethod(string str)
}");
}

[Fact]
public async Task ImplicitBooleanConversion712Async()
{
await TestConversionVisualBasicToCSharpAsync(@"Class TestClass712
Private Function TestMethod()
Dim var1 As Boolean? = Nothing
Dim var2 As Boolean? = Nothing
Return var1 OrElse Not var2
End Function
End Class", @"
internal partial class TestClass712
{
private object TestMethod()
{
bool? var1 = default;
bool? var2 = default;
return var1 == true || var2 == false;
}
}");
}

[Fact]
public async Task ImplicitIfStatementBooleanConversion712Async()
{
await TestConversionVisualBasicToCSharpAsync(@"Class TestClass712
Private Function TestMethod()
Dim var1 As Boolean? = Nothing
Dim var2 As Boolean? = Nothing
If var1 OrElse Not var2 Then Return True Else Return False
End Function
End Class", @"
internal partial class TestClass712
{
private object TestMethod()
{
bool? var1 = default;
bool? var2 = default;
if (var1 == true || var2 == false)
return true;
else
return false;
}
}");
}

[Fact]
public async Task ConversionInComparisonOperatorAsync()
{
Expand Down

0 comments on commit 28d1bef

Please sign in to comment.