Skip to content

Commit 8e09bc7

Browse files
Deal with as many combinations of cases as possible
1 parent c63f2fd commit 8e09bc7

4 files changed

Lines changed: 36 additions & 24 deletions

File tree

CodeConverter/CSharp/ExpressionNodeVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
733733
omitConversion = true; // Already handled within for the appropriate types (rhs can become int in comparison)
734734
break;
735735
case VisualBasicEqualityComparison.RequiredType.Object:
736-
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(node, lhs, rhs);
736+
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
737737
}
738738

739739
omitConversion |= lhsTypeInfo.Type != null && rhsTypeInfo.Type != null &&

CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -642,14 +642,14 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
642642
{
643643
var vbExpr = node.SelectStatement.Expression;
644644
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
645-
var (csSwitchExpr, stmts, csExprToReuse) = await GetExpressionWithoutSideEffectsAsync(vbExpr, "switchExpr");
645+
var (csExprToReuse, stmts, csSwitchExpr) = await GetExpressionWithoutSideEffectsAsync(vbExpr, "switchExpr");
646646
var switchExprTypeInfo = _semanticModel.GetTypeInfo(vbExpr);
647-
var isStringComparison = switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String;
647+
var isStringComparison = switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String || switchExprTypeInfo.ConvertedType?.IsArrayOf(SpecialType.System_Char) == true;
648648
var caseInsensitiveStringComparison = vbEquality.OptionCompareTextCaseInsensitive &&
649649
isStringComparison;
650650
if (isStringComparison) {
651-
csSwitchExpr = vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo);
652651
csExprToReuse = vbEquality.VbCoerceToNonNullString(vbExpr, csExprToReuse, switchExprTypeInfo);
652+
csSwitchExpr = vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo);
653653
}
654654

655655
var usedConstantValues = new HashSet<object>();
@@ -665,15 +665,16 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
665665
var constantValue = _semanticModel.GetConstantValue(s.Value);
666666
var caseTypeInfo = _semanticModel.GetTypeInfo(s.Value);
667667
var notAlreadyUsed = !constantValue.HasValue || usedConstantValues.Add(constantValue.Value);
668-
var csCase = correctTypeExpressionSyntax.Expr;
669668

670669
// Pass both halves in case we can optimize away the check based on the switch expr
671-
if (isStringComparison && !caseInsensitiveStringComparison) {
672-
csCase = vbEquality.VbCoerceToNonNullString(vbExpr, csExprToReuse, switchExprTypeInfo, true, s.Value, correctTypeExpressionSyntax.Expr, caseTypeInfo, false).rhs;
673-
}
674-
var caseSwitchLabelSyntax = !caseInsensitiveStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
675-
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(csCase)
676-
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csCase, caseInsensitiveStringComparison);
670+
var wrapForStringComparison = isStringComparison && (caseInsensitiveStringComparison ||
671+
vbEquality.VbCoerceToNonNullString(vbExpr, csExprToReuse, switchExprTypeInfo, true, s.Value, originalExpressionSyntax, caseTypeInfo, false).rhs != originalExpressionSyntax);
672+
673+
var csExpressionToUse = wrapForStringComparison ? originalExpressionSyntax : correctTypeExpressionSyntax.Expr;
674+
675+
var caseSwitchLabelSyntax = !wrapForStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
676+
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(csExpressionToUse)
677+
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csExpressionToUse, caseInsensitiveStringComparison);
677678
labels.Add(caseSwitchLabelSyntax);
678679
} else if (c is VBSyntax.ElseCaseClauseSyntax) {
679680
labels.Add(SyntaxFactory.DefaultSwitchLabel());
@@ -753,13 +754,16 @@ private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax
753754
patternMatch = SyntaxFactory.DeclarationPattern(
754755
ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
755756
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName), csRight = expression;
756-
if (caseInsensitiveTextComparison) {
757+
var caseTypeInfo = _semanticModel.GetTypeInfo(vbCase);
758+
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
759+
if (vbEquality.GetObjectEqualityType(typeInfo, caseTypeInfo) == VisualBasicEqualityComparison.RequiredType.Object) {
760+
expression = vbEquality.GetFullExpressionForVbObjectComparison(csLeft, csRight);
761+
} else {
757762
// We know lhs isn't null, because we always coalesce it in the switch expression
758-
(csLeft, csRight) = CommonConversions.VisualBasicEqualityComparison
759-
.AdjustForVbStringComparison(node.SelectStatement.Expression, csLeft, typeInfo, true, vbCase, csRight, _semanticModel.GetTypeInfo(vbCase), false);
763+
(csLeft, csRight) = vbEquality
764+
.AdjustForVbStringComparison(node.SelectStatement.Expression, csLeft, typeInfo, true, vbCase, csRight, caseTypeInfo, false);
765+
expression = SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csLeft, csRight);
760766
}
761-
expression = SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csLeft, csRight);
762-
763767
}
764768

765769
var casePatternSwitchLabelSyntax = SyntaxFactory.CasePatternSwitchLabel(patternMatch,

CodeConverter/CSharp/VisualBasicEqualityComparison.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,15 @@ public LiteralExpressionSyntax OptionCompareTextCaseInsensitiveBoolExpression {
5454

5555
public RequiredType GetObjectEqualityType(VBSyntax.BinaryExpressionSyntax node, TypeInfo leftType, TypeInfo rightType)
5656
{
57-
var typeInfos = new[] {leftType, rightType};
57+
var typeInfos = new[] { leftType, rightType };
5858
if (!node.IsKind(VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
5959
return RequiredType.None;
6060
}
61+
return GetObjectEqualityType(typeInfos);
62+
}
6163

64+
public RequiredType GetObjectEqualityType(params TypeInfo[] typeInfos)
65+
{
6266
bool requiresVbEqualityCheck = typeInfos.Any(t => t.Type?.SpecialType == SpecialType.System_Object);
6367

6468
if (typeInfos.All(t => t.Type != null) && typeInfos.All(
@@ -71,7 +75,6 @@ public RequiredType GetObjectEqualityType(VBSyntax.BinaryExpressionSyntax node,
7175
}
7276

7377

74-
7578
public (ExpressionSyntax lhs, ExpressionSyntax rhs) VbCoerceToNonNullString(VBSyntax.ExpressionSyntax vbLeft, ExpressionSyntax csLeft, TypeInfo lhsTypeInfo, bool leftKnownNotNull, VBSyntax.ExpressionSyntax vbRight, ExpressionSyntax csRight, TypeInfo rhsTypeInfo, bool rightKnownNotNull)
7679
{
7780
if (IsNonEmptyStringLiteral(vbLeft) || IsNonEmptyStringLiteral(vbRight)) {
@@ -198,9 +201,14 @@ private static bool IsEmptyString(VBSyntax.LiteralExpressionSyntax les)
198201

199202
private static ExpressionSyntax NegateIfNeeded(VBSyntax.BinaryExpressionSyntax node, InvocationExpressionSyntax positiveExpression)
200203
{
201-
return node.IsKind(VBasic.SyntaxKind.EqualsExpression)
202-
? (ExpressionSyntax) positiveExpression
203-
: SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, positiveExpression);
204+
return node.IsKind(VBasic.SyntaxKind.NotEqualsExpression)
205+
? Negate(positiveExpression)
206+
: (ExpressionSyntax)positiveExpression;
207+
}
208+
209+
private static PrefixUnaryExpressionSyntax Negate(InvocationExpressionSyntax positiveExpression)
210+
{
211+
return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, positiveExpression);
204212
}
205213

206214
public (ExpressionSyntax csLeft, ExpressionSyntax csRight) AdjustForVbStringComparison(VBSyntax.ExpressionSyntax vbLeft, ExpressionSyntax csLeft, TypeInfo lhsTypeInfo, bool leftKnownNotNull, VBSyntax.ExpressionSyntax vbRight, ExpressionSyntax csRight, TypeInfo rhsTypeInfo, bool rightKnownNotNull)
@@ -222,14 +230,14 @@ private static ExpressionSyntax NegateIfNeeded(VBSyntax.BinaryExpressionSyntax n
222230
return (csLeft, csRight);
223231
}
224232

225-
public ExpressionSyntax GetFullExpressionForVbObjectComparison(VBSyntax.BinaryExpressionSyntax node, ExpressionSyntax lhs, ExpressionSyntax rhs)
233+
public ExpressionSyntax GetFullExpressionForVbObjectComparison(ExpressionSyntax lhs, ExpressionSyntax rhs, bool negate = false)
226234
{
227235
ExtraUsingDirectives.Add("Microsoft.VisualBasic.CompilerServices");
228236
var optionCompareTextCaseInsensitive = SyntaxFactory.Argument(OptionCompareTextCaseInsensitiveBoolExpression);
229237
var compareObject = SyntaxFactory.InvocationExpression(ValidSyntaxFactory.MemberAccess(nameof(Operators), nameof(Operators.ConditionalCompareObjectEqual)),
230238
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[]
231239
{SyntaxFactory.Argument(lhs), SyntaxFactory.Argument(rhs), optionCompareTextCaseInsensitive})));
232-
return NegateIfNeeded(node, compareObject);
240+
return negate ? (ExpressionSyntax)Negate(compareObject) : compareObject;
233241
}
234242

235243
private static BinaryExpressionSyntax GetCompareTextCaseInsensitiveCompareOptions()

Tests/CSharp/StatementTests/StatementTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1765,7 +1765,7 @@ public int Add(int x)
17651765
public async Task Issue579SelectCaseWithCaseInsensitiveTextCompareAsync()
17661766
{
17671767
await TestConversionVisualBasicToCSharpAsync(@"
1768-
Option Compare Text
1768+
Option Compare Text ' Comments lost
17691769
17701770
Class Issue579SelectCaseWithCaseInsensitiveTextCompare
17711771
Private Function Test(astr_Temp As String) As Nullable(Of Boolean)

0 commit comments

Comments
 (0)