Skip to content

Commit

Permalink
Merge pull request #1128 from TymurGubayev/fix/SelectCaseObject/1
Browse files Browse the repository at this point in the history
convert `Select Case [object]` using `Operators.ConditionalCompareObject...`
  • Loading branch information
GrahamTheCoder committed Aug 13, 2024
2 parents 1dd49d9 + eb6872d commit 625c207
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 21 deletions.
3 changes: 2 additions & 1 deletion CodeConverter/CSharp/ExpressionNodeVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.VisualBasic;
using Microsoft.VisualBasic.CompilerServices;
using ComparisonKind = ICSharpCode.CodeConverter.CSharp.VisualBasicEqualityComparison.ComparisonKind;

namespace ICSharpCode.CodeConverter.CSharp;

Expand Down Expand Up @@ -916,7 +917,7 @@ private async Task<CSharpSyntaxNode> ConvertBinaryExpressionAsync(VBasic.Syntax.
omitConversion = true; // Already handled within for the appropriate types (rhs can become int in comparison)
break;
case VisualBasicEqualityComparison.RequiredType.Object:
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, ComparisonKind.Equals, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
}

var lhsTypeIgnoringNullable = lhsTypeInfo.Type.GetNullableUnderlyingType() ?? lhsTypeInfo.Type;
Expand Down
79 changes: 61 additions & 18 deletions CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.Text;
using ICSharpCode.CodeConverter.Util.FromRoslyn;
using ICSharpCode.CodeConverter.VB;
using ComparisonKind = ICSharpCode.CodeConverter.CSharp.VisualBasicEqualityComparison.ComparisonKind;

namespace ICSharpCode.CodeConverter.CSharp;

Expand Down Expand Up @@ -744,7 +745,8 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
var csSwitchExpr = await vbExpr.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
csSwitchExpr = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbExpr, csSwitchExpr);
var switchExprTypeInfo = _semanticModel.GetTypeInfo(vbExpr);
var isStringComparison = switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String || switchExprTypeInfo.ConvertedType?.IsArrayOf(SpecialType.System_Char) == true;
var isObjectComparison = switchExprTypeInfo.ConvertedType.SpecialType == SpecialType.System_Object;
var isStringComparison = !isObjectComparison && switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String || switchExprTypeInfo.ConvertedType?.IsArrayOf(SpecialType.System_Char) == true;
var caseInsensitiveStringComparison = vbEquality.OptionCompareTextCaseInsensitive &&
isStringComparison;
if (isStringComparison) {
Expand Down Expand Up @@ -772,32 +774,53 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
var isBooleanCase = caseTypeInfo.Type?.SpecialType == SpecialType.System_Boolean;
bool enumRelated = IsEnumOrNullableEnum(switchExprTypeInfo.ConvertedType) || IsEnumOrNullableEnum(caseTypeInfo.Type);
bool convertingEnum = IsEnumOrNullableEnum(switchExprTypeInfo.ConvertedType) ^ IsEnumOrNullableEnum(caseTypeInfo.Type);
var csExpressionToUse = !isBooleanCase && (convertingEnum || !enumRelated && correctTypeExpressionSyntax.IsConst) ? correctTypeExpressionSyntax.Expr : originalExpressionSyntax;
var csExpressionToUse = !isObjectComparison && !isBooleanCase && (convertingEnum || !enumRelated && correctTypeExpressionSyntax.IsConst)
? correctTypeExpressionSyntax.Expr
: originalExpressionSyntax;

var caseSwitchLabelSyntax = !wrapForStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
var caseSwitchLabelSyntax = !isObjectComparison && !wrapForStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(csExpressionToUse)
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csExpressionToUse, isBooleanCase);
labels.Add(caseSwitchLabelSyntax);
} else if (c is VBSyntax.ElseCaseClauseSyntax) {
labels.Add(SyntaxFactory.DefaultSwitchLabel());
} else if (c is VBSyntax.RelationalCaseClauseSyntax relational) {

var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
ExpressionSyntax csLeft = ValidSyntaxFactory.IdentifierName(varName);
var operatorKind = VBasic.VisualBasicExtensions.Kind(relational);
var csRelationalValue = await relational.Value.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
csRelationalValue = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(relational.Value, csRelationalValue);
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(), csLeft, csRelationalValue);
labels.Add(VarWhen(varName, binaryExp));
CasePatternSwitchLabelSyntax caseSwitchLabelSyntax;
if (isObjectComparison) {
caseSwitchLabelSyntax = WrapInCasePatternSwitchLabelSyntax(node, relational.Value, csRelationalValue, false, operatorKind);
}
else {
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
ExpressionSyntax csLeft = ValidSyntaxFactory.IdentifierName(varName);
csRelationalValue = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(relational.Value, csRelationalValue);
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(), csLeft, csRelationalValue);
caseSwitchLabelSyntax = VarWhen(varName, binaryExp);
}
labels.Add(caseSwitchLabelSyntax);
} else if (c is VBSyntax.RangeCaseClauseSyntax range) {
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
ExpressionSyntax csLeft = ValidSyntaxFactory.IdentifierName(varName);
ExpressionSyntax csCaseVar = ValidSyntaxFactory.IdentifierName(varName);
var lowerBound = await range.LowerBound.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
lowerBound = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(range.LowerBound, lowerBound);
var lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, lowerBound, csLeft);
ExpressionSyntax lowerBoundCheck;
if (isObjectComparison) {
var caseTypeInfo = _semanticModel.GetTypeInfo(range.LowerBound);
lowerBoundCheck = ComparisonAdjustedForStringComparison(node, range.LowerBound, caseTypeInfo, lowerBound, csCaseVar, switchExprTypeInfo, ComparisonKind.LessThanOrEqual);
} else {
lowerBound = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(range.LowerBound, lowerBound);
lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, lowerBound, csCaseVar);
}
var upperBound = await range.UpperBound.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
upperBound = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(range.UpperBound, upperBound);
var upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csLeft, upperBound);
ExpressionSyntax upperBoundCheck;
if (isObjectComparison) {
var caseTypeInfo = _semanticModel.GetTypeInfo(range.UpperBound);
upperBoundCheck = ComparisonAdjustedForStringComparison(node, range.UpperBound, switchExprTypeInfo, csCaseVar, upperBound, caseTypeInfo, ComparisonKind.LessThanOrEqual);
} else {
upperBound = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(range.UpperBound, upperBound);
upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csCaseVar, upperBound);
}
var withinBounds = SyntaxFactory.BinaryExpression(SyntaxKind.LogicalAndExpression, lowerBoundCheck, upperBoundCheck);
labels.Add(VarWhen(varName, withinBounds));
} else {
Expand Down Expand Up @@ -916,7 +939,7 @@ private async Task<bool> IsNeverMutatedAsync(VBSyntax.NameSyntax ns)
return symbol.MatchesKind(SymbolKind.Parameter, SymbolKind.Local) && await CommonConversions.Document.Project.Solution.IsNeverWrittenAsync(symbol, allowedLocation);
}

private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, ExpressionSyntax expression, bool treatAsBoolean = false)
private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, ExpressionSyntax expression, bool treatAsBoolean = false, VBasic.SyntaxKind caseClauseKind = VBasic.SyntaxKind.CaseEqualsClause)
{
var typeInfo = _semanticModel.GetTypeInfo(node.SelectStatement.Expression);

Expand All @@ -930,28 +953,48 @@ private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax
patternMatch = ValidSyntaxFactory.VarPattern(varName);
ExpressionSyntax csLeft = ValidSyntaxFactory.IdentifierName(varName), csRight = expression;
var caseTypeInfo = _semanticModel.GetTypeInfo(vbCase);
expression = EqualsAdjustedForStringComparison(node, vbCase, typeInfo, csLeft, csRight, caseTypeInfo);
expression = ComparisonAdjustedForStringComparison(node, vbCase, typeInfo, csLeft, csRight, caseTypeInfo, GetComparisonKind(caseClauseKind));
}

var colonToken = SyntaxFactory.Token(SyntaxKind.ColonToken);
return SyntaxFactory.CasePatternSwitchLabel(patternMatch, SyntaxFactory.WhenClause(expression), colonToken);
}

private ExpressionSyntax EqualsAdjustedForStringComparison(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, TypeInfo lhsTypeInfo, ExpressionSyntax csLeft, ExpressionSyntax csRight, TypeInfo rhsTypeInfo)
private ComparisonKind GetComparisonKind(VBasic.SyntaxKind caseClauseKind) => caseClauseKind switch {
VBasic.SyntaxKind.CaseLessThanClause => ComparisonKind.LessThan,
VBasic.SyntaxKind.CaseLessThanOrEqualClause => ComparisonKind.LessThanOrEqual,
VBasic.SyntaxKind.CaseEqualsClause => ComparisonKind.Equals,
VBasic.SyntaxKind.CaseNotEqualsClause => ComparisonKind.NotEquals,
VBasic.SyntaxKind.CaseGreaterThanOrEqualClause => ComparisonKind.GreaterThanOrEqual,
VBasic.SyntaxKind.CaseGreaterThanClause => ComparisonKind.GreaterThan,
_ => throw new ArgumentOutOfRangeException(nameof(caseClauseKind), caseClauseKind, null)
};

private ExpressionSyntax ComparisonAdjustedForStringComparison(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, TypeInfo lhsTypeInfo, ExpressionSyntax csLeft, ExpressionSyntax csRight, TypeInfo rhsTypeInfo, ComparisonKind comparisonKind)
{
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
switch (_visualBasicEqualityComparison.GetObjectEqualityType(lhsTypeInfo, rhsTypeInfo)) {
case VisualBasicEqualityComparison.RequiredType.Object:
return vbEquality.GetFullExpressionForVbObjectComparison(csLeft, csRight);
return vbEquality.GetFullExpressionForVbObjectComparison(csLeft, csRight, comparisonKind);
case VisualBasicEqualityComparison.RequiredType.StringOnly:
// We know lhs isn't null, because we always coalesce it in the switch expression
(csLeft, csRight) = vbEquality
.AdjustForVbStringComparison(node.SelectStatement.Expression, csLeft, lhsTypeInfo, true, vbCase, csRight, rhsTypeInfo, false);
break;
}
return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csLeft, csRight);
return SyntaxFactory.BinaryExpression(GetSyntaxKind(comparisonKind), csLeft, csRight);
}

private CS.SyntaxKind GetSyntaxKind(ComparisonKind comparisonKind) => comparisonKind switch {
ComparisonKind.LessThan => CS.SyntaxKind.LessThanExpression,
ComparisonKind.LessThanOrEqual => CS.SyntaxKind.LessThanOrEqualExpression,
ComparisonKind.Equals => CS.SyntaxKind.EqualsExpression,
ComparisonKind.NotEquals => CS.SyntaxKind.NotEqualsExpression,
ComparisonKind.GreaterThanOrEqual => CS.SyntaxKind.GreaterThanOrEqualExpression,
ComparisonKind.GreaterThan => CS.SyntaxKind.GreaterThanExpression,
_ => throw new ArgumentOutOfRangeException(nameof(comparisonKind), comparisonKind, null)
};

public override async Task<SyntaxList<StatementSyntax>> VisitWithBlock(VBSyntax.WithBlockSyntax node)
{
var (lhsExpression, prefixDeclarations, _) = await GetExpressionWithoutSideEffectsAsync(node.WithStatement.Expression, "withBlock");
Expand Down
25 changes: 23 additions & 2 deletions CodeConverter/CSharp/VisualBasicEqualityComparison.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ public enum RequiredType
Object
}

public enum ComparisonKind
{
Unknown,
LessThan,
LessThanOrEqual,
Equals,
NotEquals,
GreaterThanOrEqual,
GreaterThan,
}

public bool OptionCompareTextCaseInsensitive { get; set; }

public LiteralExpressionSyntax OptionCompareTextCaseInsensitiveBoolExpression {
Expand Down Expand Up @@ -250,11 +261,11 @@ private static PrefixUnaryExpressionSyntax Negate(InvocationExpressionSyntax pos
return (csLeft, csRight);
}

public ExpressionSyntax GetFullExpressionForVbObjectComparison(ExpressionSyntax lhs, ExpressionSyntax rhs, bool negate = false)
public ExpressionSyntax GetFullExpressionForVbObjectComparison(ExpressionSyntax lhs, ExpressionSyntax rhs, ComparisonKind comparisonKind, bool negate = false)
{
ExtraUsingDirectives.Add("Microsoft.VisualBasic.CompilerServices");
var optionCompareTextCaseInsensitive = SyntaxFactory.Argument(OptionCompareTextCaseInsensitiveBoolExpression);
var compareObject = SyntaxFactory.InvocationExpression(ValidSyntaxFactory.MemberAccess(nameof(Operators), nameof(Operators.ConditionalCompareObjectEqual)),
var compareObject = SyntaxFactory.InvocationExpression(ValidSyntaxFactory.MemberAccess(nameof(Operators), GetOperationName(comparisonKind)),
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[]
{SyntaxFactory.Argument(lhs), SyntaxFactory.Argument(rhs), optionCompareTextCaseInsensitive})));
return negate ? Negate(compareObject) : compareObject;
Expand All @@ -266,4 +277,14 @@ private static BinaryExpressionSyntax GetCompareTextCaseInsensitiveCompareOption
SyntaxFactory.BinaryExpression(SyntaxKind.BitwiseOrExpression, ValidSyntaxFactory.MemberAccess(nameof(CompareOptions), nameof(CompareOptions.IgnoreCase)), ValidSyntaxFactory.MemberAccess(nameof(CompareOptions), nameof(CompareOptions.IgnoreKanaType))), ValidSyntaxFactory.MemberAccess(nameof(CompareOptions), nameof(CompareOptions.IgnoreWidth))
);
}

private static string GetOperationName(ComparisonKind comparisonKind) => comparisonKind switch {
ComparisonKind.LessThan => nameof(Operators.ConditionalCompareObjectLess),
ComparisonKind.LessThanOrEqual => nameof(Operators.ConditionalCompareObjectLessEqual),
ComparisonKind.Equals => nameof(Operators.ConditionalCompareObjectEqual),
ComparisonKind.NotEquals => nameof(Operators.ConditionalCompareObjectNotEqual),
ComparisonKind.GreaterThanOrEqual => nameof(Operators.ConditionalCompareObjectGreaterEqual),
ComparisonKind.GreaterThan => nameof(Operators.ConditionalCompareObjectGreater),
_ => throw new ArgumentOutOfRangeException(nameof(comparisonKind), comparisonKind, null)
};
}
67 changes: 67 additions & 0 deletions Tests/CSharp/ExpressionTests/ExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2296,6 +2296,73 @@ public int OnLoad()
}");
}

[Fact]
public async Task SelectCaseObjectCaseIntegerAsync()
{
await TestConversionVisualBasicToCSharpAsync(
@"Public Class SelectObjectCaseIntegerTest
Sub S()
Dim o As Object
Dim j As Integer
o = 2.0
Select Case o
Case 1
j = 1
Case 2
j = 2
Case 3 To 4
j = 3
Case > 4
j = 4
Case Else
j = -1
End Select
End Sub
End Class", @"using Microsoft.VisualBasic.CompilerServices; // Install-Package Microsoft.VisualBasic
public partial class SelectObjectCaseIntegerTest
{
public void S()
{
object o;
int j;
o = 2.0d;
switch (o)
{
case var @case when Operators.ConditionalCompareObjectEqual(@case, 1, false):
{
j = 1;
break;
}
case var case1 when Operators.ConditionalCompareObjectEqual(case1, 2, false):
{
j = 2;
break;
}
case var case2 when Operators.ConditionalCompareObjectLessEqual(3, case2, false) && Operators.ConditionalCompareObjectLessEqual(case2, 4, false):
{
j = 3;
break;
}
case var case3 when Operators.ConditionalCompareObjectGreater(case3, 4, false):
{
j = 4;
break;
}
default:
{
j = -1;
break;
}
}
}
}
1 target compilation errors:
CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code");
//BUG: Correct textual output, but requires var pattern syntax construct not available before CodeAnalysis 3
}

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

0 comments on commit 625c207

Please sign in to comment.