diff --git a/package-lock.json b/package-lock.json index 38459f6cdf..1edc1c7290 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7570,9 +7570,9 @@ "optional": true }, "typescript": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.0.1.tgz", - "integrity": "sha512-zQIMOmC+372pC/CCVLqnQ0zSBiY7HHodU7mpQdjiZddek4GMj31I3dUJ7gAs9o65X7mnRma6OokOkc6f9jjfBg==", + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.1.1.tgz", + "integrity": "sha512-Veu0w4dTc/9wlWNf2jeRInNodKlcdLgemvPsrNpfu5Pq39sgfFjvIIgTsvUHCoLBnMhPoUA+tFxsXjU6VexVRQ==", "dev": true }, "unicode-length": { diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index db640cb841..c0e3f1451f 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -871,7 +871,6 @@ class ConcreteClass extends AbstractClass { } } - export class AbstractClassReturner { public giveMeAbstract(): AbstractClass { return new ConcreteClass(); @@ -895,3 +894,23 @@ export interface MutableObjectLiteral { export class ClassWithMutableObjectLiteralProperty { public mutableObject: MutableObjectLiteral = { value: 'default' }; } + +export class DoNotOverridePrivates { + private privateMethod(): string { + return 'privateMethod'; + } + + private privateProperty = 'privateProperty'; + + public privateMethodValue() { + return this.privateMethod(); + } + + public privatePropertyValue() { + return this.privateProperty; + } + + public changePrivatePropertyValue(newValue: string) { + this.privateProperty = newValue; + } +} diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 52a7447ecb..3ba8c034b9 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -1198,6 +1198,40 @@ } ] }, + "jsii-calc.DoNotOverridePrivates": { + "assembly": "jsii-calc", + "fqn": "jsii-calc.DoNotOverridePrivates", + "initializer": { + "initializer": true + }, + "kind": "class", + "methods": [ + { + "name": "changePrivatePropertyValue", + "parameters": [ + { + "name": "newValue", + "type": { + "primitive": "string" + } + } + ] + }, + { + "name": "privateMethodValue", + "returns": { + "primitive": "string" + } + }, + { + "name": "privatePropertyValue", + "returns": { + "primitive": "string" + } + } + ], + "name": "DoNotOverridePrivates" + }, "jsii-calc.DoubleTrouble": { "assembly": "jsii-calc", "fqn": "jsii-calc.DoubleTrouble", @@ -3263,5 +3297,5 @@ } }, "version": "0.7.6", - "fingerprint": "BFT/z3s6IMAAkCjnq3nn+dZUxSqp7tx/E54uljg/XvQ=" + "fingerprint": "IrPnQp841TiCOiG/Z2z18s0K8pxTwuMglW1UJ2t1zsM=" } 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 d1e1612212..ca11966170 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 @@ -11,6 +11,7 @@ import software.amazon.jsii.tests.calculator.Calculator; import software.amazon.jsii.tests.calculator.CalculatorProps; import software.amazon.jsii.tests.calculator.DerivedStruct; +import software.amazon.jsii.tests.calculator.DoNotOverridePrivates; import software.amazon.jsii.tests.calculator.DoubleTrouble; import software.amazon.jsii.tests.calculator.GiveMeStructs; import software.amazon.jsii.tests.calculator.IFriendlier; @@ -837,6 +838,86 @@ public void returnAbstract() { assertEquals("hello-abstract-property", obj.getReturnAbstractFromProperty().getAbstractProperty()); } + @Test + public void doNotOverridePrivates_method_public() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + public String privateMethod() { + return "privateMethod-Override"; + } + }; + + assertEquals("privateMethod", obj.privateMethodValue()); + } + + @Test + public void doNotOverridePrivates_method_private() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + private String privateMethod() { + return "privateMethod-Override"; + } + }; + + assertEquals("privateMethod", obj.privateMethodValue()); + } + + @Test + public void doNotOverridePrivates_property_by_name_private() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + private String privateProperty() { + return "privateProperty-Override"; + } + }; + + assertEquals("privateProperty", obj.privatePropertyValue()); + } + + @Test + public void doNotOverridePrivates_property_by_name_public() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + public String privateProperty() { + return "privateProperty-Override"; + } + }; + + assertEquals("privateProperty", obj.privatePropertyValue()); + } + + @Test + public void doNotOverridePrivates_property_getter_public() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + public String getPrivateProperty() { + return "privateProperty-Override"; + } + public void setPrivateProperty(String value) { + throw new RuntimeException("Boom"); + } + }; + + assertEquals("privateProperty", obj.privatePropertyValue()); + + // verify the setter override is not invoked. + obj.changePrivatePropertyValue("MyNewValue"); + assertEquals("MyNewValue", obj.privatePropertyValue()); + } + + @Test + public void doNotOverridePrivates_property_getter_private() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + private String getPrivateProperty() { + return "privateProperty-Override"; + } + public void setPrivateProperty(String value) { + throw new RuntimeException("Boom"); + } + }; + + assertEquals("privateProperty", obj.privatePropertyValue()); + + // verify the setter override is not invoked. + obj.changePrivatePropertyValue("MyNewValue"); + assertEquals("MyNewValue", obj.privatePropertyValue()); + } + static class MulTen extends Multiply { public MulTen(final int value) { super(new Number(value), new Number(10)); diff --git a/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java b/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java index 9b07f82530..c500a17522 100644 --- a/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java +++ b/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java @@ -468,6 +468,10 @@ private static Collection discoverOverrides(final Class classTo // add all the methods in the current class for (Method method : klass.getDeclaredMethods()) { + if (Modifier.isPrivate(method.getModifiers())) { + continue; + } + String methodName = method.getName(); // check if this is a property ("getXXX" or "setXXX", oh java!) diff --git a/packages/jsii-kernel/lib/kernel.ts b/packages/jsii-kernel/lib/kernel.ts index 62fc59418e..a7f5241ba9 100644 --- a/packages/jsii-kernel/lib/kernel.ts +++ b/packages/jsii-kernel/lib/kernel.ts @@ -474,12 +474,7 @@ export class Kernel { const objref = this._createObjref(obj, fqn); // overrides: for each one of the override method names, installs a - // method on the newly created object which represents the remote - // override. Overrides are always async. When an override is called, it - // returns a promise which adds a callback to the pending callbacks - // list. This list is then retrieved by the client (using - // pendingCallbacks() and promises are fulfilled using - // completeCallback(), which in turn, fulfills the internal promise. + // method on the newly created object which represents the remote "reverse proxy". if (overrides) { this._debug('overrides', overrides); @@ -495,11 +490,15 @@ export class Kernel { methods.add(override.method); - // check that the method being overridden actually exists on the - // class and is an async method. + // check that the method being overridden actually exists let methodInfo; if (fqn !== EMPTY_OBJECT_FQN) { - methodInfo = this._tryTypeInfoForMethod(fqn, override.method); // throws if method cannot be found + // error if we can find a property with this name + if (this._tryTypeInfoForProperty(fqn, override.method)) { + throw new Error(`Trying to override property '${override.method}' as a method`); + } + + methodInfo = this._tryTypeInfoForMethod(fqn, override.method); } this._applyMethodOverride(obj, objref, override, methodInfo); @@ -507,7 +506,18 @@ export class Kernel { if (override.method) { throw new Error(overrideTypeErrorMessage); } if (properties.has(override.property)) { throw Error(`Duplicate override for property '${override.property}'`); } properties.add(override.property); - this._applyPropertyOverride(obj, objref, override); + + let propInfo: spec.Property | undefined; + if (fqn !== EMPTY_OBJECT_FQN) { + // error if we can find a method with this name + if (this._tryTypeInfoForMethod(fqn, override.property)) { + throw new Error(`Trying to override method '${override.property}' as a property`); + } + + propInfo = this._tryTypeInfoForProperty(fqn, override.property); + } + + this._applyPropertyOverride(obj, objref, override, propInfo); } else { throw new Error(overrideTypeErrorMessage); } @@ -521,10 +531,16 @@ export class Kernel { return `$jsii$super$${name}$`; } - private _applyPropertyOverride(obj: any, objref: api.ObjRef, override: api.Override) { + private _applyPropertyOverride(obj: any, objref: api.ObjRef, override: api.Override, propInfo?: spec.Property) { const self = this; const propertyName = override.property!; + // if this is a private property (i.e. doesn't have `propInfo` the object has a key) + if (!propInfo && propertyName in obj) { + this._debug(`Skipping override of private property ${propertyName}`); + return; + } + this._debug('apply override', propertyName); // save the old property under $jsii$super$$ so that property overrides @@ -570,6 +586,13 @@ export class Kernel { const self = this; const methodName = override.method!; + // If this is a private method (doesn't have methodInfo, key resolves on the object), we + // are going to skip the override. + if (!methodInfo && obj[methodName]) { + this._debug(`Skipping override of private method ${methodName}`); + return; + } + // note that we are applying the override even if the method doesn't exist // on the type spec in order to allow native code to override methods from // interfaces. @@ -802,11 +825,10 @@ export class Kernel { return undefined; } - private _typeInfoForProperty(fqn: string, property: string): spec.Property { + private _tryTypeInfoForProperty(fqn: string, property: string): spec.Property | undefined { if (!fqn) { throw new Error('missing "fqn"'); } - const typeInfo = this._typeInfoForFqn(fqn); let properties; @@ -832,10 +854,21 @@ export class Kernel { // recurse to parent type (if exists) for (const baseFqn of bases) { - return this._typeInfoForProperty(baseFqn, property); + const ret = this._tryTypeInfoForProperty(baseFqn, property); + if (ret) { + return ret; + } } - throw new Error(`Type ${typeInfo.fqn} doesn't have a property '${property}'`); + return undefined; + } + + private _typeInfoForProperty(fqn: string, property: string): spec.Property { + const typeInfo = this._tryTypeInfoForProperty(fqn, property); + if (!typeInfo) { + throw new Error(`Type ${fqn} doesn't have a property '${property}'`); + } + return typeInfo; } private _toSandbox(v: any): any { diff --git a/packages/jsii-kernel/test/test.kernel.ts b/packages/jsii-kernel/test/test.kernel.ts index 1e93c27bca..bd778be7d5 100644 --- a/packages/jsii-kernel/test/test.kernel.ts +++ b/packages/jsii-kernel/test/test.kernel.ts @@ -18,6 +18,8 @@ const calcVersion = require('jsii-calc/package.json').version.replace(/\+.+$/, ' // tslint:disable:no-console // tslint:disable:max-line-length +process.setMaxListeners(9999); // since every kernel instance adds an `on('exit')` handler. + process.on('unhandledRejection', e => { console.error(e.stack); process.exit(1); @@ -890,6 +892,52 @@ defineTest('object literals are returned by reference', async (test, sandbox) => sandbox.del({ objref: property }); }); +defineTest('overrides: method instead of property with the same name', async (test, sandbox) => { + test.throws(() => { + sandbox.create({ fqn: 'jsii-calc.SyncVirtualMethods', overrides: [ + { method: 'theProperty' } + ]}); + }, /Trying to override property/); +}); + +defineTest('overrides: property instead of method with the same name', async (test, sandbox) => { + test.throws(() => { + sandbox.create({ fqn: 'jsii-calc.SyncVirtualMethods', overrides: [ + { property: 'virtualMethod' } + ]}); + }, /Trying to override method/); +}); + +defineTest('overrides: skip overrides of private methods', async (test, sandbox) => { + const objref = sandbox.create({ fqn: 'jsii-calc.DoNotOverridePrivates', overrides: [ + { method: 'privateMethod' } + ]}); + + sandbox.callbackHandler = makeSyncCallbackHandler(_ => { + test.ok(false, 'override callback should not be called'); + return 'privateMethodBoom!'; + }); + + const result = sandbox.invoke({ objref, method: 'privateMethodValue' }); + test.deepEqual(result.result, 'privateMethod'); +}); + +defineTest('overrides: skip overrides of private properties', async (test, sandbox) => { + const objref = sandbox.create({ fqn: 'jsii-calc.DoNotOverridePrivates', overrides: [ + { property: 'privateProperty' } + ]}); + + sandbox.callbackHandler = makeSyncCallbackHandler(_ => { + test.ok(false, 'override callback should not be called'); + return 'privatePropertyBoom!'; + }); + + const result = sandbox.invoke({ objref, method: 'privatePropertyValue' }); + test.deepEqual(result.result, 'privateProperty'); +}); + +// ================================================================================================= + const testNames: { [name: string]: boolean } = { }; async function createCalculatorSandbox(name: string) { diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii index 52a7447ecb..3ba8c034b9 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii @@ -1198,6 +1198,40 @@ } ] }, + "jsii-calc.DoNotOverridePrivates": { + "assembly": "jsii-calc", + "fqn": "jsii-calc.DoNotOverridePrivates", + "initializer": { + "initializer": true + }, + "kind": "class", + "methods": [ + { + "name": "changePrivatePropertyValue", + "parameters": [ + { + "name": "newValue", + "type": { + "primitive": "string" + } + } + ] + }, + { + "name": "privateMethodValue", + "returns": { + "primitive": "string" + } + }, + { + "name": "privatePropertyValue", + "returns": { + "primitive": "string" + } + } + ], + "name": "DoNotOverridePrivates" + }, "jsii-calc.DoubleTrouble": { "assembly": "jsii-calc", "fqn": "jsii-calc.DoubleTrouble", @@ -3263,5 +3297,5 @@ } }, "version": "0.7.6", - "fingerprint": "BFT/z3s6IMAAkCjnq3nn+dZUxSqp7tx/E54uljg/XvQ=" + "fingerprint": "IrPnQp841TiCOiG/Z2z18s0K8pxTwuMglW1UJ2t1zsM=" } diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DoNotOverridePrivates.cs b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DoNotOverridePrivates.cs new file mode 100644 index 0000000000..47256a259b --- /dev/null +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DoNotOverridePrivates.cs @@ -0,0 +1,38 @@ +using Amazon.JSII.Runtime.Deputy; + +namespace Amazon.JSII.Tests.CalculatorNamespace +{ + [JsiiClass(typeof(DoNotOverridePrivates), "jsii-calc.DoNotOverridePrivates", "[]")] + public class DoNotOverridePrivates : DeputyBase + { + public DoNotOverridePrivates(): base(new DeputyProps(new object[]{})) + { + } + + protected DoNotOverridePrivates(ByRefValue reference): base(reference) + { + } + + protected DoNotOverridePrivates(DeputyProps props): base(props) + { + } + + [JsiiMethod("changePrivatePropertyValue", null, "[{\"name\":\"newValue\",\"type\":{\"primitive\":\"string\"}}]")] + public virtual void ChangePrivatePropertyValue(string newValue) + { + InvokeInstanceVoidMethod(new object[]{newValue}); + } + + [JsiiMethod("privateMethodValue", "{\"primitive\":\"string\"}", "[]")] + public virtual string PrivateMethodValue() + { + return InvokeInstanceMethod(new object[]{}); + } + + [JsiiMethod("privatePropertyValue", "{\"primitive\":\"string\"}", "[]")] + public virtual string PrivatePropertyValue() + { + return InvokeInstanceMethod(new object[]{}); + } + } +} \ No newline at end of file diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java index e173852e15..f4fe19a282 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java +++ b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java @@ -34,6 +34,7 @@ protected Class resolveClass(final String fqn) throws ClassNotFoundException case "jsii-calc.DerivedClassHasNoProperties.Base": return software.amazon.jsii.tests.calculator.DerivedClassHasNoProperties.Base.class; case "jsii-calc.DerivedClassHasNoProperties.Derived": return software.amazon.jsii.tests.calculator.DerivedClassHasNoProperties.Derived.class; case "jsii-calc.DerivedStruct": return software.amazon.jsii.tests.calculator.DerivedStruct.class; + case "jsii-calc.DoNotOverridePrivates": return software.amazon.jsii.tests.calculator.DoNotOverridePrivates.class; case "jsii-calc.DoubleTrouble": return software.amazon.jsii.tests.calculator.DoubleTrouble.class; case "jsii-calc.GiveMeStructs": return software.amazon.jsii.tests.calculator.GiveMeStructs.class; case "jsii-calc.IFriendlier": return software.amazon.jsii.tests.calculator.IFriendlier.class; diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DoNotOverridePrivates.java b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DoNotOverridePrivates.java new file mode 100644 index 0000000000..71a6033b87 --- /dev/null +++ b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DoNotOverridePrivates.java @@ -0,0 +1,25 @@ +package software.amazon.jsii.tests.calculator; + +@javax.annotation.Generated(value = "jsii-pacmak") +@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.$Module.class, fqn = "jsii-calc.DoNotOverridePrivates") +public class DoNotOverridePrivates extends software.amazon.jsii.JsiiObject { + protected DoNotOverridePrivates(final software.amazon.jsii.JsiiObject.InitializationMode mode) { + super(mode); + } + public DoNotOverridePrivates() { + super(software.amazon.jsii.JsiiObject.InitializationMode.Jsii); + software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this); + } + + public void changePrivatePropertyValue(final java.lang.String newValue) { + this.jsiiCall("changePrivatePropertyValue", Void.class, java.util.stream.Stream.of(java.util.Objects.requireNonNull(newValue, "newValue is required")).toArray()); + } + + public java.lang.String privateMethodValue() { + return this.jsiiCall("privateMethodValue", java.lang.String.class); + } + + public java.lang.String privatePropertyValue() { + return this.jsiiCall("privatePropertyValue", java.lang.String.class); + } +} diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst b/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst index 739564b2d4..b45e6582bd 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst +++ b/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst @@ -1228,6 +1228,50 @@ DerivedStruct (interface) :type: string[] or ``undefined`` *(abstract)* +DoNotOverridePrivates +^^^^^^^^^^^^^^^^^^^^^ + +.. py:class:: DoNotOverridePrivates() + + **Language-specific names:** + + .. tabs:: + + .. code-tab:: c# + + using Amazon.JSII.Tests.CalculatorNamespace; + + .. code-tab:: java + + import software.amazon.jsii.tests.calculator.DoNotOverridePrivates; + + .. code-tab:: javascript + + const { DoNotOverridePrivates } = require('jsii-calc'); + + .. code-tab:: typescript + + import { DoNotOverridePrivates } from 'jsii-calc'; + + + + + .. py:method:: changePrivatePropertyValue(newValue) + + :param newValue: + :type newValue: string + + + .. py:method:: privateMethodValue() -> string + + :rtype: string + + + .. py:method:: privatePropertyValue() -> string + + :rtype: string + + DoubleTrouble ^^^^^^^^^^^^^