From c977e34bd3cd2716dd8dda9a914daf4424574c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 13 Nov 2019 09:52:17 +0100 Subject: [PATCH 1/3] fix(dotnet): allow down-casting to parent interface type Cast controls were too strict and prevented the framework from successfully down-casting an object reference to a parent interface of it's declared type; causing a cast error. The new code looks for class compatibility using the standard .NET primitives and successfully performs the requested cast. Fixes #982 --- packages/jsii-calc/lib/compliance.ts | 30 +++++ packages/jsii-calc/test/assembly.jsii | 124 +++++++++++++++++- ...mazon.JSII.Runtime.IntegrationTests.csproj | 4 + .../ComplianceTests.cs | 7 + .../Amazon.JSII.Runtime/Deputy/DeputyBase.cs | 59 ++++++--- .../amazon/jsii/testing/ComplianceTest.java | 6 + 6 files changed, 209 insertions(+), 21 deletions(-) diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index 8a21f271bd..b9725d3e53 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -2208,3 +2208,33 @@ export class RootStructValidator { private constructor() { } } + +/** https://github.com/aws/jsii/issues/982 */ +export interface ParentStruct982 { + readonly foo: string; +} +export interface ChildStruct982 extends ParentStruct982 { + readonly bar: number; +} +/** + * 1. call #takeThis() -> An ObjectRef will be provisioned for the value (it'll be re-used!) + * 2. call #takeThisToo() -> The ObjectRef from before will need to be down-cased to the ParentStruct982 type + */ +export class Demonstrate982 { + private static readonly value = { + foo: 'foo', + bar: 1337, + }; + + /** It's dangerous to go alone! */ + public static takeThis(): ChildStruct982 { + return this.value; + } + + /** It's dangerous to go alone! */ + public static takeThisToo(): ParentStruct982 { + return this.value; + } + + public constructor() { } +} diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 5550b1c509..0cd3e48e98 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -1664,6 +1664,40 @@ } ] }, + "jsii-calc.ChildStruct982": { + "assembly": "jsii-calc", + "datatype": true, + "docs": { + "stability": "experimental" + }, + "fqn": "jsii-calc.ChildStruct982", + "interfaces": [ + "jsii-calc.ParentStruct982" + ], + "kind": "interface", + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2216 + }, + "name": "ChildStruct982", + "properties": [ + { + "abstract": true, + "docs": { + "stability": "experimental" + }, + "immutable": true, + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2217 + }, + "name": "bar", + "type": { + "primitive": "number" + } + } + ] + }, "jsii-calc.ClassThatImplementsTheInternalInterface": { "assembly": "jsii-calc", "docs": { @@ -2811,6 +2845,62 @@ } ] }, + "jsii-calc.Demonstrate982": { + "assembly": "jsii-calc", + "docs": { + "remarks": "call #takeThis() -> An ObjectRef will be provisioned for the value (it'll be re-used!)\n2. call #takeThisToo() -> The ObjectRef from before will need to be down-cased to the ParentStruct982 type", + "stability": "experimental", + "summary": "1." + }, + "fqn": "jsii-calc.Demonstrate982", + "initializer": { + "docs": { + "stability": "experimental" + } + }, + "kind": "class", + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2223 + }, + "methods": [ + { + "docs": { + "stability": "experimental", + "summary": "It's dangerous to go alone!" + }, + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2230 + }, + "name": "takeThis", + "returns": { + "type": { + "fqn": "jsii-calc.ChildStruct982" + } + }, + "static": true + }, + { + "docs": { + "stability": "experimental", + "summary": "It's dangerous to go alone!" + }, + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2235 + }, + "name": "takeThisToo", + "returns": { + "type": { + "fqn": "jsii-calc.ParentStruct982" + } + }, + "static": true + } + ], + "name": "Demonstrate982" + }, "jsii-calc.DeprecatedClass": { "assembly": "jsii-calc", "docs": { @@ -7604,6 +7694,38 @@ ], "name": "OverrideReturnsObject" }, + "jsii-calc.ParentStruct982": { + "assembly": "jsii-calc", + "datatype": true, + "docs": { + "stability": "experimental", + "summary": "https://github.com/aws/jsii/issues/982." + }, + "fqn": "jsii-calc.ParentStruct982", + "kind": "interface", + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2213 + }, + "name": "ParentStruct982", + "properties": [ + { + "abstract": true, + "docs": { + "stability": "experimental" + }, + "immutable": true, + "locationInModule": { + "filename": "lib/compliance.ts", + "line": 2214 + }, + "name": "foo", + "type": { + "primitive": "string" + } + } + ] + }, "jsii-calc.PartiallyInitializedThisConsumer": { "abstract": true, "assembly": "jsii-calc", @@ -10913,5 +11035,5 @@ } }, "version": "0.20.3", - "fingerprint": "1F+uskR3++T5mjRcWge9oG3H/jJvXm1C3IhR1AwsBTE=" + "fingerprint": "OKPyEfqSbC1Z0sgAKDwTTG8RRdQtwnKPf+OZQllfVUM=" } diff --git a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj index 14df2b9791..d80703157c 100644 --- a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj +++ b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj @@ -19,4 +19,8 @@ + + + + diff --git a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs index f303b84aa2..c1e1560e7b 100644 --- a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs +++ b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs @@ -1251,5 +1251,12 @@ public double Next() return next; } } + + [Fact(DisplayName = Prefix + nameof(StructsCanBeDowncastedToParentType))] + public void StructsCanBeDowncastedToParentType() + { + Assert.NotNull(Demonstrate982.TakeThis()); + Assert.NotNull(Demonstrate982.TakeThisToo()); + } } } diff --git a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs index 2d9eca424a..30013ddaae 100644 --- a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs +++ b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs @@ -455,12 +455,12 @@ private static JsiiMethodAttribute GetMethodAttributeCore(System.Type type, stri private IDictionary Proxies { get; } = new Dictionary(); - public TypeCode GetTypeCode() + TypeCode IConvertible.GetTypeCode() { return TypeCode.Object; } - public object ToType(System.Type conversionType, IFormatProvider provider) + object IConvertible.ToType(System.Type conversionType, IFormatProvider provider) { if (Proxies.ContainsKey(conversionType)) { @@ -498,14 +498,15 @@ bool ToTypeCore(out object result) return false; } - if (!Reference.Interfaces.Contains(interfaceAttribute.FullyQualifiedName)) + var types = ServiceContainer.ServiceProvider.GetRequiredService(); + + if (!TryFindSupportedInterface(interfaceAttribute.FullyQualifiedName, Reference.Interfaces, types, out var adequateFqn)) { // We can only convert to interfaces declared by this Reference result = null; return false; } - - var types = ServiceContainer.ServiceProvider.GetRequiredService(); + var proxyType = types.GetProxyType(interfaceAttribute.FullyQualifiedName); var constructorInfo = proxyType.GetConstructor( BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, @@ -520,82 +521,100 @@ bool ToTypeCore(out object result) result = constructorInfo.Invoke(new object[]{ Reference.ForProxy() }); return true; + + bool TryFindSupportedInterface(string declaredFqn, string[] availableFqns, ITypeCache types, out string foundFqn) + { + var declaredType = types.GetInterfaceType(declaredFqn); + + foreach (var candidate in availableFqns) + { + var candidateType = types.GetInterfaceType(candidate); + if (declaredType.IsAssignableFrom(candidateType)) + { + foundFqn = candidate; + return true; + } + } + + foundFqn = null; + return false; + } } } #region Impossible Conversions - public bool ToBoolean(IFormatProvider provider) + bool IConvertible.ToBoolean(IFormatProvider provider) { throw new InvalidCastException(); } - public byte ToByte(IFormatProvider provider) + byte IConvertible.ToByte(IFormatProvider provider) { throw new InvalidCastException(); } - public char ToChar(IFormatProvider provider) + char IConvertible.ToChar(IFormatProvider provider) { throw new InvalidCastException(); } - public DateTime ToDateTime(IFormatProvider provider) + DateTime IConvertible.ToDateTime(IFormatProvider provider) { throw new InvalidCastException(); } - public decimal ToDecimal(IFormatProvider provider) + decimal IConvertible.ToDecimal(IFormatProvider provider) { throw new InvalidCastException(); } - public double ToDouble(IFormatProvider provider) + double IConvertible.ToDouble(IFormatProvider provider) { throw new InvalidCastException(); } - public short ToInt16(IFormatProvider provider) + short IConvertible.ToInt16(IFormatProvider provider) { throw new InvalidCastException(); } - public int ToInt32(IFormatProvider provider) + int IConvertible.ToInt32(IFormatProvider provider) { throw new InvalidCastException(); } - public long ToInt64(IFormatProvider provider) + long IConvertible.ToInt64(IFormatProvider provider) { throw new InvalidCastException(); } - public sbyte ToSByte(IFormatProvider provider) + sbyte IConvertible.ToSByte(IFormatProvider provider) { throw new InvalidCastException(); } - public float ToSingle(IFormatProvider provider) + float IConvertible.ToSingle(IFormatProvider provider) { throw new InvalidCastException(); } - public string ToString(IFormatProvider provider) + string IConvertible.ToString(IFormatProvider provider) { throw new InvalidCastException(); } - public ushort ToUInt16(IFormatProvider provider) + ushort IConvertible.ToUInt16(IFormatProvider provider) { throw new InvalidCastException(); } - public uint ToUInt32(IFormatProvider provider) + uint IConvertible.ToUInt32(IFormatProvider provider) { throw new InvalidCastException(); } - public ulong ToUInt64(IFormatProvider provider) + ulong IConvertible.ToUInt64(IFormatProvider provider) { throw new InvalidCastException(); } diff --git a/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java b/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java index a904d729d5..5a497c7982 100644 --- a/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java +++ b/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java @@ -1391,6 +1391,12 @@ public void correctlyDeserializesStructUnions() { assertTrue(StructUnionConsumer.isStructB(b1)); } + @Test + public void testStructsCanBeDowncastedToParentType() { + assertNotNull(Demonstrate982.takeThis()); + assertNotNull(Demonstrate982.takeThisToo()); + } + static class PartiallyInitializedThisConsumerImpl extends PartiallyInitializedThisConsumer { @Override public String consumePartiallyInitializedThis(final ConstructorPassesThisOut obj, From 99af441ff09b808264fa9e8201e5ff72ddd05893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 13 Nov 2019 11:01:27 +0100 Subject: [PATCH 2/3] add python test --- packages/jsii-python-runtime/tests/test_compliance.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/jsii-python-runtime/tests/test_compliance.py b/packages/jsii-python-runtime/tests/test_compliance.py index 93fcd3b166..e845d6630e 100644 --- a/packages/jsii-python-runtime/tests/test_compliance.py +++ b/packages/jsii-python-runtime/tests/test_compliance.py @@ -17,6 +17,7 @@ ConsumerCanRingBell, ConstructorPassesThisOut, DataRenderer, + Demonstrate982, DoNotOverridePrivates, DoubleTrouble, GreetingAugmenter, @@ -1039,6 +1040,10 @@ def test_return_subclass_that_implements_interface_976_raises_attributeerror_whe def test_return_anonymous_implementation_of_interface(): assert SomeTypeJsii976.return_anonymous() is not None +def test_structs_can_be_downcasted_to_parent_type(): + assert Demonstrate982.take_this() is not None + assert Demonstrate985.take_this_too() is not None + @jsii.implements(IBellRinger) class PythonBellRinger: def your_turn(self, bell): From b09800519119106061af61b549598e11c748a7cb Mon Sep 17 00:00:00 2001 From: Romain Marcadier-Muller Date: Wed, 13 Nov 2019 11:35:58 +0100 Subject: [PATCH 3/3] fix typo #cringe --- packages/jsii-python-runtime/tests/test_compliance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii-python-runtime/tests/test_compliance.py b/packages/jsii-python-runtime/tests/test_compliance.py index e845d6630e..179a5f405a 100644 --- a/packages/jsii-python-runtime/tests/test_compliance.py +++ b/packages/jsii-python-runtime/tests/test_compliance.py @@ -1042,7 +1042,7 @@ def test_return_anonymous_implementation_of_interface(): def test_structs_can_be_downcasted_to_parent_type(): assert Demonstrate982.take_this() is not None - assert Demonstrate985.take_this_too() is not None + assert Demonstrate982.take_this_too() is not None @jsii.implements(IBellRinger) class PythonBellRinger: