Skip to content

Commit 81e41bd

Browse files
authored
fix(dotnet): Correctly handle 'void' callback results (#471)
Previously, it attempted to generate an OptionalValue instance with no `Type`, which is illegal.
1 parent 7c6eeac commit 81e41bd

File tree

18 files changed

+357
-16
lines changed

18 files changed

+357
-16
lines changed

packages/jsii-calc/lib/compliance.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1663,3 +1663,22 @@ export class StaticContext {
16631663

16641664
private constructor() { }
16651665
}
1666+
1667+
/**
1668+
* This test is used to validate the runtimes can return correctly from a void callback.
1669+
*
1670+
* - Implement `overrideMe` (method does not have to do anything).
1671+
* - Invoke `callMe`
1672+
* - Verify that `methodWasCalled` is `true`.
1673+
*/
1674+
export abstract class VoidCallback {
1675+
private _methodWasCalled = false;
1676+
public get methodWasCalled(): boolean {
1677+
return this._methodWasCalled;
1678+
}
1679+
public callMe(): void {
1680+
this.overrideMe();
1681+
this._methodWasCalled = true;
1682+
}
1683+
protected abstract overrideMe(): void;
1684+
}

packages/jsii-calc/test/assembly.jsii

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6551,6 +6551,53 @@
65516551
],
65526552
"name": "VirtualMethodPlayground"
65536553
},
6554+
"jsii-calc.VoidCallback": {
6555+
"abstract": true,
6556+
"assembly": "jsii-calc",
6557+
"docs": {
6558+
"remarks": "- Implement `overrideMe` (method does not have to do anything).\n- Invoke `callMe`\n- Verify that `methodWasCalled` is `true`.",
6559+
"summary": "This test is used to validate the runtimes can return correctly from a void callback."
6560+
},
6561+
"fqn": "jsii-calc.VoidCallback",
6562+
"initializer": {},
6563+
"kind": "class",
6564+
"locationInModule": {
6565+
"filename": "lib/compliance.ts",
6566+
"line": 1674
6567+
},
6568+
"methods": [
6569+
{
6570+
"locationInModule": {
6571+
"filename": "lib/compliance.ts",
6572+
"line": 1679
6573+
},
6574+
"name": "callMe"
6575+
},
6576+
{
6577+
"abstract": true,
6578+
"locationInModule": {
6579+
"filename": "lib/compliance.ts",
6580+
"line": 1683
6581+
},
6582+
"name": "overrideMe",
6583+
"protected": true
6584+
}
6585+
],
6586+
"name": "VoidCallback",
6587+
"properties": [
6588+
{
6589+
"immutable": true,
6590+
"locationInModule": {
6591+
"filename": "lib/compliance.ts",
6592+
"line": 1676
6593+
},
6594+
"name": "methodWasCalled",
6595+
"type": {
6596+
"primitive": "boolean"
6597+
}
6598+
}
6599+
]
6600+
},
65546601
"jsii-calc.composition.CompositeOperation": {
65556602
"abstract": true,
65566603
"assembly": "jsii-calc",
@@ -6697,5 +6744,5 @@
66976744
}
66986745
},
66996746
"version": "0.10.1",
6700-
"fingerprint": "xAP41ArR/7KF9tmWHGtBlronNIVBQ/oDKKrhPoZ8otA="
6747+
"fingerprint": "975zFxt3ZCRvNKhp/Yh0HUwj9Gqz6I7YcagcGWY7UIY="
67016748
}

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,24 @@ public void ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut()
905905

906906
Assert.NotNull(obj);
907907
}
908+
909+
[Fact(DisplayName = Prefix + nameof(CorrectlyReturnsFromVoidCallback))]
910+
public void CorrectlyReturnsFromVoidCallback()
911+
{
912+
var voidCallback = new VoidCallbackImpl();
913+
voidCallback.CallMe();
914+
915+
Assert.True(voidCallback.MethodWasCalled);
916+
}
908917

918+
class VoidCallbackImpl : VoidCallback
919+
{
920+
protected override void OverrideMe()
921+
{
922+
// Do nothing!
923+
}
924+
}
925+
909926
class PartiallyInitializedThisConsumerImpl : PartiallyInitializedThisConsumer
910927
{
911928
public override String ConsumePartiallyInitializedThis(ConstructorPassesThisOut obj, DateTime dt, AllTypesEnum ev)
@@ -961,10 +978,6 @@ public AddTen(int value)
961978
}
962979
}
963980

964-
class DerivedFromAllTypes : AllTypes
965-
{
966-
}
967-
968981
class OverrideAsyncMethods : AsyncVirtualMethods
969982
{
970983
public override double OverrideMe(double mult)

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,8 @@ static CallbackResult InvokeMethod(InvokeRequest request, IReferenceMap referenc
7777
}
7878

7979
JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
80-
return new CallbackResult(
81-
attribute?.Returns,
82-
methodInfo.Invoke(deputy, request.Arguments.Select(arg => FromKernel(arg, referenceMap)).ToArray())
83-
);
80+
var returnValue = methodInfo.Invoke(deputy, request.Arguments.Select(arg => FromKernel(arg, referenceMap)).ToArray());
81+
return attribute?.Returns != null ? new CallbackResult(attribute?.Returns, returnValue) : null;
8482
}
8583

8684
static CallbackResult InvokeGetter(GetRequest request, IReferenceMap referenceMap)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Override[] GetOverrides()
6363

6464
IEnumerable<Override> GetMethodOverrides()
6565
{
66-
foreach (MethodInfo method in type.GetMethods())
66+
foreach (MethodInfo method in type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
6767
{
6868
JsiiMethodAttribute inheritedAttribute = method.GetCustomAttribute<JsiiMethodAttribute>(true);
6969
JsiiMethodAttribute uninheritedAttribute = method.GetCustomAttribute<JsiiMethodAttribute>(false);
@@ -77,7 +77,7 @@ IEnumerable<Override> GetMethodOverrides()
7777

7878
IEnumerable<Override> GetPropertyOverrides()
7979
{
80-
foreach (PropertyInfo property in type.GetProperties())
80+
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
8181
{
8282
JsiiPropertyAttribute inheritedAttribute = property.GetCustomAttribute<JsiiPropertyAttribute>(true);
8383
JsiiPropertyAttribute uninheritedAttribute = property.GetCustomAttribute<JsiiPropertyAttribute>(false);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static FieldInfo GetEnumMember(Type enumType, string name)
2626

2727
public static MethodInfo GetNativeMethod(Type classType, string name)
2828
{
29-
MethodInfo methodInfo = classType.GetMethods().FirstOrDefault(method =>
29+
MethodInfo methodInfo = classType.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic).FirstOrDefault(method =>
3030
{
3131
JsiiMethodAttribute attribute = method.GetCustomAttribute<JsiiMethodAttribute>();
3232

@@ -44,7 +44,7 @@ public static MethodInfo GetNativeMethod(Type classType, string name)
4444

4545
public static PropertyInfo GetNativeProperty(Type classType, string name)
4646
{
47-
PropertyInfo propertyInfo = classType.GetProperties().FirstOrDefault(property =>
47+
PropertyInfo propertyInfo = classType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic).FirstOrDefault(property =>
4848
{
4949
JsiiPropertyAttribute attribute = property.GetCustomAttribute<JsiiPropertyAttribute>();
5050

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6551,6 +6551,53 @@
65516551
],
65526552
"name": "VirtualMethodPlayground"
65536553
},
6554+
"jsii-calc.VoidCallback": {
6555+
"abstract": true,
6556+
"assembly": "jsii-calc",
6557+
"docs": {
6558+
"remarks": "- Implement `overrideMe` (method does not have to do anything).\n- Invoke `callMe`\n- Verify that `methodWasCalled` is `true`.",
6559+
"summary": "This test is used to validate the runtimes can return correctly from a void callback."
6560+
},
6561+
"fqn": "jsii-calc.VoidCallback",
6562+
"initializer": {},
6563+
"kind": "class",
6564+
"locationInModule": {
6565+
"filename": "lib/compliance.ts",
6566+
"line": 1674
6567+
},
6568+
"methods": [
6569+
{
6570+
"locationInModule": {
6571+
"filename": "lib/compliance.ts",
6572+
"line": 1679
6573+
},
6574+
"name": "callMe"
6575+
},
6576+
{
6577+
"abstract": true,
6578+
"locationInModule": {
6579+
"filename": "lib/compliance.ts",
6580+
"line": 1683
6581+
},
6582+
"name": "overrideMe",
6583+
"protected": true
6584+
}
6585+
],
6586+
"name": "VoidCallback",
6587+
"properties": [
6588+
{
6589+
"immutable": true,
6590+
"locationInModule": {
6591+
"filename": "lib/compliance.ts",
6592+
"line": 1676
6593+
},
6594+
"name": "methodWasCalled",
6595+
"type": {
6596+
"primitive": "boolean"
6597+
}
6598+
}
6599+
]
6600+
},
65546601
"jsii-calc.composition.CompositeOperation": {
65556602
"abstract": true,
65566603
"assembly": "jsii-calc",
@@ -6697,5 +6744,5 @@
66976744
}
66986745
},
66996746
"version": "0.10.1",
6700-
"fingerprint": "xAP41ArR/7KF9tmWHGtBlronNIVBQ/oDKKrhPoZ8otA="
6747+
"fingerprint": "975zFxt3ZCRvNKhp/Yh0HUwj9Gqz6I7YcagcGWY7UIY="
67016748
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using Amazon.JSII.Runtime.Deputy;
2+
3+
namespace Amazon.JSII.Tests.CalculatorNamespace
4+
{
5+
/// <summary>This test is used to validate the runtimes can return correctly from a void callback.</summary>
6+
/// <remarks>
7+
/// - Implement `overrideMe` (method does not have to do anything).
8+
/// - Invoke `callMe`
9+
/// - Verify that `methodWasCalled` is `true`.
10+
/// </remarks>
11+
[JsiiClass(nativeType: typeof(VoidCallback), fullyQualifiedName: "jsii-calc.VoidCallback")]
12+
public abstract class VoidCallback : DeputyBase
13+
{
14+
protected VoidCallback(): base(new DeputyProps(new object[]{}))
15+
{
16+
}
17+
18+
protected VoidCallback(ByRefValue reference): base(reference)
19+
{
20+
}
21+
22+
protected VoidCallback(DeputyProps props): base(props)
23+
{
24+
}
25+
26+
[JsiiProperty(name: "methodWasCalled", typeJson: "{\"primitive\":\"boolean\"}")]
27+
public virtual bool MethodWasCalled
28+
{
29+
get => GetInstanceProperty<bool>();
30+
}
31+
32+
[JsiiMethod(name: "callMe")]
33+
public virtual void CallMe()
34+
{
35+
InvokeInstanceVoidMethod(new object[]{});
36+
}
37+
38+
[JsiiMethod(name: "overrideMe")]
39+
protected abstract void OverrideMe();
40+
}
41+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using Amazon.JSII.Runtime.Deputy;
2+
3+
namespace Amazon.JSII.Tests.CalculatorNamespace
4+
{
5+
/// <summary>This test is used to validate the runtimes can return correctly from a void callback.</summary>
6+
/// <remarks>
7+
/// - Implement `overrideMe` (method does not have to do anything).
8+
/// - Invoke `callMe`
9+
/// - Verify that `methodWasCalled` is `true`.
10+
/// </remarks>
11+
[JsiiTypeProxy(nativeType: typeof(VoidCallback), fullyQualifiedName: "jsii-calc.VoidCallback")]
12+
internal sealed class VoidCallbackProxy : VoidCallback
13+
{
14+
private VoidCallbackProxy(ByRefValue reference): base(reference)
15+
{
16+
}
17+
18+
[JsiiMethod(name: "overrideMe")]
19+
protected override void OverrideMe()
20+
{
21+
InvokeInstanceVoidMethod(new object[]{});
22+
}
23+
}
24+
}

packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
126126
case "jsii-calc.UsesInterfaceWithProperties": return software.amazon.jsii.tests.calculator.UsesInterfaceWithProperties.class;
127127
case "jsii-calc.VariadicMethod": return software.amazon.jsii.tests.calculator.VariadicMethod.class;
128128
case "jsii-calc.VirtualMethodPlayground": return software.amazon.jsii.tests.calculator.VirtualMethodPlayground.class;
129+
case "jsii-calc.VoidCallback": return software.amazon.jsii.tests.calculator.VoidCallback.class;
129130
case "jsii-calc.composition.CompositeOperation": return software.amazon.jsii.tests.calculator.composition.CompositeOperation.class;
130131
case "jsii-calc.composition.CompositeOperation.CompositionStringStyle": return software.amazon.jsii.tests.calculator.composition.CompositeOperation.CompositionStringStyle.class;
131132
default: throw new ClassNotFoundException("Unknown JSII type: " + fqn);

0 commit comments

Comments
 (0)