Consider the underlying type if the conversion is a nullable value co…

…nversion - fixes #1160
icsharpcode · Jan 5, 2025 · b9f5a55 · b9f5a55
1 parent abea701
commit b9f5a55
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 12 deletions.
diff --git a/CodeConverter/CSharp/TypeConversionAnalyzer.cs b/CodeConverter/CSharp/TypeConversionAnalyzer.cs
@@ -178,7 +178,7 @@ public TypeConversionKind AnalyzeConversion(VBSyntax.ExpressionSyntax vbNode, bo
         var csConvertedType = GetCSType(vbConvertedType);
 
         if (csType != null && csConvertedType != null &&
-            TryAnalyzeCsConversion(vbNode, csType, csConvertedType, vbConversion, vbConvertedType, vbType, isConst, forceSourceType != null, out TypeConversionKind analyzeConversion)) {
+            TryAnalyzeCsConversion(vbCompilation, vbNode, csType, csConvertedType, vbConversion, vbConvertedType, vbType, isConst, forceSourceType != null, out TypeConversionKind analyzeConversion)) {
             return analyzeConversion;
         }
 
@@ -273,20 +273,28 @@ private ITypeSymbol GetCSType(ITypeSymbol vbType, VBSyntax.ExpressionSyntax vbNo
         return csType;
     }
 
-    private bool TryAnalyzeCsConversion(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol csType,
+    private bool TryAnalyzeCsConversion(VBasic.VisualBasicCompilation vbCompilation, VBSyntax.ExpressionSyntax vbNode, ITypeSymbol csType,
         ITypeSymbol csConvertedType, Conversion vbConversion, ITypeSymbol vbConvertedType, ITypeSymbol vbType, bool isConst, bool sourceForced,
         out TypeConversionKind typeConversionKind)
     {
         var csConversion = _csCompilation.ClassifyConversion(csType, csConvertedType);
-        vbType.IsNullable(out var underlyingType);
-        vbConvertedType.IsNullable(out var underlyingConvertedType);
-        var nullableVbType = underlyingType ?? vbType;
-        var nullableVbConvertedType = underlyingConvertedType ?? vbConvertedType;
+
+        vbType.IsNullable(out var underlyingVbType);
+        vbConvertedType.IsNullable(out var underlyingVbConvertedType);
+        underlyingVbType ??= vbType;
+        underlyingVbConvertedType ??= vbConvertedType;
+        var vbUnderlyingConversion = vbCompilation.ClassifyConversion(underlyingVbType, underlyingVbConvertedType);
+
+        csType.IsNullable(out var underlyingCsType);
+        csConvertedType.IsNullable(out var underlyingCsConvertedType);
+        underlyingCsType ??= csType;
+        underlyingCsConvertedType ??= csConvertedType;
+        var csUnderlyingConversion = _csCompilation.ClassifyConversion(underlyingCsType, underlyingCsConvertedType);
 
         bool isConvertToString =
             (vbConversion.IsString || vbConversion.IsReference && vbConversion.IsNarrowing) && vbConvertedType.SpecialType == SpecialType.System_String;
         bool isConvertFractionalToInt =
-            !csConversion.IsImplicit && nullableVbType.IsFractionalNumericType() && nullableVbConvertedType.IsIntegralOrEnumType();
+            !csConversion.IsImplicit && underlyingVbType.IsFractionalNumericType() && underlyingVbConvertedType.IsIntegralOrEnumType();
 
         if (!csConversion.Exists || csConversion.IsUnboxing) {
             if (ConvertStringToCharLiteral(vbNode, vbConvertedType, out _)) {
@@ -300,27 +308,27 @@ private bool TryAnalyzeCsConversion(VBSyntax.ExpressionSyntax vbNode, ITypeSymbo
                 return true;
             }
             if (isConvertToString || vbConversion.IsNarrowing) {
-                typeConversionKind = nullableVbConvertedType.IsEnumType() && !csConversion.Exists
+                typeConversionKind = underlyingVbConvertedType.IsEnumType() && !csConversion.Exists
                     ? TypeConversionKind.EnumConversionThenCast
                     : TypeConversionKind.Conversion;
                 return true;
             }
         } else if (vbConversion.IsNarrowing && vbConversion.IsNullableValueType && isConvertFractionalToInt) {
             typeConversionKind = TypeConversionKind.FractionalNumberRoundThenCast;
             return true;
-        } else if (vbConversion.IsNumeric && (csConversion.IsNumeric || nullableVbConvertedType.IsEnumType()) && isConvertFractionalToInt) {
+        } else if (vbConversion.IsNumeric && (csConversion.IsNumeric || underlyingVbConvertedType.IsEnumType()) && isConvertFractionalToInt) {
             typeConversionKind = TypeConversionKind.FractionalNumberRoundThenCast;
             return true;
         } else if (csConversion is {IsExplicit: true, IsEnumeration: true} or {IsBoxing: true, IsImplicit: false}) {
             typeConversionKind = TypeConversionKind.NonDestructiveCast;
             return true;
-        } else if (vbConversion.IsNumeric && csConversion.IsNumeric) {
+        } else if (vbUnderlyingConversion.IsNumeric && csUnderlyingConversion.IsNumeric) {
             // For widening, implicit, a cast is really only needed to help resolve the overload for the operator/method used.
             // e.g. When VB "&" changes to C# "+", there are lots more overloads available that implicit casts could match.
             // e.g. sbyte * ulong uses the decimal * operator in VB. In C# it's ambiguous - see ExpressionTests.vb "TestMul".
             typeConversionKind =
-                isConst && IsImplicitConstantConversion(vbNode) || csConversion.IsIdentity || !sourceForced && IsExactTypeNumericLiteral(vbNode, vbConvertedType) ? TypeConversionKind.Identity :
-                csConversion.IsImplicit || vbType.IsNumericType() ? TypeConversionKind.NonDestructiveCast
+                isConst && IsImplicitConstantConversion(vbNode) || csUnderlyingConversion.IsIdentity || !sourceForced && IsExactTypeNumericLiteral(vbNode, underlyingVbConvertedType) ? TypeConversionKind.Identity :
+                csUnderlyingConversion.IsImplicit || underlyingVbType.IsNumericType() ? TypeConversionKind.NonDestructiveCast
                 : TypeConversionKind.Conversion;
             return true;
         } else if (isConvertToString && vbType.SpecialType == SpecialType.System_Object) {

diff --git a/Tests/CSharp/ExpressionTests/BinaryExpressionTests.cs b/Tests/CSharp/ExpressionTests/BinaryExpressionTests.cs
@@ -122,6 +122,30 @@ private void TestMethod()
 }");
     }
 
+    [Fact]
+    public async Task NullableDoubleArithmeticAsync()
+    {
+        await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
+    Private Sub TestMethod()
+               Dim TotalRead As Long = 1
+        Dim ContentLength As Long? = 2 '(It is supposed that TotalRead < ContentLength)
+        Dim percentage1 As Integer = Convert.ToInt32((TotalRead / ContentLength) * 100.0)
+        Dim percentage2 As Integer = Convert.ToInt32(TotalRead / ContentLength * 100.0)
+    End Sub
+End Class", @"using System;
+
+internal partial class TestClass
+{
+    private void TestMethod()
+    {
+        long TotalRead = 1L;
+        long? ContentLength = 2; // (It is supposed that TotalRead < ContentLength)
+        int percentage1 = Convert.ToInt32(TotalRead / (double?)ContentLength * 100.0d);
+        int percentage2 = Convert.ToInt32(TotalRead / (double?)ContentLength * 100.0d);
+    }
+}");
+    }
+
     [Fact]
     public async Task ImplicitConversionsAsync()
     {

diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ArithmeticTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ArithmeticTests.vb
@@ -10,6 +10,14 @@ Public Class ArithmeticTests
         Assert.Equal(x, 3.5)
     End Sub
 
+    <Fact>
+    Public Sub TestNullableFloatingPointDivision()
+        Dim TotalRead As Long = 1
+        Dim ContentLength As Long? = 2 '(It is supposed that TotalRead < ContentLength)
+        Dim percentage As Integer = Convert.ToInt32((TotalRead / ContentLength) * 100.0)
+        Assert.Equal(50, percentage)
+    End Sub
+
     <Fact>
     Public Sub TestIntegerDivisionOfIntegers()
         Dim x = 7 \ 2