fix(runtime/dotnet): Correct a number of type mappings (#291)

The `jsii` compiler incorrectly mapped types homonym with a standard
type (`String`, `Number`, ...) to the primitive type on usage sites.
This commit corrects this behavior and adjusts the C# tests accordingly.

Additionally, when the return type of a callback was an interface, the
C# runtime was unable to determine the correct JSII type to use, despite
it has the information on the type of the callback method available. The
resolution behavior has also been fixes, as well as a couple of other
glitches that became apparent as a result of the `Number` type starting
to be correctly represented as the `@scope/jsii-calc-lib.Number` type.

Fixes #290
Fixes aws/aws-cdk#1027

packages/jsii-calc-lib/lib/index.ts

@@ -7,20 +7,27 @@ export abstract class Value extends base.Base {
     /**
      * The value.
      */
-    abstract readonly value: number
+    public abstract readonly value: number;
 
     /**
      * String representation of the value.
      */
-    toString() {
+    public toString() {
         return this.value.toString();
     }
 }
 
+/**
+ * The general contract for a concrete number.
+ */
+export interface IDoublable {
+    readonly doubleValue: number;
+}
+
 /**
  * Represents a concrete number.
  */
-export class Number extends Value {
+export class Number extends Value implements IDoublable {
     /**
      * Creates a Number object.
      * @param value The number.
@@ -41,7 +48,7 @@ export class Number extends Value {
  * Represents an operation on values.
  */
 export abstract class Operation extends Value {
-    abstract toString(): string
+    public abstract toString(): string;
 }
 
 /**

packages/jsii-calc-lib/test/assembly.jsii

@@ -92,6 +92,25 @@
       ],
       "name": "EnumFromScopedModule"
     },
+    "@scope/jsii-calc-lib.IDoublable": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "comment": "The general contract for a concrete number."
+      },
+      "fqn": "@scope/jsii-calc-lib.IDoublable",
+      "kind": "interface",
+      "name": "IDoublable",
+      "properties": [
+        {
+          "abstract": true,
+          "immutable": true,
+          "name": "doubleValue",
+          "type": {
+            "primitive": "number"
+          }
+        }
+      ]
+    },
     "@scope/jsii-calc-lib.IFriendly": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {
@@ -184,6 +203,11 @@
           }
         ]
       },
+      "interfaces": [
+        {
+          "fqn": "@scope/jsii-calc-lib.IDoublable"
+        }
+      ],
       "kind": "class",
       "name": "Number",
       "properties": [
@@ -193,6 +217,9 @@
           },
           "immutable": true,
           "name": "doubleValue",
+          "overrides": {
+            "fqn": "@scope/jsii-calc-lib.IDoublable"
+          },
           "type": {
             "primitive": "number"
           }
@@ -324,5 +351,5 @@
     }
   },
   "version": "0.7.8",
-  "fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
+  "fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
 }

packages/jsii-calc/lib/compliance.ts

@@ -1,5 +1,5 @@
 // tslint:disable
-import { Value, Number, IFriendly, MyFirstStruct, StructWithOnlyOptionals, EnumFromScopedModule } from '@scope/jsii-calc-lib';
+import { Value, Number, IFriendly, IDoublable, MyFirstStruct, StructWithOnlyOptionals, EnumFromScopedModule } from '@scope/jsii-calc-lib';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
@@ -574,7 +574,7 @@ export class AllowedMethodNames {
 }
 
 export interface IReturnsNumber {
-    obtainNumber(): Number;
+    obtainNumber(): IDoublable;
     readonly numberProp: Number;
 }
 
@@ -938,4 +938,4 @@ export interface IInterfaceWithMethods {
  */
 export interface IInterfaceThatShouldNotBeADataType extends IInterfaceWithMethods {
     readonly otherValue: string;
-}
+}

packages/jsii-calc/test/assembly.jsii

@@ -436,7 +436,7 @@
           "type": {
             "collection": {
               "elementtype": {
-                "primitive": "number"
+                "fqn": "@scope/jsii-calc-lib.Number"
               },
               "kind": "map"
             }
@@ -486,6 +486,9 @@
                     },
                     {
                       "primitive": "number"
+                    },
+                    {
+                      "fqn": "@scope/jsii-calc-lib.Number"
                     }
                   ]
                 }
@@ -507,6 +510,9 @@
                 },
                 {
                   "fqn": "jsii-calc.Multiply"
+                },
+                {
+                  "fqn": "@scope/jsii-calc-lib.Number"
                 }
               ]
             }
@@ -1559,7 +1565,7 @@
           "abstract": true,
           "name": "obtainNumber",
           "returns": {
-            "primitive": "number"
+            "fqn": "@scope/jsii-calc-lib.IDoublable"
           }
         }
       ],
@@ -1570,7 +1576,7 @@
           "immutable": true,
           "name": "numberProp",
           "type": {
-            "primitive": "number"
+            "fqn": "@scope/jsii-calc-lib.Number"
           }
         }
       ]
@@ -3401,5 +3407,5 @@
     }
   },
   "version": "0.7.8",
-  "fingerprint": "fhzPkiQLwsWAnEdA5+YEotaWom2Av1au0q2FzpexXaQ="
+  "fingerprint": "jHSXTzCSZbwYMvLKpeZB6SE8hNgYgt9/2JF1ihM41SI="
 }

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

@@ -83,10 +83,10 @@ public void CollectionTypes()
             Assert.Equal("World", types.ArrayProperty[1]);
 
             // map
-            IDictionary<string, double> map = new Dictionary<string, double>();
-            map["Foo"] = 123;
+            IDictionary<string, Number> map = new Dictionary<string, Number>();
+            map["Foo"] = new Number(123);
             types.MapProperty = map;
-            Assert.Equal((double) 123, types.MapProperty["Foo"]);
+            Assert.Equal((double) 123, types.MapProperty["Foo"].Value);
         }
 
         [Fact(DisplayName = Prefix + nameof(DynamicTypes))]
@@ -818,6 +818,43 @@ public void TestClassWithPrivateConstructorAndAutomaticProperties()
             Assert.Equal("Hello", obj.ReadOnlyString);
         }
 
+        [Fact(DisplayName = Prefix + nameof(TestReturnInterfaceFromOverride))]
+        public void TestReturnInterfaceFromOverride()
+        {
+            var n = 1337;
+            var obj = new OverrideReturnsObject();
+            var arg = new NumberReturner(n);
+            Assert.Equal(4 * n, obj.Test(arg));
+        }
+
+        class NumberReturner : DeputyBase, IIReturnsNumber
+        {
+            public NumberReturner(double number)
+            {
+                NumberProp = new Number(number);
+            }
+
+            [JsiiProperty("numberProp", "{\"fqn\":\"@scope/jsii-calc-lib.Number\"}", true)]
+            public Number NumberProp { get; }
+
+            [JsiiMethod("obtainNumber", "{\"fqn\":\"@scope/jsii-calc-lib.IDoublable\"}", "[]",true)]
+            public IIDoublable ObtainNumber()
+            {
+                return new Doublable(this.NumberProp);
+            }
+
+            class Doublable : DeputyBase, IIDoublable
+            {
+                public Doublable(Number number)
+                {
+                    this.DoubleValue = number.DoubleValue;
+                }
+
+                [JsiiProperty("doubleValue","{\"primitive\":\"number\"}",true)]
+                public Double DoubleValue { get; }
+            }
+        }
+
         class MulTen : Multiply
         {
             public MulTen(int value)

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs

@@ -15,12 +15,12 @@ public static object InvokeCallback(this Callback callback, IReferenceMap refere
         {
             try
             {
-                object frameworkResult = callback.InvokeCallbackCore(referenceMap);
+                CallbackResult frameworkResult = callback.InvokeCallbackCore(referenceMap);
 
                 converter.TryConvert(
-                    new TypeReference(primitive: PrimitiveType.Any),
+                    frameworkResult?.Type ?? new TypeReference(primitive: PrimitiveType.Any),
                     referenceMap,
-                    frameworkResult,
+                    frameworkResult?.Value,
                     out object result
                 );
 
@@ -41,7 +41,7 @@ public static object InvokeCallback(this Callback callback, IReferenceMap refere
             }
         }
 
-        static object InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap)
+        static CallbackResult InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap)
         {
             if (callback.Invoke != null)
             {
@@ -62,7 +62,7 @@ static object InvokeCallbackCore(this Callback callback, IReferenceMap reference
             throw new ArgumentException("Callback does not specificy a method, getter, or setter to invoke");
         }
 
-        static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
+        static CallbackResult InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
         {
             request = request ?? throw new ArgumentNullException(nameof(request));
             DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
@@ -74,10 +74,11 @@ static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} getter, but this method does not exist");
             }
 
-            return methodInfo.Invoke(deputy, request.Arguments);
+            JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
+            return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments));
         }
 
-        static object InvokeGetter(GetRequest request, IReferenceMap referenceMap)
+        static CallbackResult InvokeGetter(GetRequest request, IReferenceMap referenceMap)
         {
             request = request ?? throw new ArgumentNullException(nameof(request));
             DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
@@ -88,13 +89,15 @@ static object InvokeGetter(GetRequest request, IReferenceMap referenceMap)
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} getter, but this property does not exist");
             }
 
+            JsiiPropertyAttribute attribute = propertyInfo.GetCustomAttribute<JsiiPropertyAttribute>();
+
             MethodInfo methodInfo = propertyInfo.GetGetMethod();
             if (methodInfo == null)
             {
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} getter, but this property does not have a getter");
             }
 
-            return methodInfo.Invoke(deputy, new object[] { });
+            return new CallbackResult(attribute?.Type, methodInfo.Invoke(deputy, new object[] { }));
         }
 
         static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
@@ -117,4 +120,16 @@ static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
             methodInfo.Invoke(deputy, new object[] { request.Value });
         }
     }
+
+    internal class CallbackResult
+    {
+        public CallbackResult(TypeReference type, object value)
+        {
+            Type = type;
+            Value = value;
+        }
+
+        public TypeReference Type { get; }
+        public object Value { get; }
+    }
 }

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/Converters/FrameworkToJsiiConverter.cs

@@ -251,6 +251,10 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
                     return false;
                 }
 
+                if (convertedElement != null && !(convertedElement is String) && !convertedElement.GetType().IsPrimitive)
+                {
+                    convertedElement = JObject.FromObject(convertedElement);
+                }
                 resultObject.Add(new JProperty(key, convertedElement));
             }
 

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

@@ -109,8 +109,8 @@ public void collectionTypes() {
         assertEquals("World", types.getArrayProperty().get(1));
 
         // map
-        Map<String, java.lang.Number> map = new HashMap<>();
-        map.put("Foo", 123);
+        Map<String, Number> map = new HashMap<>();
+        map.put("Foo", new Number(123));
         types.setMapProperty(map);
     }
 

packages/jsii-pacmak/test/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/.jsii

@@ -92,6 +92,25 @@
       ],
       "name": "EnumFromScopedModule"
     },
+    "@scope/jsii-calc-lib.IDoublable": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "comment": "The general contract for a concrete number."
+      },
+      "fqn": "@scope/jsii-calc-lib.IDoublable",
+      "kind": "interface",
+      "name": "IDoublable",
+      "properties": [
+        {
+          "abstract": true,
+          "immutable": true,
+          "name": "doubleValue",
+          "type": {
+            "primitive": "number"
+          }
+        }
+      ]
+    },
     "@scope/jsii-calc-lib.IFriendly": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {
@@ -184,6 +203,11 @@
           }
         ]
       },
+      "interfaces": [
+        {
+          "fqn": "@scope/jsii-calc-lib.IDoublable"
+        }
+      ],
       "kind": "class",
       "name": "Number",
       "properties": [
@@ -193,6 +217,9 @@
           },
           "immutable": true,
           "name": "doubleValue",
+          "overrides": {
+            "fqn": "@scope/jsii-calc-lib.IDoublable"
+          },
           "type": {
             "primitive": "number"
           }
@@ -324,5 +351,5 @@
     }
   },
   "version": "0.7.8",
-  "fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
+  "fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
 }