Make switch case labels const where possible

GrahamTheCoder · GrahamTheCoder · commit 268dbf26c36c · 2020-05-09T17:42:21.000+01:00
Fixes part of #544
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * Converted non-static field initializers moved to constructor - [#281](https://github.com/icsharpcode/CodeConverter/issues/281)
 * Convert assignments using "Mid" built-in function
 * Improve conversion of array initializer types
+* Improve detection of compile-time constant case labels
 
 ### C# -> VB
 
diff --git a/CodeConverter/CSharp/ExpressionEvaluator.cs b/CodeConverter/CSharp/ExpressionEvaluator.cs
@@ -43,20 +43,26 @@ private static IEnumerable<MethodInfo> GetStringsMethods()
                 .Where(g => g.Count() == 1).SelectMany(ms => ms);
         }
 
-        public ExpressionSyntax GetConstantOrNull(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol type, ExpressionSyntax csNode)
+        public (ExpressionSyntax Expr, bool IsCorrectType) GetConstantOrNull(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol type, TypeConversionAnalyzer.TypeConversionKind analyzedConversionKind, ExpressionSyntax csNode)
         {
             var vbOperation = _semanticModel.GetOperation(vbNode).SkipParens(true);
 
             // Guideline tradeoff: Usually would aim for erring on the side of correct runtime behaviour. But making lots of simple constants unreadable for the sake of an edge case that will turn into an easily fixed compile error seems overkill.
             // See https://github.com/icsharpcode/CodeConverter/blob/master/.github/CONTRIBUTING.md#deciding-what-the-output-should-be
-            if (Equals(vbOperation.Type, type) && IsProbablyConstExpression(vbOperation)) return csNode;
-
-            if (TryCompileTimeEvaluate(vbOperation, out var result) && ConversionsTypeFullNames.TryGetValue(type.GetFullMetadataName(), out var method)) {
-                result = method.Invoke(null, new[] { result });
-                return LiteralConversions.GetLiteralExpression(result, convertedType: type);
+            bool isExactType = analyzedConversionKind == TypeConversionAnalyzer.TypeConversionKind.Identity;
+            if ((isExactType || analyzedConversionKind == TypeConversionAnalyzer.TypeConversionKind.NonDestructiveCast)
+                && IsProbablyConstExpression(vbOperation)) return (csNode, isExactType);
+
+            if (TryCompileTimeEvaluate(vbOperation, out var result)) {
+                if (type.Name == "Char" && result is int resultInt) {
+                    result = Strings.ChrW(resultInt);
+                } else if (ConversionsTypeFullNames.TryGetValue(type.GetFullMetadataName(), out var method)) {
+                    result = method.Invoke(null, new[] { result });
+                }
+                return (LiteralConversions.GetLiteralExpression(result, convertedType: type), true);
             }
 
-            return null;
+            return (null, false);
         }
 
         /// <remarks>Deal with cases like "2*PI" without inlining the const</remarks>
@@ -68,16 +74,19 @@ private bool IsProbablyConstExpression(IOperation op)
                 return true;
             }
 
-            if (op is IBinaryOperation bo && IsProbablyConstExpression(bo.LeftOperand) && IsProbablyConstExpression(bo.RightOperand)) {
-                return true;
+            if (op is IBinaryOperation bo) {
+                return IsProbablyConstExpression(bo.LeftOperand) && IsProbablyConstExpression(bo.RightOperand);
+            }
+
+            if (op is IUnaryOperation uo) {
+                return IsProbablyConstExpression(uo.Operand);
             }
 
             return false;
         }
 
         private bool TryCompileTimeEvaluate(IOperation vbOperation, out object result)
         {
-            result = null;
             if (vbOperation.ConstantValue.HasValue) {
                 result = vbOperation.ConstantValue.Value;
                 return true;
diff --git a/CodeConverter/CSharp/ExpressionNodeVisitor.cs b/CodeConverter/CSharp/ExpressionNodeVisitor.cs
@@ -1537,12 +1537,12 @@ private ArgumentListSyntax CreateArgList(IMethodSymbol methodSymbol)
 
         private async Task<CSharpSyntaxNode> SubstituteVisualBasicMethodOrNullAsync(VBasic.Syntax.InvocationExpressionSyntax node)
         {
-            CastExpressionSyntax cSharpSyntaxNode = null;
+            ExpressionSyntax cSharpSyntaxNode = null;
             var symbol = _semanticModel.GetSymbolInfo(node.Expression).ExtractBestMatch<ISymbol>();
             if (symbol?.Name == "ChrW" || symbol?.Name == "Chr") {
-                var args = await ConvertArgumentsAsync(node.ArgumentList);
-                cSharpSyntaxNode = ValidSyntaxFactory.CastExpression(SyntaxFactory.ParseTypeName("char"),
-                    args.Single().Expression);
+                var vbArg = node.ArgumentList.Arguments.Single().GetExpression();
+                var csArg = (ExpressionSyntax) await vbArg.AcceptAsync(TriviaConvertingExpressionVisitor);
+                cSharpSyntaxNode = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbArg, csArg, true, true, true, forceTargetType: _semanticModel.GetTypeInfo(node).Type);
             }
 
             return cSharpSyntaxNode;
diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs
@@ -647,16 +647,12 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
                         var originalExpressionSyntax = (ExpressionSyntax)await s.Value.AcceptAsync(_expressionVisitor);
                         // CSharp requires an explicit cast from the base type (e.g. int) in most cases switching on an enum
                         var typeConversionKind = CommonConversions.TypeConversionAnalyzer.AnalyzeConversion(s.Value);
-                        var expressionSyntax = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(s.Value, originalExpressionSyntax, typeConversionKind, true);
-                        SwitchLabelSyntax caseSwitchLabelSyntax = SyntaxFactory.CaseSwitchLabel(expressionSyntax);
+                        var correctTypeExpressionSyntax = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(s.Value, originalExpressionSyntax, typeConversionKind, true, true);
                         var constantValue = _semanticModel.GetConstantValue(s.Value);
-                        var isRepeatedConstantValue = constantValue.HasValue && !usedConstantValues.Add(constantValue);
-                        if (!constantValue.HasValue || isRepeatedConstantValue ||
-                            (typeConversionKind != TypeConversionAnalyzer.TypeConversionKind.NonDestructiveCast &&
-                             typeConversionKind != TypeConversionAnalyzer.TypeConversionKind.Identity)) {
-                            caseSwitchLabelSyntax =
-                                WrapInCasePatternSwitchLabelSyntax(node, expressionSyntax);
-                        }
+                        var notAlreadyUsed = !constantValue.HasValue || usedConstantValues.Add(constantValue.Value);
+                        var caseSwitchLabelSyntax = correctTypeExpressionSyntax.IsConst && notAlreadyUsed
+                            ? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(correctTypeExpressionSyntax.Expr)
+                            : WrapInCasePatternSwitchLabelSyntax(node, correctTypeExpressionSyntax.Expr);
                         labels.Add(caseSwitchLabelSyntax);
                     } else if (c is VBSyntax.ElseCaseClauseSyntax) {
                         labels.Add(SyntaxFactory.DefaultSwitchLabel());
@@ -731,6 +727,7 @@ private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax
                     SyntaxFactory.DiscardDesignation());
             } else {
                 var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
+                //CodeAnalysis upgrade to 3.0.0 needed for VarPattern. Correct text comes out, but tree is invalid so the tests this will generate "CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code"
                 patternMatch = SyntaxFactory.DeclarationPattern(
                     ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
                 expression = SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, SyntaxFactory.IdentifierName(varName), expression);
diff --git a/CodeConverter/CSharp/TypeConversionAnalyzer.cs b/CodeConverter/CSharp/TypeConversionAnalyzer.cs
@@ -9,7 +9,7 @@
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.FindSymbols;
 using Microsoft.CodeAnalysis.Operations;
-using Microsoft.CodeAnalysis.VisualBasic;
+using VBasic = Microsoft.CodeAnalysis.VisualBasic;
 using Microsoft.VisualBasic;
 using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
 using Microsoft.VisualBasic.CompilerServices;
@@ -24,6 +24,7 @@
 using TypeSyntax = Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using System.Linq.Expressions;
+using Microsoft.CodeAnalysis.VisualBasic.Syntax;
 
 namespace ICSharpCode.CodeConverter.CSharp
 {
@@ -53,29 +54,42 @@ public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic
             csNode = addParenthesisIfNeeded && (conversionKind == TypeConversionKind.DestructiveCast || conversionKind == TypeConversionKind.NonDestructiveCast)
                 ? VbSyntaxNodeExtensions.ParenthesizeIfPrecedenceCouldChange(vbNode, csNode)
                 : csNode;
-            return AddExplicitConversion(vbNode, csNode, conversionKind, addParenthesisIfNeeded, isConst, forceSourceType: forceSourceType, forceTargetType: forceTargetType);
+            return AddExplicitConversion(vbNode, csNode, conversionKind, addParenthesisIfNeeded, isConst, forceSourceType: forceSourceType, forceTargetType: forceTargetType).Expr;
         }
 
-        public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, bool isConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
+        public (ExpressionSyntax Expr, bool IsConst) AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, bool requiresConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
         {
             var typeInfo = ModelExtensions.GetTypeInfo(_semanticModel, vbNode);
             var vbType = forceSourceType ?? typeInfo.Type;
             var vbConvertedType = forceTargetType ?? typeInfo.ConvertedType;
+            bool resultConst = false;
 
-            if (isConst && _expressionEvaluator.GetConstantOrNull(vbNode, vbConvertedType, csNode) is ExpressionSyntax constLiteral) {
-                return constLiteral;
+            if (requiresConst) {
+                var (constExpression, isCorrectType) = _expressionEvaluator.GetConstantOrNull(vbNode, vbConvertedType, conversionKind, csNode);
+                if (isCorrectType) {
+                    return (constExpression, true);
+                }
+                if (constExpression != null) {
+                    csNode = constExpression ?? csNode;
+                    resultConst = true;
+                }
             }
 
-            switch (conversionKind)
-            {
+            var typeConvertedResult = AddTypeConversion(vbNode, csNode, conversionKind, addParenthesisIfNeeded, vbType, vbConvertedType);
+            return (typeConvertedResult, resultConst);
+        }
+
+        private ExpressionSyntax AddTypeConversion(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded, ITypeSymbol vbType, ITypeSymbol vbConvertedType)
+        {
+            switch (conversionKind) {
                 case TypeConversionKind.Unknown:
                 case TypeConversionKind.Identity:
                     return addParenthesisIfNeeded ? VbSyntaxNodeExtensions.ParenthesizeIfPrecedenceCouldChange(vbNode, csNode) : csNode;
                 case TypeConversionKind.DestructiveCast:
                 case TypeConversionKind.NonDestructiveCast:
                     return CreateCast(csNode, vbConvertedType);
                 case TypeConversionKind.Conversion:
-                    return AddExplicitConvertTo(vbNode, csNode, vbType, vbConvertedType);;
+                    return AddExplicitConvertTo(vbNode, csNode, vbType, vbConvertedType); ;
                 case TypeConversionKind.NullableBool:
                     return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csNode,
                         LiteralConversions.GetLiteralExpression(true));
@@ -120,7 +134,7 @@ public TypeConversionKind AnalyzeConversion(Microsoft.CodeAnalysis.VisualBasic.S
                 }
             }
 
-            var vbCompilation = (VisualBasicCompilation) _semanticModel.Compilation;
+            var vbCompilation = (VBasic.VisualBasicCompilation) _semanticModel.Compilation;
             var vbConversion = vbCompilation.ClassifyConversion(vbType, vbConvertedType);
             var csType = GetCSType(vbType, vbNode);
             var csConvertedType = GetCSType(vbConvertedType);
@@ -149,7 +163,7 @@ private ITypeSymbol GetCSType(ITypeSymbol vbType, VBSyntax.ExpressionSyntax vbNo
 
         private bool TryAnalyzeCsConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ITypeSymbol csType,
             ITypeSymbol csConvertedType, Conversion vbConversion, ITypeSymbol vbConvertedType, ITypeSymbol vbType,
-            VisualBasicCompilation vbCompilation, bool isConst, out TypeConversionKind typeConversionKind)
+            VBasic.VisualBasicCompilation vbCompilation, bool isConst, out TypeConversionKind typeConversionKind)
         {
             var csConversion = _csCompilation.ClassifyConversion(csType, csConvertedType);
 
diff --git a/Tests/CSharp/MissingSemanticModelInfo/ExpressionTests.cs b/Tests/CSharp/MissingSemanticModelInfo/ExpressionTests.cs
@@ -258,6 +258,44 @@ public string PositionEnumStringFromConstant(PositionEnum pS)
 CS0246: The type or namespace name 'MissingType' could not be found (are you missing a using directive or an assembly reference?)");
         }
 
+        [Fact]
+        public async Task CastToSameTypeAsync()
+        {
+            await TestConversionVisualBasicToCSharpAsync(@"Public Class CastToSameTypeTest
+
+    Sub PositionEnumFromString(ByVal c As Char)
+        Select Case c
+            Case CChar(""."")
+                Console.WriteLine(1)
+            Case CChar("","")
+                Console.WriteLine(2)
+        End Select
+    End Sub
+End Class",
+    @"using System;
+
+public partial class CastToSameTypeTest
+{
+    public void PositionEnumFromString(char c)
+    {
+        switch (c)
+        {
+            case '.':
+                {
+                    Console.WriteLine(1);
+                    break;
+                }
+
+            case ',':
+                {
+                    Console.WriteLine(2);
+                    break;
+                }
+        }
+    }
+}") ;
+        }
+
         [Fact]
         public async Task UnknownTypeInvocationAsync()
         {
diff --git a/Tests/CSharp/SpecialConversionTests.cs b/Tests/CSharp/SpecialConversionTests.cs
@@ -206,6 +206,43 @@ private string numstr(double aDouble)
 
         return str_Txt;
     }
+}");
+        }
+
+        [Fact]
+        public async Task TestConstCharacterConversionsAsync()
+        {
+            await TestConversionVisualBasicToCSharpAsync(@"Imports System.Data
+
+Class TestConstCharacterConversions
+    Function GetItem(dr As DataRow) As Object
+        Const a As String = Chr(7)
+        Const b As String = ChrW(8)
+        Const t As String = Chr(9)
+        Const n As String = ChrW(10)
+        Const v As String = Chr(11)
+        Const f As String = ChrW(12)
+        Const r As String = Chr(13)
+        Const x As String = Chr(14)
+        Const 字 As String = ChrW(&H5B57)
+   End Function
+End Class", @"using System.Data;
+
+internal partial class TestConstCharacterConversions
+{
+    public object GetItem(DataRow dr)
+    {
+        const string a = ""\a"";
+        const string b = ""\b"";
+        const string t = ""\t"";
+        const string n = ""\n"";
+        const string v = ""\v"";
+        const string f = ""\f"";
+        const string r = ""\r"";
+        const string x = ""\u000e"";
+        const string 字 = ""字"";
+        return default;
+    }
 }");
         }
     }
diff --git a/Tests/CSharp/TypeCastTests.cs b/Tests/CSharp/TypeCastTests.cs
@@ -289,7 +289,7 @@ await TestConversionVisualBasicToCSharpAsync(
 End Sub
 ", @"private void Test()
 {
-    char CR = (char)0xF;
+    char CR = '\u000f';
 }
 ");
         }
@@ -303,7 +303,7 @@ await TestConversionVisualBasicToCSharpAsync(
 End Sub
 ", @"private void Test()
 {
-    char CR = (char)0xF;
+    char CR = '\u000f';
 }
 ");
         }

Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,7 @@ await TestConversionVisualBasicToCSharpAsync(`
`289`	`289`	`End Sub`
`290`	`290`	`", @"private void Test()`
`291`	`291`	`{`
`292`		`- char CR = (char)0xF;`
	`292`	`+ char CR = '\u000f';`
`293`	`293`	`}`
`294`	`294`	`");`
`295`	`295`	`}`
`@@ -303,7 +303,7 @@ await TestConversionVisualBasicToCSharpAsync(`
`303`	`303`	`End Sub`
`304`	`304`	`", @"private void Test()`
`305`	`305`	`{`
`306`		`- char CR = (char)0xF;`
	`306`	`+ char CR = '\u000f';`
`307`	`307`	`}`
`308`	`308`	`");`
`309`	`309`	`}`