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.

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(kernel): Improve tagged type of wire values #346

Merged
merged 3 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 24 additions & 1 deletion packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1232,4 +1232,27 @@ export class JsiiAgent {
public static get jsiiAgent(): string | undefined {
return process.env.JSII_AGENT;
}
};
};

// Ensure the JSII kernel tags instances with the "most appropriate" FQN type label, so that runtimes are able to
// correctly choose the implementation proxy that should be used. Failure to do so could cause situations where userland
// needs to up-cast an instance to an incompatible type, which certain runtimes (such as Java) will prevent.
// @See https://github.com/awslabs/jsii/issues/345
export class PublicClass {
public hello(): void {}
}
export interface IPublicInterface {
bye(): void;
}
export class InbetweenClass extends PublicClass {}
class PrivateClass extends InbetweenClass implements IPublicInterface {
public bye(): void {}
}
export class Constructors {
public static makeClass(): PublicClass {
return new PrivateClass();
}
public static makeInterface(): IPublicInterface {
return new PrivateClass();
}
}
65 changes: 64 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,31 @@
}
]
},
"jsii-calc.Constructors": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.Constructors",
"initializer": {
"initializer": true
},
"kind": "class",
"methods": [
{
"name": "makeClass",
"returns": {
"fqn": "jsii-calc.PublicClass"
},
"static": true
},
{
"name": "makeInterface",
"returns": {
"fqn": "jsii-calc.IPublicInterface"
},
"static": true
}
],
"name": "Constructors"
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.DefaultedConstructorArgument",
Expand Down Expand Up @@ -1675,6 +1700,18 @@
}
]
},
"jsii-calc.IPublicInterface": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.IPublicInterface",
"kind": "interface",
"methods": [
{
"abstract": true,
"name": "bye"
}
],
"name": "IPublicInterface"
},
"jsii-calc.IRandomNumberGenerator": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -1743,6 +1780,18 @@
}
]
},
"jsii-calc.InbetweenClass": {
"assembly": "jsii-calc",
"base": {
"fqn": "jsii-calc.PublicClass"
},
"fqn": "jsii-calc.InbetweenClass",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "InbetweenClass"
},
"jsii-calc.InterfaceImplementedByAbstractClass": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -2852,6 +2901,20 @@
}
]
},
"jsii-calc.PublicClass": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.PublicClass",
"initializer": {
"initializer": true
},
"kind": "class",
"methods": [
{
"name": "hello"
}
],
"name": "PublicClass"
},
"jsii-calc.ReferenceEnumFromScopedPackage": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -3762,5 +3825,5 @@
}
},
"version": "0.7.13",
"fingerprint": "mc9Rni6GLgVDz8M6CGHapEiYbWsdEIl7CmBI0Qn+KXU="
"fingerprint": "WNp0Sw1d2+3FaQD0Fn01K4wYQi7Ms3w+NQatOylD1kM="
}
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,16 @@ public void ReceiveInstanceOfPrivateClass()
Assert.True(new ReturnsPrivateImplementationOfInterface().PrivateImplementation.Success);
}

[Fact(DisplayName = Prefix + nameof(ObjRefsAreLabelledUsingWithTheMostCorrectType))]
public void ObjRefsAreLabelledUsingWithTheMostCorrectType()
{
var classRef = Constructors.MakeClass();
var ifaceRef = Constructors.MakeInterface();

Assert.Equal(typeof(InbetweenClass), classRef.GetType());
Assert.NotEqual(typeof(InbetweenClass), ifaceRef.GetType());
}

class NumberReturner : DeputyBase, IIReturnsNumber
{
public NumberReturner(double number)
Expand Down Expand Up @@ -1038,4 +1048,4 @@ public double Next()
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
import software.amazon.jsii.tests.calculator.GreetingAugmenter;
import software.amazon.jsii.tests.calculator.IFriendlier;
import software.amazon.jsii.tests.calculator.IFriendlyRandomGenerator;
import software.amazon.jsii.tests.calculator.IPublicInterface;
import software.amazon.jsii.tests.calculator.InterfaceWithProperties;
import software.amazon.jsii.tests.calculator.IRandomNumberGenerator;
import software.amazon.jsii.tests.calculator.InbetweenClass;
import software.amazon.jsii.tests.calculator.InterfaceImplementedByAbstractClass;
import software.amazon.jsii.tests.calculator.JSObjectLiteralForInterface;
import software.amazon.jsii.tests.calculator.JSObjectLiteralToNative;
Expand All @@ -31,6 +33,7 @@
import software.amazon.jsii.tests.calculator.NumberGenerator;
import software.amazon.jsii.tests.calculator.Polymorphism;
import software.amazon.jsii.tests.calculator.Power;
import software.amazon.jsii.tests.calculator.PublicClass;
import software.amazon.jsii.tests.calculator.ReferenceEnumFromScopedPackage;
import software.amazon.jsii.tests.calculator.ReturnsPrivateImplementationOfInterface;
import software.amazon.jsii.tests.calculator.Statics;
Expand All @@ -48,6 +51,8 @@
import software.amazon.jsii.tests.calculator.lib.Value;
import software.amazon.jsii.tests.calculator.JavaReservedWords;
import software.amazon.jsii.tests.calculator.ClassWithPrivateConstructorAndAutomaticProperties;
import software.amazon.jsii.tests.calculator.Constructors;

import org.junit.Test;

import java.io.IOException;
Expand Down Expand Up @@ -954,7 +959,7 @@ public void nullShouldBeTreatedAsUndefined() {
obj.setChangeMeToUndefined(null);
obj.verifyPropertyIsUndefined();
}

@Test
public void testJsiiAgent() {
assertEquals("Java/" + System.getProperty("java.version"), JsiiAgent.getJsiiAgent());
Expand All @@ -968,6 +973,15 @@ public void receiveInstanceOfPrivateClass() {
assertTrue(new ReturnsPrivateImplementationOfInterface().getPrivateImplementation().getSuccess());
}

@Test
public void objRefsAreLabelledUsingWithTheMostCorrectType() {
final PublicClass classRef = Constructors.makeClass();
final IPublicInterface ifaceRef = Constructors.makeInterface();

assertTrue(classRef instanceof InbetweenClass);
RomainMuller marked this conversation as resolved.
Show resolved Hide resolved
assertTrue(ifaceRef instanceof IPublicInterface);
}

static class MulTen extends Multiply {
public MulTen(final int value) {
super(new Number(value), new Number(10));
Expand Down
27 changes: 26 additions & 1 deletion packages/jsii-kernel/lib/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ export class Kernel {
// have an object id, so we need to allocate one for it.
this._debug('creating objref for', v);
const fqn = this._fqnForObject(v);
if (!targetType || !spec.isNamedTypeReference(targetType) || fqn === targetType.fqn) {
if (!targetType || !spec.isNamedTypeReference(targetType) || this._isAssignable(fqn, targetType)) {
return this._createObjref(v, fqn);
}
}
Expand Down Expand Up @@ -1029,6 +1029,31 @@ export class Kernel {
return v;
}

/**
* Tests whether a given type (by it's FQN) can be assigned to a named type reference.
*
* @param actualTypeFqn the FQN of the type that is being tested.
* @param requiredType the required reference type.
*
* @returns true if ``requiredType`` is a super-type (base class or implemented interface) of the type designated by
* ``actualTypeFqn``.
*/
private _isAssignable(actualTypeFqn: string, requiredType: spec.NamedTypeReference): boolean {
if (requiredType.fqn === actualTypeFqn) {
return true;
}
const actualType = this._typeInfoForFqn(actualTypeFqn);
if (spec.isClassType(actualType) && actualType.base) {
if (this._isAssignable(actualType.base.fqn, requiredType)) {
return true;
}
}
if (spec.isClassOrInterfaceType(actualType) && actualType.interfaces) {
return actualType.interfaces.find(iface => this._isAssignable(iface.fqn, requiredType)) != null;
}
return false;
}

private _toSandboxValues(args: any[]) {
return args.map(v => this._toSandbox(v));
}
Expand Down
12 changes: 11 additions & 1 deletion packages/jsii-kernel/test/test.kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ defineTest('object literals are returned by reference', async (test, sandbox) =>

test.equal(newValue,
sandbox.get({
objref: sandbox.get({ objref, property: 'mutableObject' }).value,
objref: sandbox.get({ objref, property: 'mutableObject' }).value,
property: 'value'
}).value);

Expand Down Expand Up @@ -963,6 +963,16 @@ defineTest('JSII_AGENT is undefined in node.js', async (test, sandbox) => {
test.equal(sandbox.sget({ fqn: 'jsii-calc.JsiiAgent', property: 'jsiiAgent' }).value, undefined);
});

defineTest('ObjRefs are labeled with the "most correct" type', async (test, sandbox) => {
const classRef = sandbox.sinvoke({ fqn: 'jsii-calc.Constructors', method: 'makeClass' }).result as api.ObjRef;
const ifaceRef = sandbox.sinvoke({ fqn: 'jsii-calc.Constructors', method: 'makeInterface' }).result as api.ObjRef;

test.ok(classRef[api.TOKEN_REF].startsWith('jsii-calc.InbetweenClass'),
`${classRef[api.TOKEN_REF]} starts with jsii-calc.InbetweenClass`);
test.ok(ifaceRef[api.TOKEN_REF].startsWith('jsii-calc.IPublicInterface'),
`${ifaceRef[api.TOKEN_REF]} starts with jsii-calc.IPublicInterface`);
});

// =================================================================================================

const testNames: { [name: string]: boolean } = { };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,31 @@
}
]
},
"jsii-calc.Constructors": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.Constructors",
"initializer": {
"initializer": true
},
"kind": "class",
"methods": [
{
"name": "makeClass",
"returns": {
"fqn": "jsii-calc.PublicClass"
},
"static": true
},
{
"name": "makeInterface",
"returns": {
"fqn": "jsii-calc.IPublicInterface"
},
"static": true
}
],
"name": "Constructors"
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.DefaultedConstructorArgument",
Expand Down Expand Up @@ -1675,6 +1700,18 @@
}
]
},
"jsii-calc.IPublicInterface": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.IPublicInterface",
"kind": "interface",
"methods": [
{
"abstract": true,
"name": "bye"
}
],
"name": "IPublicInterface"
},
"jsii-calc.IRandomNumberGenerator": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -1743,6 +1780,18 @@
}
]
},
"jsii-calc.InbetweenClass": {
"assembly": "jsii-calc",
"base": {
"fqn": "jsii-calc.PublicClass"
},
"fqn": "jsii-calc.InbetweenClass",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "InbetweenClass"
},
"jsii-calc.InterfaceImplementedByAbstractClass": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -2852,6 +2901,20 @@
}
]
},
"jsii-calc.PublicClass": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.PublicClass",
"initializer": {
"initializer": true
},
"kind": "class",
"methods": [
{
"name": "hello"
}
],
"name": "PublicClass"
},
"jsii-calc.ReferenceEnumFromScopedPackage": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -3762,5 +3825,5 @@
}
},
"version": "0.7.13",
"fingerprint": "mc9Rni6GLgVDz8M6CGHapEiYbWsdEIl7CmBI0Qn+KXU="
"fingerprint": "WNp0Sw1d2+3FaQD0Fn01K4wYQi7Ms3w+NQatOylD1kM="
}
Loading