Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dotnet): allow down-casting to parent interface type #983

Merged
merged 4 commits into from
Nov 13, 2019
Merged

fix(dotnet): allow down-casting to parent interface type #983

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c977e34
fix(dotnet): allow down-casting to parent interface type
RomainMuller Nov 13, 2019
cd4578a
Merge remote-tracking branch 'origin/master' into rmuller/fix/982
RomainMuller Nov 13, 2019
99af441
add python test
RomainMuller Nov 13, 2019
b098005
fix typo
RomainMuller Nov 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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>
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this coming from and how come it wasn't required so far?

Copy link
Contributor Author

@RomainMuller RomainMuller Nov 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So. I've added this because it makes working with the integration tests in an IDE much nicer... It isn't required per se, but ensures that the local source is used instead of what's in the symbols package (makes debugging more efficient, reduces cycle time because a single solution build gets all the work done, ...).

I routinely add this when I work in the kernel, and since this is not a published artifact, I figured I'd leave it in this time... Because it'd help everyone in this situation.


</Project>
7 changes: 7 additions & 0 deletions ...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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