From 3d3285878d868e50cbd3d55f08e8f828fbbde602 Mon Sep 17 00:00:00 2001 From: Timur Kelman Date: Mon, 5 Aug 2024 11:51:15 +0200 Subject: [PATCH 1/2] convert `Select Case [object]` using `Operators.ConditionalCompareObject...` --- CodeConverter/CSharp/ExpressionNodeVisitor.cs | 3 +- .../MethodBodyExecutableStatementVisitor.cs | 75 +++++++++++++++---- .../CSharp/VisualBasicEqualityComparison.cs | 25 ++++++- .../CSharp/ExpressionTests/ExpressionTests.cs | 67 +++++++++++++++++ 4 files changed, 151 insertions(+), 19 deletions(-) diff --git a/CodeConverter/CSharp/ExpressionNodeVisitor.cs b/CodeConverter/CSharp/ExpressionNodeVisitor.cs index fa1018c1b..dc1eb31e4 100644 --- a/CodeConverter/CSharp/ExpressionNodeVisitor.cs +++ b/CodeConverter/CSharp/ExpressionNodeVisitor.cs @@ -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; @@ -916,7 +917,7 @@ private async Task 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; diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs index 2b041e164..2c334a63d 100644 --- a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs @@ -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; @@ -744,7 +745,8 @@ public override async Task> VisitSelectBlock(VBSynta var csSwitchExpr = await vbExpr.AcceptAsync(_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) { @@ -772,9 +774,11 @@ public override async Task> 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); @@ -786,18 +790,37 @@ public override async Task> VisitSelectBlock(VBSynta ExpressionSyntax csLeft = ValidSyntaxFactory.IdentifierName(varName); var operatorKind = VBasic.VisualBasicExtensions.Kind(relational); var csRelationalValue = await relational.Value.AcceptAsync(_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 { + 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(_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(_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 { @@ -916,7 +939,7 @@ private async Task 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); @@ -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> VisitWithBlock(VBSyntax.WithBlockSyntax node) { var (lhsExpression, prefixDeclarations, _) = await GetExpressionWithoutSideEffectsAsync(node.WithStatement.Expression, "withBlock"); diff --git a/CodeConverter/CSharp/VisualBasicEqualityComparison.cs b/CodeConverter/CSharp/VisualBasicEqualityComparison.cs index 731a36d49..6a70cf5cd 100644 --- a/CodeConverter/CSharp/VisualBasicEqualityComparison.cs +++ b/CodeConverter/CSharp/VisualBasicEqualityComparison.cs @@ -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 { @@ -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; @@ -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) + }; } \ No newline at end of file diff --git a/Tests/CSharp/ExpressionTests/ExpressionTests.cs b/Tests/CSharp/ExpressionTests/ExpressionTests.cs index 2ae355c15..24ff9eee5 100644 --- a/Tests/CSharp/ExpressionTests/ExpressionTests.cs +++ b/Tests/CSharp/ExpressionTests/ExpressionTests.cs @@ -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 case4 when Operators.ConditionalCompareObjectGreater(case4, 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() { From eb6872db10d661790627a4f012ec74f3357958b5 Mon Sep 17 00:00:00 2001 From: Timur Kelman Date: Mon, 5 Aug 2024 12:57:55 +0200 Subject: [PATCH 2/2] don't discard result of `GetUniqueVariableNameInScope` --- CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs | 4 ++-- Tests/CSharp/ExpressionTests/ExpressionTests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs index 2c334a63d..c5978cb73 100644 --- a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs @@ -786,8 +786,6 @@ public override async Task> VisitSelectBlock(VBSynta 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(_expressionVisitor); CasePatternSwitchLabelSyntax caseSwitchLabelSyntax; @@ -795,6 +793,8 @@ public override async Task> VisitSelectBlock(VBSynta 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); diff --git a/Tests/CSharp/ExpressionTests/ExpressionTests.cs b/Tests/CSharp/ExpressionTests/ExpressionTests.cs index 24ff9eee5..1002400dd 100644 --- a/Tests/CSharp/ExpressionTests/ExpressionTests.cs +++ b/Tests/CSharp/ExpressionTests/ExpressionTests.cs @@ -2344,7 +2344,7 @@ public void S() j = 3; break; } - case var case4 when Operators.ConditionalCompareObjectGreater(case4, 4, false): + case var case3 when Operators.ConditionalCompareObjectGreater(case3, 4, false): { j = 4; break;