Evaluate simple compile time conversions within const declarations

GrahamTheCoder · GrahamTheCoder · commit cde4d4dc2130 · 2020-03-30T10:34:47.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * Convert optional ref parameters - fixes #91
 * Always convert Call statement to method call [#445](https://github.com/icsharpcode/CodeConverter/issues/445)
 * Avoid compilation errors when converting const Dates [#213](https://github.com/icsharpcode/CodeConverter/issues/213)
+* Evaluate simple compile time conversions within const declarations
 
 ### C# -> VB
 
diff --git a/CodeConverter/CSharp/OperationExtensions.cs b/CodeConverter/CSharp/OperationExtensions.cs
@@ -1,4 +1,5 @@
-﻿using Microsoft.CodeAnalysis;
+﻿using ICSharpCode.CodeConverter.Util.FromRoslyn;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Operations;
 
 namespace ICSharpCode.CodeConverter.CSharp
@@ -15,21 +16,12 @@ public static IOperation GetParentIgnoringConversions(this IOperation operation)
             return parent;
         }
 
-        public static IOperation GetIgnoringParentheses(this IOperation operation)
-        {
-            while (operation is IParenthesizedOperation) {
-                operation = operation?.Parent;
-            }
-
-            return operation;
-        }
-
-        public static IOperation GetNonConversionOperation(this IOperation operation)
+        public static IOperation SkipParens(this IOperation operation, bool skipImplicitNumericConvert = false)
         {
             while (true) {
                 switch (operation)
                 {
-                    case IConversionOperation co:
+                    case IConversionOperation co when skipImplicitNumericConvert && co.IsImplicit && co.Conversion.IsNumeric && !co.Conversion.IsUserDefined && co.Operand.Type.IsNumericType() && co.Type.IsNumericType():
                         operation = co.Operand;
                         break;
                     case IParenthesizedOperation po:
diff --git a/CodeConverter/CSharp/TypeConversionAnalyzer.cs b/CodeConverter/CSharp/TypeConversionAnalyzer.cs
@@ -23,6 +23,7 @@
 using SyntaxKind = Microsoft.CodeAnalysis.CSharp.SyntaxKind;
 using TypeSyntax = Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using System.Linq.Expressions;
 
 namespace ICSharpCode.CodeConverter.CSharp
 {
@@ -73,14 +74,19 @@ 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, forceTargetType: forceTargetType);
+            return AddExplicitConversion(vbNode, csNode, conversionKind, addParenthesisIfNeeded, isConst, forceTargetType: forceTargetType);
         }
 
-        public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, ITypeSymbol forceTargetType = null)
+        public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, bool isConst = false, ITypeSymbol forceTargetType = null)
         {
             var typeInfo = ModelExtensions.GetTypeInfo(_semanticModel, vbNode);
             var vbType = typeInfo.Type;
             var vbConvertedType = forceTargetType ?? typeInfo.ConvertedType;
+
+            if (isConst && GetConstantOrNull(vbNode, vbConvertedType, csNode) is ExpressionSyntax constLiteral) {
+                return constLiteral;
+            }
+
             switch (conversionKind)
             {
                 case TypeConversionKind.Unknown:
@@ -90,9 +96,7 @@ public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic
                 case TypeConversionKind.NonDestructiveCast:
                     return CreateCast(csNode, vbConvertedType);
                 case TypeConversionKind.Conversion:
-                    return AddExplicitConvertTo(vbNode, csNode, vbType, vbConvertedType);
-                case TypeConversionKind.ConstConversion:
-                    return ConstantFold(vbNode, vbConvertedType);
+                    return AddExplicitConvertTo(vbNode, csNode, vbType, vbConvertedType);;
                 case TypeConversionKind.NullableBool:
                     return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csNode,
                         LiteralConversions.GetLiteralExpression(true));
@@ -189,7 +193,7 @@ private bool TryAnalyzeCsConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.Ex
                     return true;
                 }
                 if (isConvertToString || vbConversion.IsNarrowing) {
-                    typeConversionKind = isConst ? TypeConversionKind.ConstConversion : TypeConversionKind.Conversion;
+                    typeConversionKind = TypeConversionKind.Conversion;
                     return true;
                 }
             } else if (vbConversion.IsWidening && vbConversion.IsNumeric && csConversion.IsImplicit &&
@@ -207,21 +211,21 @@ private bool TryAnalyzeCsConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.Ex
                     vbCompilation.ClassifyConversion(vbConvertedType,
                         vbCompilation.GetTypeByMetadataName("System.Int32"));
                 if (arithmeticConversion.IsWidening && !arithmeticConversion.IsIdentity) {
-                    typeConversionKind = isConst ? TypeConversionKind.ConstConversion : TypeConversionKind.Conversion;
+                    typeConversionKind = TypeConversionKind.Conversion;
                     return true;
                 }
             } else if (csConversion.IsExplicit && csConversion.IsNumeric && vbConversion.IsNarrowing && isConst) {
                 typeConversionKind = IsImplicitConstantConversion(vbNode) ? TypeConversionKind.Identity : TypeConversionKind.NonDestructiveCast;
                 return true;
             } else if (csConversion.IsExplicit && vbConversion.IsNumeric && vbType.TypeKind != TypeKind.Enum) {
                 typeConversionKind = IsImplicitConstantConversion(vbNode) ? TypeConversionKind.Identity :
-                    isConst ? TypeConversionKind.ConstConversion : TypeConversionKind.Conversion;
+                    TypeConversionKind.Conversion;
                 return true;
             } else if (csConversion.IsExplicit && vbConversion.IsIdentity && csConversion.IsNumeric && vbType.TypeKind != TypeKind.Enum) {
-                typeConversionKind = isConst ? TypeConversionKind.ConstConversion : TypeConversionKind.Conversion;
+                typeConversionKind = TypeConversionKind.Conversion;
                 return true;
             } else if (isConvertToString && vbType.SpecialType == SpecialType.System_Object) {
-                typeConversionKind = isConst ? TypeConversionKind.ConstConversion : TypeConversionKind.Conversion;
+                typeConversionKind = TypeConversionKind.Conversion;
                 return true;
             } else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean) {
                 typeConversionKind = TypeConversionKind.NullableBool;
@@ -260,16 +264,36 @@ private static TypeConversionKind AnalyzeVbConversion(bool alwaysExplicit, IType
             return TypeConversionKind.Unknown;
         }
 
-        private ExpressionSyntax ConstantFold(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol type)
+        private ExpressionSyntax GetConstantOrNull(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol type, ExpressionSyntax csNode)
         {
-            var vbOperation = _semanticModel.GetOperation(vbNode);
+            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);
+                return LiteralConversions.GetLiteralExpression(result, convertedType: type);
+            }
+
+            return null;
+        }
+
+        /// <remarks>Deal with cases like "2*PI" without inlining the const</remarks>
+        private bool IsProbablyConstExpression(IOperation op)
+        {
+            op = op.SkipParens(true);
+
+            if (op is IFieldReferenceOperation fro && fro.Field.IsConst || op is ILocalReferenceOperation lro && lro.Local.IsConst || op is ILiteralOperation) {
+                return true;
             }
 
-            throw new NotImplementedException("Cannot generate constant C# expression");
+            if (op is IBinaryOperation bo && IsProbablyConstExpression(bo.LeftOperand) && IsProbablyConstExpression(bo.RightOperand)) {
+                return true;
+            }
+
+            return false;
         }
 
         private bool TryCompileTimeEvaluate(IOperation vbOperation, out object result)
@@ -427,7 +451,6 @@ public enum TypeConversionKind
             DestructiveCast,
             NonDestructiveCast,
             Conversion,
-            ConstConversion,
             NullableBool,
             StringToCharArray
         }
diff --git a/Tests/CSharp/ExpressionTests/ExpressionTests.cs b/Tests/CSharp/ExpressionTests/ExpressionTests.cs
@@ -1504,6 +1504,7 @@ await TestConversionVisualBasicToCSharp(
     Const k As UInteger = 1
     Const l As UShort = 1
     Const m As ULong = 1
+    Const Nl As String = ChrW(13) + ChrW(10)
 
     Sub Main()
         Const x As SByte = 4
@@ -1512,7 +1513,7 @@ End Sub
 internal static partial class Module1
 {
     private const bool a = true;
-    private const char b = (char)1;
+    private const char b = '\u0001';
     private const float c = 1;
     private const double d = 1;
     private const decimal e = 1;
@@ -1524,6 +1525,7 @@ internal static partial class Module1
     private const uint k = 1;
     private const ushort l = 1;
     private const ulong m = 1;
+    private const string Nl = ""\r\n"";
 
     public static void Main()
     {
diff --git a/Tests/CSharp/StatementTests.cs b/Tests/CSharp/StatementTests.cs
@@ -1073,7 +1073,7 @@ await TestConversionVisualBasicToCSharp(
     Private Sub TestMethod()
 the_beginning:
         Dim value As Integer = 1
-        Const myPIe As Double = System.Math.PI
+        Const myPIe As Double = 2 * System.Math.PI
         Dim text = ""This is my text!""
         GoTo the_beginning
     End Sub
@@ -1086,7 +1086,7 @@ private void TestMethod()
     the_beginning:
         ;
         int value = 1;
-        const double myPIe = Math.PI;
+        const double myPIe = 2 * Math.PI;
         string text = ""This is my text!"";
         goto the_beginning;
     }
