Skip to content

Commit 0d59dab

Browse files
RomainMullerRomainMuller
authored
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
1 parent 1b851e1 commit 0d59dab

File tree

25 files changed

+319
-72
lines changed

25 files changed

+319
-72
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,27 @@ export abstract class Value extends base.Base {
77
/**
88
* The value.
99
*/
10-
abstract readonly value: number
10+
public abstract readonly value: number;
1111

1212
/**
1313
* String representation of the value.
1414
*/
15-
toString() {
15+
public toString() {
1616
return this.value.toString();
1717
}
1818
}
1919

20+
/**
21+
* The general contract for a concrete number.
22+
*/
23+
export interface IDoublable {
24+
readonly doubleValue: number;
25+
}
26+
2027
/**
2128
* Represents a concrete number.
2229
*/
23-
export class Number extends Value {
30+
export class Number extends Value implements IDoublable {
2431
/**
2532
* Creates a Number object.
2633
* @param value The number.
@@ -41,7 +48,7 @@ export class Number extends Value {
4148
* Represents an operation on values.
4249
*/
4350
export abstract class Operation extends Value {
44-
abstract toString(): string
51+
public abstract toString(): string;
4552
}
4653

4754
/**

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,25 @@
9292
],
9393
"name": "EnumFromScopedModule"
9494
},
95+
"@scope/jsii-calc-lib.IDoublable": {
96+
"assembly": "@scope/jsii-calc-lib",
97+
"docs": {
98+
"comment": "The general contract for a concrete number."
99+
},
100+
"fqn": "@scope/jsii-calc-lib.IDoublable",
101+
"kind": "interface",
102+
"name": "IDoublable",
103+
"properties": [
104+
{
105+
"abstract": true,
106+
"immutable": true,
107+
"name": "doubleValue",
108+
"type": {
109+
"primitive": "number"
110+
}
111+
}
112+
]
113+
},
95114
"@scope/jsii-calc-lib.IFriendly": {
96115
"assembly": "@scope/jsii-calc-lib",
97116
"docs": {
@@ -184,6 +203,11 @@
184203
}
185204
]
186205
},
206+
"interfaces": [
207+
{
208+
"fqn": "@scope/jsii-calc-lib.IDoublable"
209+
}
210+
],
187211
"kind": "class",
188212
"name": "Number",
189213
"properties": [
@@ -193,6 +217,9 @@
193217
},
194218
"immutable": true,
195219
"name": "doubleValue",
220+
"overrides": {
221+
"fqn": "@scope/jsii-calc-lib.IDoublable"
222+
},
196223
"type": {
197224
"primitive": "number"
198225
}
@@ -324,5 +351,5 @@
324351
}
325352
},
326353
"version": "0.7.8",
327-
"fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
354+
"fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
328355
}

‎packages/jsii-calc/lib/compliance.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// tslint:disable
2-
import { Value, Number, IFriendly, MyFirstStruct, StructWithOnlyOptionals, EnumFromScopedModule } from '@scope/jsii-calc-lib';
2+
import { Value, Number, IFriendly, IDoublable, MyFirstStruct, StructWithOnlyOptionals, EnumFromScopedModule } from '@scope/jsii-calc-lib';
33
import * as fs from 'fs';
44
import * as path from 'path';
55
import * as os from 'os';
@@ -574,7 +574,7 @@ export class AllowedMethodNames {
574574
}
575575

576576
export interface IReturnsNumber {
577-
obtainNumber(): Number;
577+
obtainNumber(): IDoublable;
578578
readonly numberProp: Number;
579579
}
580580

@@ -938,4 +938,4 @@ export interface IInterfaceWithMethods {
938938
*/
939939
export interface IInterfaceThatShouldNotBeADataType extends IInterfaceWithMethods {
940940
readonly otherValue: string;
941-
}
941+
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@
436436
"type": {
437437
"collection": {
438438
"elementtype": {
439-
"primitive": "number"
439+
"fqn": "@scope/jsii-calc-lib.Number"
440440
},
441441
"kind": "map"
442442
}
@@ -486,6 +486,9 @@
486486
},
487487
{
488488
"primitive": "number"
489+
},
490+
{
491+
"fqn": "@scope/jsii-calc-lib.Number"
489492
}
490493
]
491494
}
@@ -507,6 +510,9 @@
507510
},
508511
{
509512
"fqn": "jsii-calc.Multiply"
513+
},
514+
{
515+
"fqn": "@scope/jsii-calc-lib.Number"
510516
}
511517
]
512518
}
@@ -1559,7 +1565,7 @@
15591565
"abstract": true,
15601566
"name": "obtainNumber",
15611567
"returns": {
1562-
"primitive": "number"
1568+
"fqn": "@scope/jsii-calc-lib.IDoublable"
15631569
}
15641570
}
15651571
],
@@ -1570,7 +1576,7 @@
15701576
"immutable": true,
15711577
"name": "numberProp",
15721578
"type": {
1573-
"primitive": "number"
1579+
"fqn": "@scope/jsii-calc-lib.Number"
15741580
}
15751581
}
15761582
]
@@ -3401,5 +3407,5 @@
34013407
}
34023408
},
34033409
"version": "0.7.8",
3404-
"fingerprint": "fhzPkiQLwsWAnEdA5+YEotaWom2Av1au0q2FzpexXaQ="
3410+
"fingerprint": "jHSXTzCSZbwYMvLKpeZB6SE8hNgYgt9/2JF1ihM41SI="
34053411
}

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

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ public void CollectionTypes()
8383
Assert.Equal("World", types.ArrayProperty[1]);
8484

8585
// map
86-
IDictionary<string, double> map = new Dictionary<string, double>();
87-
map["Foo"] = 123;
86+
IDictionary<string, Number> map = new Dictionary<string, Number>();
87+
map["Foo"] = new Number(123);
8888
types.MapProperty = map;
89-
Assert.Equal((double) 123, types.MapProperty["Foo"]);
89+
Assert.Equal((double) 123, types.MapProperty["Foo"].Value);
9090
}
9191

9292
[Fact(DisplayName = Prefix + nameof(DynamicTypes))]
@@ -818,6 +818,43 @@ public void TestClassWithPrivateConstructorAndAutomaticProperties()
818818
Assert.Equal("Hello", obj.ReadOnlyString);
819819
}
820820

821+
[Fact(DisplayName = Prefix + nameof(TestReturnInterfaceFromOverride))]
822+
public void TestReturnInterfaceFromOverride()
823+
{
824+
var n = 1337;
825+
var obj = new OverrideReturnsObject();
826+
var arg = new NumberReturner(n);
827+
Assert.Equal(4 * n, obj.Test(arg));
828+
}
829+
830+
class NumberReturner : DeputyBase, IIReturnsNumber
831+
{
832+
public NumberReturner(double number)
833+
{
834+
NumberProp = new Number(number);
835+
}
836+
837+
[JsiiProperty("numberProp", "{\"fqn\":\"@scope/jsii-calc-lib.Number\"}", true)]
838+
public Number NumberProp { get; }
839+
840+
[JsiiMethod("obtainNumber", "{\"fqn\":\"@scope/jsii-calc-lib.IDoublable\"}", "[]",true)]
841+
public IIDoublable ObtainNumber()
842+
{
843+
return new Doublable(this.NumberProp);
844+
}
845+
846+
class Doublable : DeputyBase, IIDoublable
847+
{
848+
public Doublable(Number number)
849+
{
850+
this.DoubleValue = number.DoubleValue;
851+
}
852+
853+
[JsiiProperty("doubleValue","{\"primitive\":\"number\"}",true)]
854+
public Double DoubleValue { get; }
855+
}
856+
}
857+
821858
class MulTen : Multiply
822859
{
823860
public MulTen(int value)

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ public static object InvokeCallback(this Callback callback, IReferenceMap refere
1515
{
1616
try
1717
{
18-
object frameworkResult = callback.InvokeCallbackCore(referenceMap);
18+
CallbackResult frameworkResult = callback.InvokeCallbackCore(referenceMap);
1919

2020
converter.TryConvert(
21-
new TypeReference(primitive: PrimitiveType.Any),
21+
frameworkResult?.Type ?? new TypeReference(primitive: PrimitiveType.Any),
2222
referenceMap,
23-
frameworkResult,
23+
frameworkResult?.Value,
2424
out object result
2525
);
2626

@@ -41,7 +41,7 @@ out object result
4141
}
4242
}
4343

44-
static object InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap)
44+
static CallbackResult InvokeCallbackCore(this Callback callback, IReferenceMap referenceMap)
4545
{
4646
if (callback.Invoke != null)
4747
{
@@ -62,7 +62,7 @@ static object InvokeCallbackCore(this Callback callback, IReferenceMap reference
6262
throw new ArgumentException("Callback does not specificy a method, getter, or setter to invoke");
6363
}
6464

65-
static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
65+
static CallbackResult InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
6666
{
6767
request = request ?? throw new ArgumentNullException(nameof(request));
6868
DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
@@ -74,10 +74,11 @@ static object InvokeMethod(InvokeRequest request, IReferenceMap referenceMap)
7474
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} getter, but this method does not exist");
7575
}
7676

77-
return methodInfo.Invoke(deputy, request.Arguments);
77+
JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
78+
return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments));
7879
}
7980

80-
static object InvokeGetter(GetRequest request, IReferenceMap referenceMap)
81+
static CallbackResult InvokeGetter(GetRequest request, IReferenceMap referenceMap)
8182
{
8283
request = request ?? throw new ArgumentNullException(nameof(request));
8384
DeputyBase deputy = referenceMap.GetOrCreateNativeReference(request.ObjectReference);
@@ -88,13 +89,15 @@ static object InvokeGetter(GetRequest request, IReferenceMap referenceMap)
8889
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} getter, but this property does not exist");
8990
}
9091

92+
JsiiPropertyAttribute attribute = propertyInfo.GetCustomAttribute<JsiiPropertyAttribute>();
93+
9194
MethodInfo methodInfo = propertyInfo.GetGetMethod();
9295
if (methodInfo == null)
9396
{
9497
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} getter, but this property does not have a getter");
9598
}
9699

97-
return methodInfo.Invoke(deputy, new object[] { });
100+
return new CallbackResult(attribute?.Type, methodInfo.Invoke(deputy, new object[] { }));
98101
}
99102

100103
static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
@@ -117,4 +120,16 @@ static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
117120
methodInfo.Invoke(deputy, new object[] { request.Value });
118121
}
119122
}
123+
124+
internal class CallbackResult
125+
{
126+
public CallbackResult(TypeReference type, object value)
127+
{
128+
Type = type;
129+
Value = value;
130+
}
131+
132+
public TypeReference Type { get; }
133+
public object Value { get; }
134+
}
120135
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,10 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
251251
return false;
252252
}
253253

254+
if (convertedElement != null && !(convertedElement is String) && !convertedElement.GetType().IsPrimitive)
255+
{
256+
convertedElement = JObject.FromObject(convertedElement);
257+
}
254258
resultObject.Add(new JProperty(key, convertedElement));
255259
}
256260

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ public void collectionTypes() {
109109
assertEquals("World", types.getArrayProperty().get(1));
110110

111111
// map
112-
Map<String, java.lang.Number> map = new HashMap<>();
113-
map.put("Foo", 123);
112+
Map<String, Number> map = new HashMap<>();
113+
map.put("Foo", new Number(123));
114114
types.setMapProperty(map);
115115
}
116116

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,25 @@
9292
],
9393
"name": "EnumFromScopedModule"
9494
},
95+
"@scope/jsii-calc-lib.IDoublable": {
96+
"assembly": "@scope/jsii-calc-lib",
97+
"docs": {
98+
"comment": "The general contract for a concrete number."
99+
},
100+
"fqn": "@scope/jsii-calc-lib.IDoublable",
101+
"kind": "interface",
102+
"name": "IDoublable",
103+
"properties": [
104+
{
105+
"abstract": true,
106+
"immutable": true,
107+
"name": "doubleValue",
108+
"type": {
109+
"primitive": "number"
110+
}
111+
}
112+
]
113+
},
95114
"@scope/jsii-calc-lib.IFriendly": {
96115
"assembly": "@scope/jsii-calc-lib",
97116
"docs": {
@@ -184,6 +203,11 @@
184203
}
185204
]
186205
},
206+
"interfaces": [
207+
{
208+
"fqn": "@scope/jsii-calc-lib.IDoublable"
209+
}
210+
],
187211
"kind": "class",
188212
"name": "Number",
189213
"properties": [
@@ -193,6 +217,9 @@
193217
},
194218
"immutable": true,
195219
"name": "doubleValue",
220+
"overrides": {
221+
"fqn": "@scope/jsii-calc-lib.IDoublable"
222+
},
196223
"type": {
197224
"primitive": "number"
198225
}
@@ -324,5 +351,5 @@
324351
}
325352
},
326353
"version": "0.7.8",
327-
"fingerprint": "16sTfW7oHGAWfPOj50gWvXsI1REjbNbpk7VUpG1JVVI="
354+
"fingerprint": "HzcyHys0b9gFmP4dogeIJmGE6GVtrSo/P0S54Vd/X8U="
328355
}

0 commit comments

Comments
 (0)