Skip to content

Commit

Permalink
Merge pull request #860 from Yozer/fix-vb-nullables
Browse files Browse the repository at this point in the history 
Fix binary expressions for nullable types in VB->C# conversion
  • Loading branch information
@GrahamTheCoder
GrahamTheCoder authored Apr 10, 2022
2 parents 7a4bcd1 + 445bbf2 commit db94fb3
Show file tree
Hide file tree
Showing 10 changed files with 1,301 additions and 90 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
### VB -> C#

* Convert immediately executed lambdas without causing a compiler error [#869](https://github.com/icsharpcode/CodeConverter/issues/869)
* Fix binary expressions for nullable types in VB->C# conversion [#840](https://github.com/icsharpcode/CodeConverter/issues/840)

### C# -> VB

Expand Down
3 changes: 2 additions & 1 deletion CodeConverter/CSharp/DeclarationNodeVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ public DeclarationNodeVisitor(Document document, Compilation compilation, Semant
TriviaConvertingDeclarationVisitor = new CommentConvertingVisitorWrapper(this, _semanticModel.SyntaxTree);
var expressionEvaluator = new ExpressionEvaluator(semanticModel, _visualBasicEqualityComparison);
var typeConversionAnalyzer = new TypeConversionAnalyzer(semanticModel, csCompilation, _extraUsingDirectives, _csSyntaxGenerator, expressionEvaluator);
var nullableExpressionsConverter = new VisualBasicNullableExpressionsConverter();
CommonConversions = new CommonConversions(document, semanticModel, typeConversionAnalyzer, csSyntaxGenerator, compilation, csCompilation, _typeContext, _visualBasicEqualityComparison);
var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _typeContext, CommonConversions, _extraUsingDirectives, _xmlImportContext);
var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _typeContext, CommonConversions, _extraUsingDirectives, _xmlImportContext, nullableExpressionsConverter);
_triviaConvertingExpressionVisitor = expressionNodeVisitor.TriviaConvertingExpressionVisitor;
_createMethodBodyVisitorAsync = expressionNodeVisitor.CreateMethodBodyVisitorAsync;
CommonConversions.TriviaConvertingExpressionVisitor = _triviaConvertingExpressionVisitor;
Expand Down
24 changes: 8 additions & 16 deletions CodeConverter/CSharp/ExpressionNodeVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ internal class ExpressionNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSha
private readonly Lazy<IDictionary<ITypeSymbol, string>> _convertMethodsLookupByReturnType;
private readonly LambdaConverter _lambdaConverter;
private readonly INamedTypeSymbol _vbBooleanTypeSymbol;
private readonly VisualBasicNullableExpressionsConverter _visualBasicNullableTypesConverter;

public ExpressionNodeVisitor(SemanticModel semanticModel,
VisualBasicEqualityComparison visualBasicEqualityComparison, ITypeContext typeContext, CommonConversions commonConversions,
HashSet<string> extraUsingDirectives, XmlImportContext xmlImportContext)
HashSet<string> extraUsingDirectives, XmlImportContext xmlImportContext, VisualBasicNullableExpressionsConverter visualBasicNullableTypesConverter)
{
CommonConversions = commonConversions;
_semanticModel = semanticModel;
Expand All @@ -48,6 +49,7 @@ public ExpressionNodeVisitor(SemanticModel semanticModel,
_typeContext = typeContext;
_extraUsingDirectives = extraUsingDirectives;
_xmlImportContext = xmlImportContext;
_visualBasicNullableTypesConverter = visualBasicNullableTypesConverter;
_operatorConverter = VbOperatorConversion.Create(TriviaConvertingExpressionVisitor, semanticModel, visualBasicEqualityComparison, commonConversions.TypeConversionAnalyzer);
// If this isn't needed, the assembly with Conversions may not be referenced, so this must be done lazily
_convertMethodsLookupByReturnType =
Expand Down Expand Up @@ -774,7 +776,6 @@ 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 && _semanticModel.GetTypeInfo(node.Operand).ConvertedType is { } t) {
if (t.IsNumericType() || t.IsEnumType()) {
csTokenKind = SyntaxKind.TildeToken;
Expand All @@ -795,8 +796,7 @@ private async Task<ExpressionSyntax> NegateAndSimplifyOrNullAsync(VBSyntax.Unary
if (await _operatorConverter.ConvertReferenceOrNothingComparisonOrNullAsync(node.Operand.SkipIntoParens(), true) is { } nothingComparison) {
return nothingComparison;
}

if (operandConvertedType.GetNullableUnderlyingType()?.SpecialType == SpecialType.System_Boolean) {
if (operandConvertedType.GetNullableUnderlyingType()?.SpecialType == SpecialType.System_Boolean && node.AlwaysHasBooleanTypeInCSharp()) {
return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, expr, LiteralConversions.GetLiteralExpression(false));
}

Expand Down Expand Up @@ -830,7 +830,6 @@ 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 (lhsTypeInfo.Type != null && rhsTypeInfo.Type != null)
Expand All @@ -846,13 +845,6 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
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;
}
}
}

var objectEqualityType = VisualBasicEqualityComparison.GetObjectEqualityType(node, lhsTypeInfo, rhsTypeInfo);
Expand All @@ -873,16 +865,16 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
omitConversion |= lhsTypeInfo.Type != null && rhsTypeInfo.Type != null &&
lhsTypeInfo.Type.IsEnumType() && SymbolEqualityComparer.IncludeNullability.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)
&& forceLhsTargetType == null && forceRhsTargetType == null;
&& forceLhsTargetType == null;
lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs, forceTargetType: forceRhsTargetType);

rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs);

var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
var op = SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));

var csBinExp = SyntaxFactory.BinaryExpression(kind, lhs, op, rhs);
return node.Parent.IsKind(VBasic.SyntaxKind.SimpleArgument) ? csBinExp : csBinExp.AddParens();
var exp = _visualBasicNullableTypesConverter.WithLogicForNullableTypes(node, lhsTypeInfo, rhsTypeInfo, csBinExp, lhs, rhs);
return node.Parent.IsKind(VBasic.SyntaxKind.SimpleArgument) ? exp : exp.AddParens();
}

private async Task<CSharpSyntaxNode> WithRemovedRedundantConversionOrNullAsync(VBSyntax.InvocationExpressionSyntax conversionNode, ISymbol invocationSymbol)
Expand Down
13 changes: 8 additions & 5 deletions CodeConverter/CSharp/TypeConversionAnalyzer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using ICSharpCode.CodeConverter.Util.FromRoslyn;
using ICSharpCode.CodeConverter.Util.FromRoslyn;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.FindSymbols;
Expand Down Expand Up @@ -47,7 +47,7 @@ public ExpressionSyntax AddExplicitConversion(VBSyntax.ExpressionSyntax vbNode,
{
if (csNode == null) return null;
var conversionKind = AnalyzeConversion(vbNode, defaultToCast, isConst, forceSourceType, forceTargetType);
csNode = addParenthesisIfNeeded && (conversionKind == TypeConversionKind.DestructiveCast || conversionKind == TypeConversionKind.NonDestructiveCast)
csNode = addParenthesisIfNeeded && conversionKind is TypeConversionKind.DestructiveCast or TypeConversionKind.NonDestructiveCast
? vbNode.ParenthesizeIfPrecedenceCouldChange(csNode)
: csNode;
return AddExplicitConversion(vbNode, csNode, conversionKind, addParenthesisIfNeeded, isConst, forceSourceType: forceSourceType, forceTargetType: forceTargetType).Expr;
Expand Down Expand Up @@ -295,10 +295,13 @@ private bool TryAnalyzeCsConversion(VBSyntax.ExpressionSyntax vbNode, ITypeSymbo
} else if (isConvertToString && vbType.SpecialType == SpecialType.System_Object) {
typeConversionKind = TypeConversionKind.Conversion;
return true;
} else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean) {
typeConversionKind = vbNode.AlwaysHasBooleanTypeInCSharp() ? TypeConversionKind.Identity : TypeConversionKind.NullableBool;
}
else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean && vbNode.AlwaysHasBooleanTypeInCSharp() &&
(vbNode is not VBSyntax.BinaryExpressionSyntax and not VBSyntax.UnaryExpressionSyntax || vbNode.IsKind(VBasic.SyntaxKind.AndExpression, VBasic.SyntaxKind.OrExpression, VBasic.SyntaxKind.ExclusiveOrExpression))) {
typeConversionKind = TypeConversionKind.NullableBool;
return true;
} else if (csConversion.IsExplicit) {
}
else if (csConversion.IsExplicit) {
typeConversionKind = TypeConversionKind.DestructiveCast;
return true;
}
Expand Down
14 changes: 11 additions & 3 deletions CodeConverter/CSharp/VbSyntaxNodeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,21 @@ public static bool PrecedenceCouldChange(this VBasic.VisualBasicSyntaxNode node)
{
bool parentIsBinaryExpression = node is VBSyntax.BinaryExpressionSyntax;
bool parentIsLambda = node.Parent is VBSyntax.LambdaExpressionSyntax;
bool parentIsNonArgumentExpression = node.Parent is VBSyntax.ExpressionSyntax && !(node.Parent is VBSyntax.ArgumentSyntax);
bool parentIsNonArgumentExpression = node.Parent is VBSyntax.ExpressionSyntax && node.Parent is not VBSyntax.ArgumentSyntax;
bool parentIsParenthesis = node.Parent is VBSyntax.ParenthesizedExpressionSyntax;
bool parentIsMemberAccessExpression = node.Parent is VBSyntax.MemberAccessExpressionSyntax;

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);
public static bool AlwaysHasBooleanTypeInCSharp(this VBSyntax.ExpressionSyntax vbNode)
{
var parent = vbNode.SkipOutOfParens()?.Parent;

return parent is VBSyntax.SingleLineIfStatementSyntax singleLine && singleLine.Condition == vbNode ||
parent is VBSyntax.IfStatementSyntax ifStatement && ifStatement.Condition == vbNode ||
parent is VBSyntax.ElseIfStatementSyntax elseIfStatement && elseIfStatement.Condition == vbNode ||
parent is VBSyntax.TernaryConditionalExpressionSyntax ternary && ternary.Condition == vbNode ||
parent is VBSyntax.BinaryConditionalExpressionSyntax binary && binary.FirstExpression == vbNode;
}
}
Loading

0 comments on commit db94fb3

Please sign in to comment.