Skip to content

Commit

Permalink
fix(dotnet): allow down-casting to parent interface type (#983)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@RomainMuller
RomainMuller authored Nov 13, 2019
1 parent c2de100 commit 8a390e5
Show file tree
Hide file tree
Showing 22 changed files with 1,004 additions and 23 deletions.
30 changes: 30 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2236,3 +2236,33 @@ export class SomeTypeJsii976 {
return new Derived();
}
}

/** 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() { }
}
124 changes: 123 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,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": 2244
},
"name": "ChildStruct982",
"properties": [
{
"abstract": true,
"docs": {
"stability": "experimental"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2245
},
"name": "bar",
"type": {
"primitive": "number"
}
}
]
},
"jsii-calc.ClassThatImplementsTheInternalInterface": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -2825,6 +2859,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": 2251
},
"methods": [
{
"docs": {
"stability": "experimental",
"summary": "It's dangerous to go alone!"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2258
},
"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": 2263
},
"name": "takeThisToo",
"returns": {
"type": {
"fqn": "jsii-calc.ParentStruct982"
}
},
"static": true
}
],
"name": "Demonstrate982"
},
"jsii-calc.DeprecatedClass": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -7649,6 +7739,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": 2241
},
"name": "ParentStruct982",
"properties": [
{
"abstract": true,
"docs": {
"stability": "experimental"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2242
},
"name": "foo",
"type": {
"primitive": "string"
}
}
]
},
"jsii-calc.PartiallyInitializedThisConsumer": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -11006,5 +11128,5 @@
}
},
"version": "0.20.3",
"fingerprint": "umMeNAH41pX11GUAjHkw6RTyAwGCeTqRNJ2vPv5aeM0="
"fingerprint": "cb2xJuwu7eu9+ulwYnbZUyzYyE3IkL5fCM4b8Gqr7Vo="
}
4 changes: 4 additions & 0 deletions ...est/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,8 @@
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\..\jsii-dotnet-runtime\src\Amazon.JSII.Runtime\Amazon.JSII.Runtime.csproj" />
</ItemGroup>

</Project>
7 changes: 7 additions & 0 deletions ...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1258,5 +1258,12 @@ public double Next()
return next;
}
}

[Fact(DisplayName = Prefix + nameof(StructsCanBeDowncastedToParentType))]
public void StructsCanBeDowncastedToParentType()
{
Assert.NotNull(Demonstrate982.TakeThis());
Assert.NotNull(Demonstrate982.TakeThisToo());
}
}
}
59 changes: 39 additions & 20 deletions packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,12 +455,12 @@ private static JsiiMethodAttribute GetMethodAttributeCore(System.Type type, stri

private IDictionary<System.Type, object> Proxies { get; } = new Dictionary<System.Type, object>();

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))
{
Expand Down Expand Up @@ -498,14 +498,15 @@ bool ToTypeCore(out object result)
return false;
}

if (!Reference.Interfaces.Contains(interfaceAttribute.FullyQualifiedName))
var types = ServiceContainer.ServiceProvider.GetRequiredService<ITypeCache>();

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<ITypeCache>();

var proxyType = types.GetProxyType(interfaceAttribute.FullyQualifiedName);
var constructorInfo = proxyType.GetConstructor(
BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic,
Expand All @@ -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();
}
Expand Down
6 changes: 6 additions & 0 deletions ...-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,12 @@ public void returnSubclassThatImplementsInterface976() {
assertEquals(obj.getFoo(), 333);
}

@Test
public void testStructsCanBeDowncastedToParentType() {
assertNotNull(Demonstrate982.takeThis());
assertNotNull(Demonstrate982.takeThisToo());
}

static class PartiallyInitializedThisConsumerImpl extends PartiallyInitializedThisConsumer {
@Override
public String consumePartiallyInitializedThis(final ConstructorPassesThisOut obj,
Expand Down
Loading

0 comments on commit 8a390e5

Please sign in to comment.