Skip to content

Commit d5b62f8

Browse files
GrahamTheCoderGrahamTheCoder
committed
Extract BinaryExpressionConverter
1 parent 39fbf44 commit d5b62f8

2 files changed

Lines changed: 118 additions & 107 deletions

File tree

‎CodeConverter/CSharp/BinaryExpressionConverter.cs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
using ICSharpCode.CodeConverter.Util.FromRoslyn;
2+
3+
namespace ICSharpCode.CodeConverter.CSharp;
4+
5+
internal class BinaryExpressionConverter
6+
{
7+
private readonly SemanticModel _semanticModel;
8+
private readonly IOperatorConverter _operatorConverter;
9+
private readonly VisualBasicEqualityComparison _visualBasicEqualityComparison;
10+
private readonly VisualBasicNullableExpressionsConverter _visualBasicNullableTypesConverter;
11+
public CommonConversions CommonConversions { get; }
12+
13+
public CommentConvertingVisitorWrapper TriviaConvertingExpressionVisitor { get; }
14+
15+
public BinaryExpressionConverter(SemanticModel semanticModel, IOperatorConverter operatorConverter, VisualBasicEqualityComparison visualBasicEqualityComparison,
16+
VisualBasicNullableExpressionsConverter visualBasicNullableTypesConverter, CommonConversions commonConversions)
17+
{
18+
CommonConversions = commonConversions;
19+
_semanticModel = semanticModel;
20+
_operatorConverter = operatorConverter;
21+
_visualBasicEqualityComparison = visualBasicEqualityComparison;
22+
_visualBasicNullableTypesConverter = visualBasicNullableTypesConverter;
23+
TriviaConvertingExpressionVisitor = commonConversions.TriviaConvertingExpressionVisitor;
24+
}
25+
26+
public async Task<CSharpSyntaxNode> ConvertBinaryExpressionAsync(VBSyntax.BinaryExpressionSyntax entryNode)
27+
{
28+
// Walk down the syntax tree for deeply nested binary expressions to avoid stack overflow
29+
// e.g. 3 + 4 + 5 + ...
30+
// Test "DeeplyNestedBinaryExpressionShouldNotStackOverflowAsync()" skipped because it's too slow
31+
32+
CSSyntax.ExpressionSyntax csLhs = null;
33+
int levelsToConvert = 0;
34+
VBSyntax.BinaryExpressionSyntax currentNode = entryNode;
35+
36+
// Walk down the nested levels to count them
37+
for (var nextNode = entryNode; nextNode != null; currentNode = nextNode, nextNode = currentNode.Left as VBSyntax.BinaryExpressionSyntax, levelsToConvert++) {
38+
// Don't go beyond a rewritten operator because that code has many paths that can call VisitBinaryExpression. Passing csLhs through all of that would harm the code quality more than it's worth to help that edge case.
39+
if (await RewriteBinaryOperatorOrNullAsync(nextNode) is { } operatorNode) {
40+
csLhs = operatorNode;
41+
break;
42+
}
43+
}
44+
45+
// Walk back up the same levels converting as we go.
46+
for (; levelsToConvert > 0; currentNode = currentNode!.Parent as VBSyntax.BinaryExpressionSyntax, levelsToConvert--) {
47+
csLhs = (CSSyntax.ExpressionSyntax)await ConvertBinaryExpressionAsync(currentNode, csLhs);
48+
}
49+
50+
return csLhs;
51+
}
52+
53+
private async Task<CSharpSyntaxNode> ConvertBinaryExpressionAsync(VBasic.Syntax.BinaryExpressionSyntax node, CSSyntax.ExpressionSyntax lhs, CSSyntax.ExpressionSyntax rhs = null)
54+
{
55+
lhs ??= await node.Left.AcceptAsync<CSSyntax.ExpressionSyntax>(TriviaConvertingExpressionVisitor);
56+
rhs ??= await node.Right.AcceptAsync<CSSyntax.ExpressionSyntax>(TriviaConvertingExpressionVisitor);
57+
58+
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
59+
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);
60+
61+
ITypeSymbol forceLhsTargetType = null;
62+
bool omitRightConversion = false;
63+
bool omitConversion = false;
64+
if (lhsTypeInfo.Type != null && rhsTypeInfo.Type != null)
65+
{
66+
if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression) &&
67+
!lhsTypeInfo.Type.IsEnumType() && !rhsTypeInfo.Type.IsEnumType() &&
68+
!lhsTypeInfo.Type.IsDateType() && !rhsTypeInfo.Type.IsDateType())
69+
{
70+
omitRightConversion = true;
71+
omitConversion = lhsTypeInfo.Type.SpecialType == SpecialType.System_String ||
72+
rhsTypeInfo.Type.SpecialType == SpecialType.System_String;
73+
if (lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String) {
74+
forceLhsTargetType = CommonConversions.KnownTypes.String;
75+
}
76+
}
77+
}
78+
79+
var objectEqualityType = _visualBasicEqualityComparison.GetObjectEqualityType(node, lhsTypeInfo, rhsTypeInfo);
80+
81+
switch (objectEqualityType) {
82+
case VisualBasicEqualityComparison.RequiredType.StringOnly:
83+
if (lhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
84+
rhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
85+
_visualBasicEqualityComparison.TryConvertToNullOrEmptyCheck(node, lhs, rhs, out CSharpSyntaxNode visitBinaryExpression)) {
86+
return visitBinaryExpression;
87+
}
88+
(lhs, rhs) = _visualBasicEqualityComparison.AdjustForVbStringComparison(node.Left, lhs, lhsTypeInfo, false, node.Right, rhs, rhsTypeInfo, false);
89+
omitConversion = true; // Already handled within for the appropriate types (rhs can become int in comparison)
90+
break;
91+
case VisualBasicEqualityComparison.RequiredType.Object:
92+
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, VisualBasicEqualityComparison.ComparisonKind.Equals, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
93+
}
94+
95+
var lhsTypeIgnoringNullable = lhsTypeInfo.Type.GetNullableUnderlyingType() ?? lhsTypeInfo.Type;
96+
var rhsTypeIgnoringNullable = rhsTypeInfo.Type.GetNullableUnderlyingType() ?? rhsTypeInfo.Type;
97+
omitConversion |= lhsTypeIgnoringNullable != null && rhsTypeIgnoringNullable != null &&
98+
lhsTypeIgnoringNullable.IsEnumType() && SymbolEqualityComparer.Default.Equals(lhsTypeIgnoringNullable, rhsTypeIgnoringNullable)
99+
&& !node.IsKind(VBasic.SyntaxKind.AddExpression, VBasic.SyntaxKind.SubtractExpression, VBasic.SyntaxKind.MultiplyExpression, VBasic.SyntaxKind.DivideExpression, VBasic.SyntaxKind.IntegerDivideExpression, VBasic.SyntaxKind.ModuloExpression)
100+
&& forceLhsTargetType == null;
101+
lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
102+
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs);
103+
104+
var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken();
105+
var op = CS.SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));
106+
107+
var csBinExp = CS.SyntaxFactory.BinaryExpression(kind, lhs, op, rhs);
108+
var exp = _visualBasicNullableTypesConverter.WithBinaryExpressionLogicForNullableTypes(node, lhsTypeInfo, rhsTypeInfo, csBinExp, lhs, rhs);
109+
return node.Parent.IsKind(VBasic.SyntaxKind.SimpleArgument) ? exp : exp.AddParens();
110+
}
111+
112+
private async Task<CSSyntax.ExpressionSyntax> RewriteBinaryOperatorOrNullAsync(VBSyntax.BinaryExpressionSyntax node) =>
113+
await _operatorConverter.ConvertRewrittenBinaryOperatorOrNullAsync(node, TriviaConvertingExpressionVisitor.IsWithinQuery);
114+
}

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 4 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,19 @@ internal partial class ExpressionNodeVisitor : VBasic.VisualBasicSyntaxVisitor<T
3333
private readonly QueryConverter _queryConverter;
3434
private readonly Lazy<IReadOnlyDictionary<ITypeSymbol, string>> _convertMethodsLookupByReturnType;
3535
private readonly LambdaConverter _lambdaConverter;
36-
private readonly VisualBasicNullableExpressionsConverter _visualBasicNullableTypesConverter;
3736
private readonly Dictionary<string, Stack<(SyntaxNode Scope, string TempName)>> _tempNameForAnonymousScope = new();
3837
private readonly HashSet<string> _generatedNames = new(StringComparer.OrdinalIgnoreCase);
3938
private readonly XmlExpressionConverter _xmlExpressionConverter;
4039
private readonly NameExpressionNodeVisitor _nameExpressionNodeVisitor;
4140
private readonly ArgumentConverter _argumentConverter;
41+
private readonly BinaryExpressionConverter _binaryExpressionConverter;
4242

4343
public ExpressionNodeVisitor(SemanticModel semanticModel,
4444
VisualBasicEqualityComparison visualBasicEqualityComparison, ITypeContext typeContext, CommonConversions commonConversions,
4545
HashSet<string> extraUsingDirectives, XmlImportContext xmlImportContext, VisualBasicNullableExpressionsConverter visualBasicNullableTypesConverter)
4646
{
47-
CommonConversions = commonConversions;
4847
_semanticModel = semanticModel;
48+
CommonConversions = commonConversions;;
4949
_lambdaConverter = new LambdaConverter(commonConversions, semanticModel);
5050
_visualBasicEqualityComparison = visualBasicEqualityComparison;
5151
TriviaConvertingExpressionVisitor = new CommentConvertingVisitorWrapper(this, _semanticModel.SyntaxTree);
@@ -57,6 +57,7 @@ public ExpressionNodeVisitor(SemanticModel semanticModel,
5757
_nameExpressionNodeVisitor = new NameExpressionNodeVisitor(semanticModel, _generatedNames, typeContext, extraUsingDirectives, _tempNameForAnonymousScope, _withBlockLhs, commonConversions, _argumentConverter, TriviaConvertingExpressionVisitor);
5858
_visualBasicNullableTypesConverter = visualBasicNullableTypesConverter;
5959
_operatorConverter = VbOperatorConversion.Create(TriviaConvertingExpressionVisitor, semanticModel, visualBasicEqualityComparison, commonConversions.TypeConversionAnalyzer);
60+
_binaryExpressionConverter = new BinaryExpressionConverter(semanticModel, _operatorConverter, visualBasicEqualityComparison, visualBasicNullableTypesConverter, commonConversions);
6061
// If this isn't needed, the assembly with Conversions may not be referenced, so this must be done lazily
6162
_convertMethodsLookupByReturnType =
6263
new Lazy<IReadOnlyDictionary<ITypeSymbol, string>>(() => CreateConvertMethodsLookupByReturnType(semanticModel));
@@ -118,6 +119,7 @@ public override async Task<CSharpSyntaxNode> DefaultVisit(SyntaxNode node)
118119
public override Task<CSharpSyntaxNode> VisitXmlBracketedName(VBSyntax.XmlBracketedNameSyntax node) => _xmlExpressionConverter.ConvertXmlBracketedNameAsync(node);
119120
public override Task<CSharpSyntaxNode> VisitXmlName(VBSyntax.XmlNameSyntax node) => _xmlExpressionConverter.ConvertXmlNameAsync(node);
120121
public override async Task<CSharpSyntaxNode> VisitSimpleArgument(VBasic.Syntax.SimpleArgumentSyntax node) => await _argumentConverter.ConvertSimpleArgumentAsync(node);
122+
public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax.BinaryExpressionSyntax entryNode) => await _binaryExpressionConverter.ConvertBinaryExpressionAsync(entryNode);
121123

122124
public override async Task<CSharpSyntaxNode> VisitGetTypeExpression(VBasic.Syntax.GetTypeExpressionSyntax node)
123125
{
@@ -548,96 +550,6 @@ private CSharpSyntaxNode ConvertAddressOf(VBSyntax.UnaryExpressionSyntax node, E
548550
return expr;
549551
}
550552

551-
public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax.BinaryExpressionSyntax entryNode)
552-
{
553-
// Walk down the syntax tree for deeply nested binary expressions to avoid stack overflow
554-
// e.g. 3 + 4 + 5 + ...
555-
// Test "DeeplyNestedBinaryExpressionShouldNotStackOverflowAsync()" skipped because it's too slow
556-
557-
ExpressionSyntax csLhs = null;
558-
int levelsToConvert = 0;
559-
VBSyntax.BinaryExpressionSyntax currentNode = entryNode;
560-
561-
// Walk down the nested levels to count them
562-
for (var nextNode = entryNode; nextNode != null; currentNode = nextNode, nextNode = currentNode.Left as VBSyntax.BinaryExpressionSyntax, levelsToConvert++) {
563-
// Don't go beyond a rewritten operator because that code has many paths that can call VisitBinaryExpression. Passing csLhs through all of that would harm the code quality more than it's worth to help that edge case.
564-
if (await RewriteBinaryOperatorOrNullAsync(nextNode) is { } operatorNode) {
565-
csLhs = operatorNode;
566-
break;
567-
}
568-
}
569-
570-
// Walk back up the same levels converting as we go.
571-
for (; levelsToConvert > 0; currentNode = currentNode!.Parent as VBSyntax.BinaryExpressionSyntax, levelsToConvert--) {
572-
csLhs = (ExpressionSyntax)await ConvertBinaryExpressionAsync(currentNode, csLhs);
573-
}
574-
575-
return csLhs;
576-
}
577-
578-
private async Task<CSharpSyntaxNode> ConvertBinaryExpressionAsync(VBasic.Syntax.BinaryExpressionSyntax node, ExpressionSyntax lhs = null, ExpressionSyntax rhs = null)
579-
{
580-
lhs ??= await node.Left.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
581-
rhs ??= await node.Right.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
582-
583-
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
584-
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);
585-
586-
ITypeSymbol forceLhsTargetType = null;
587-
bool omitRightConversion = false;
588-
bool omitConversion = false;
589-
if (lhsTypeInfo.Type != null && rhsTypeInfo.Type != null)
590-
{
591-
if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression) &&
592-
!lhsTypeInfo.Type.IsEnumType() && !rhsTypeInfo.Type.IsEnumType() &&
593-
!lhsTypeInfo.Type.IsDateType() && !rhsTypeInfo.Type.IsDateType())
594-
{
595-
omitRightConversion = true;
596-
omitConversion = lhsTypeInfo.Type.SpecialType == SpecialType.System_String ||
597-
rhsTypeInfo.Type.SpecialType == SpecialType.System_String;
598-
if (lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String) {
599-
forceLhsTargetType = CommonConversions.KnownTypes.String;
600-
}
601-
}
602-
}
603-
604-
var objectEqualityType = _visualBasicEqualityComparison.GetObjectEqualityType(node, lhsTypeInfo, rhsTypeInfo);
605-
606-
switch (objectEqualityType) {
607-
case VisualBasicEqualityComparison.RequiredType.StringOnly:
608-
if (lhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
609-
rhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
610-
_visualBasicEqualityComparison.TryConvertToNullOrEmptyCheck(node, lhs, rhs, out CSharpSyntaxNode visitBinaryExpression)) {
611-
return visitBinaryExpression;
612-
}
613-
(lhs, rhs) = _visualBasicEqualityComparison.AdjustForVbStringComparison(node.Left, lhs, lhsTypeInfo, false, node.Right, rhs, rhsTypeInfo, false);
614-
omitConversion = true; // Already handled within for the appropriate types (rhs can become int in comparison)
615-
break;
616-
case VisualBasicEqualityComparison.RequiredType.Object:
617-
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, ComparisonKind.Equals, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
618-
}
619-
620-
var lhsTypeIgnoringNullable = lhsTypeInfo.Type.GetNullableUnderlyingType() ?? lhsTypeInfo.Type;
621-
var rhsTypeIgnoringNullable = rhsTypeInfo.Type.GetNullableUnderlyingType() ?? rhsTypeInfo.Type;
622-
omitConversion |= lhsTypeIgnoringNullable != null && rhsTypeIgnoringNullable != null &&
623-
lhsTypeIgnoringNullable.IsEnumType() && SymbolEqualityComparer.Default.Equals(lhsTypeIgnoringNullable, rhsTypeIgnoringNullable)
624-
&& !node.IsKind(VBasic.SyntaxKind.AddExpression, VBasic.SyntaxKind.SubtractExpression, VBasic.SyntaxKind.MultiplyExpression, VBasic.SyntaxKind.DivideExpression, VBasic.SyntaxKind.IntegerDivideExpression, VBasic.SyntaxKind.ModuloExpression)
625-
&& forceLhsTargetType == null;
626-
lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
627-
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs);
628-
629-
var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken();
630-
var op = SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));
631-
632-
var csBinExp = SyntaxFactory.BinaryExpression(kind, lhs, op, rhs);
633-
var exp = _visualBasicNullableTypesConverter.WithBinaryExpressionLogicForNullableTypes(node, lhsTypeInfo, rhsTypeInfo, csBinExp, lhs, rhs);
634-
return node.Parent.IsKind(VBasic.SyntaxKind.SimpleArgument) ? exp : exp.AddParens();
635-
}
636-
637-
638-
639-
private async Task<ExpressionSyntax> RewriteBinaryOperatorOrNullAsync(VBSyntax.BinaryExpressionSyntax node) =>
640-
await _operatorConverter.ConvertRewrittenBinaryOperatorOrNullAsync(node, TriviaConvertingExpressionVisitor.IsWithinQuery);
641553

642554
public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBasic.Syntax.SingleLineLambdaExpressionSyntax node)
643555
{
@@ -941,21 +853,6 @@ private static InvocationExpressionSyntax Invoke(ExpressionSyntax toInvoke, Expr
941853
);
942854
}
943855

944-
private ExpressionSyntax GetConvertMethodForKeywordOrNull(SyntaxNode type)
945-
{
946-
var targetType = _semanticModel.GetTypeInfo(type).Type;
947-
return GetConvertMethodForKeywordOrNull(targetType);
948-
}
949-
950-
private ExpressionSyntax GetConvertMethodForKeywordOrNull(ITypeSymbol targetType)
951-
{
952-
_extraUsingDirectives.Add(ConvertType.Namespace);
953-
return targetType != null &&
954-
_convertMethodsLookupByReturnType.Value.TryGetValue(targetType, out var convertMethodName)
955-
? SyntaxFactory.ParseExpression(convertMethodName)
956-
: null;
957-
}
958-
959856
private SyntaxToken ConvertIdentifier(SyntaxToken identifierIdentifier, bool isAttribute = false)
960857
{
961858
return CommonConversions.ConvertIdentifier(identifierIdentifier, isAttribute);

0 commit comments

Comments
 (0)