Skip to content

Commit

Permalink
fix(runtime/dotnet): Correct a number of type mappings
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RomainMuller committed Nov 2, 2018
1 parent 1b851e1 commit 96ff7f7
Show file tree
Hide file tree
Showing 27 changed files with 312 additions and 69 deletions.
15 changes: 11 additions & 4 deletions packages/jsii-calc-lib/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down
29 changes: 28 additions & 1 deletion packages/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -184,6 +203,11 @@
}
]
},
"interfaces": [
{
"fqn": "@scope/jsii-calc-lib.IDoublable"
}
],
"kind": "class",
"name": "Number",
"properties": [
Expand All @@ -193,6 +217,9 @@
},
"immutable": true,
"name": "doubleValue",
"overrides": {
"fqn": "@scope/jsii-calc-lib.IDoublable"
},
"type": {
"primitive": "number"
}
Expand Down Expand Up @@ -324,5 +351,5 @@
}
},
"version": "0.7.8",
"fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
"fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
}
6 changes: 3 additions & 3 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -574,7 +574,7 @@ export class AllowedMethodNames {
}

export interface IReturnsNumber {
obtainNumber(): Number;
obtainNumber(): IDoublable;
readonly numberProp: Number;
}

Expand Down Expand Up @@ -938,4 +938,4 @@ export interface IInterfaceWithMethods {
*/
export interface IInterfaceThatShouldNotBeADataType extends IInterfaceWithMethods {
readonly otherValue: string;
}
}
14 changes: 10 additions & 4 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@
"type": {
"collection": {
"elementtype": {
"primitive": "number"
"fqn": "@scope/jsii-calc-lib.Number"
},
"kind": "map"
}
Expand Down Expand Up @@ -486,6 +486,9 @@
},
{
"primitive": "number"
},
{
"fqn": "@scope/jsii-calc-lib.Number"
}
]
}
Expand All @@ -507,6 +510,9 @@
},
{
"fqn": "jsii-calc.Multiply"
},
{
"fqn": "@scope/jsii-calc-lib.Number"
}
]
}
Expand Down Expand Up @@ -1559,7 +1565,7 @@
"abstract": true,
"name": "obtainNumber",
"returns": {
"primitive": "number"
"fqn": "@scope/jsii-calc-lib.IDoublable"
}
}
],
Expand All @@ -1570,7 +1576,7 @@
"immutable": true,
"name": "numberProp",
"type": {
"primitive": "number"
"fqn": "@scope/jsii-calc-lib.Number"
}
}
]
Expand Down Expand Up @@ -3401,5 +3407,5 @@
}
},
"version": "0.7.8",
"fingerprint": "fhzPkiQLwsWAnEdA5+YEotaWom2Av1au0q2FzpexXaQ="
"fingerprint": "jHSXTzCSZbwYMvLKpeZB6SE8hNgYgt9/2JF1ihM41SI="
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ public static object InvokeCallback(this Callback callback, IReferenceMap refere
{
try
{
object frameworkResult = callback.InvokeCallbackCore(referenceMap);
TypeReference returnType;
object frameworkResult = callback.InvokeCallbackCore(referenceMap, out returnType);

converter.TryConvert(
new TypeReference(primitive: PrimitiveType.Any),
returnType ?? new TypeReference(primitive: PrimitiveType.Any),
referenceMap,
frameworkResult,
out object result
Expand All @@ -41,28 +42,29 @@ out object result
}
}

static object InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap)
static object InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap, out TypeReference returnType)
{
if (callback.Invoke != null)
{
return InvokeMethod(callback.Invoke, referenceMap);
return InvokeMethod(callback.Invoke, referenceMap, out returnType);
}

if (callback.Get != null)
{
return InvokeGetter(callback.Get, referenceMap);
return InvokeGetter(callback.Get, referenceMap, out returnType);
}

if (callback.Set != null)
{
InvokeSetter(callback.Set, referenceMap);
returnType = null;
return null;
}

throw new ArgumentException("Callback does not specificy a method, getter, or setter to invoke");
}

static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap, out TypeReference returnType)
{
request = request ?? throw new ArgumentNullException(nameof(request));
DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
Expand All @@ -74,10 +76,13 @@ 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");
}

JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
returnType = attribute?.Returns;

return methodInfo.Invoke(deputy, request.Arguments);
}

static object InvokeGetter(GetRequest request, IReferenceMap referenceMap)
static object InvokeGetter(GetRequest request, IReferenceMap referenceMap, out TypeReference returnType)
{
request = request ?? throw new ArgumentNullException(nameof(request));
DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
Expand All @@ -88,6 +93,9 @@ 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>();
returnType = attribute?.Type;

MethodInfo methodInfo = propertyInfo.GetGetMethod();
if (methodInfo == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -184,6 +203,11 @@
}
]
},
"interfaces": [
{
"fqn": "@scope/jsii-calc-lib.IDoublable"
}
],
"kind": "class",
"name": "Number",
"properties": [
Expand All @@ -193,6 +217,9 @@
},
"immutable": true,
"name": "doubleValue",
"overrides": {
"fqn": "@scope/jsii-calc-lib.IDoublable"
},
"type": {
"primitive": "number"
}
Expand Down Expand Up @@ -324,5 +351,5 @@
}
},
"version": "0.7.8",
"fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
"fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace.LibNamespace
{
/// <summary>The general contract for a concrete number.</summary>
[JsiiTypeProxy(typeof(IIDoublable), "@scope/jsii-calc-lib.IDoublable")]
internal sealed class IDoublableProxy : DeputyBase, IIDoublable
{
private IDoublableProxy(ByRefValue reference): base(reference)
{
}

[JsiiProperty("doubleValue", "{\"primitive\":\"number\"}")]
public double DoubleValue
{
get => GetInstanceProperty<double>();
}
}
}
Loading

0 comments on commit 96ff7f7

Please sign in to comment.