From 1fd295ba868a9f77317167a36a263a7208e886de Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 26 Sep 2018 10:52:50 +0300 Subject: [PATCH 1/5] fix(kernel): skip overrides of private methods/properties jsii-kernel will ignore override requests for methods/properties that resolve on the object but are not defined in the public API of the type. java runtime will not request overrides for methods/properties that use "private" accessibility. Fixes #244 --- packages/jsii-calc/lib/compliance.ts | 17 ++++- packages/jsii-calc/test/assembly.jsii | 25 ++++++- .../amazon/jsii/testing/ComplianceTest.java | 68 +++++++++++++++++++ .../java/software/amazon/jsii/JsiiEngine.java | 4 ++ packages/jsii-kernel/lib/kernel.ts | 65 ++++++++++++++---- packages/jsii-kernel/test/test.kernel.ts | 48 +++++++++++++ .../.jsii | 25 ++++++- .../DoNotOverridePrivates.cs | 32 +++++++++ .../amazon/jsii/tests/calculator/$Module.java | 1 + .../calculator/DoNotOverridePrivates.java | 21 ++++++ .../expected.jsii-calc/sphinx/jsii-calc.rst | 38 +++++++++++ 11 files changed, 326 insertions(+), 18 deletions(-) create mode 100644 packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DoNotOverridePrivates.cs create mode 100644 packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DoNotOverridePrivates.java diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index 09990ed8af..83dad310c2 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(); @@ -887,3 +886,19 @@ export class AbstractClassReturner { } } } + +export class DoNotOverridePrivates { + private privateMethod(): string { + return 'privateMethod'; + } + + private privateProperty = 'privateProperty'; + + public privateMethodValue() { + return this.privateMethod(); + } + + public privatePropertyValue() { + return this.privateProperty; + } +} \ No newline at end of file diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index e312a04915..664a6b8a0a 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -1181,6 +1181,29 @@ } ] }, + "jsii-calc.DoNotOverridePrivates": { + "assembly": "jsii-calc", + "fqn": "jsii-calc.DoNotOverridePrivates", + "initializer": { + "initializer": true + }, + "kind": "class", + "methods": [ + { + "name": "privateMethodValue", + "returns": { + "primitive": "string" + } + }, + { + "name": "privatePropertyValue", + "returns": { + "primitive": "string" + } + } + ], + "name": "DoNotOverridePrivates" + }, "jsii-calc.DoubleTrouble": { "assembly": "jsii-calc", "fqn": "jsii-calc.DoubleTrouble", @@ -3230,5 +3253,5 @@ } }, "version": "0.7.6", - "fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY=" + "fingerprint": "eSNc0E/b0z3EKspPQoD/VEU1sfZLTkQJpTS8y7XqbJM=" } 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..0d8e94cbaf 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,73 @@ 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"; + } + }; + + assertEquals("privateProperty", obj.privatePropertyValue()); + } + + @Test + public void doNotOverridePrivates_property_getter_private() { + DoNotOverridePrivates obj = new DoNotOverridePrivates() { + private String getPrivateProperty() { + return "privateProperty-Override"; + } + }; + + assertEquals("privateProperty", 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 867f4d5d00..f7ef9098aa 100644 --- a/packages/jsii-kernel/lib/kernel.ts +++ b/packages/jsii-kernel/lib/kernel.ts @@ -467,12 +467,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); @@ -488,11 +483,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); @@ -500,7 +499,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); } @@ -514,10 +524,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 @@ -563,6 +579,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. @@ -589,6 +612,8 @@ export class Kernel { }); } }); + } else if (!methodInfo && Object.keys(obj).includes(methodName)) { + throw new Error('boom'); } else { // sync method override (method info is not required) Object.defineProperty(obj, methodName, { @@ -795,11 +820,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; @@ -825,10 +849,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 92a963e86f..b6937f9002 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); @@ -873,6 +875,52 @@ defineTest('node.js standard library', async (test, sandbox) => { { result: "6a2da20943931e9834fc12cfe5bb47bbd9ae43489a30726962b576f4e3993e50" }); }); +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 e312a04915..664a6b8a0a 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 @@ -1181,6 +1181,29 @@ } ] }, + "jsii-calc.DoNotOverridePrivates": { + "assembly": "jsii-calc", + "fqn": "jsii-calc.DoNotOverridePrivates", + "initializer": { + "initializer": true + }, + "kind": "class", + "methods": [ + { + "name": "privateMethodValue", + "returns": { + "primitive": "string" + } + }, + { + "name": "privatePropertyValue", + "returns": { + "primitive": "string" + } + } + ], + "name": "DoNotOverridePrivates" + }, "jsii-calc.DoubleTrouble": { "assembly": "jsii-calc", "fqn": "jsii-calc.DoubleTrouble", @@ -3230,5 +3253,5 @@ } }, "version": "0.7.6", - "fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY=" + "fingerprint": "eSNc0E/b0z3EKspPQoD/VEU1sfZLTkQJpTS8y7XqbJM=" } 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..2d65408fc8 --- /dev/null +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DoNotOverridePrivates.cs @@ -0,0 +1,32 @@ +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("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 a0bd08be2b..98cf1786cd 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 @@ -33,6 +33,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..0ca9d8f5ac --- /dev/null +++ b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DoNotOverridePrivates.java @@ -0,0 +1,21 @@ +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 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 5e7176ac48..ba8445bf94 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 @@ -1195,6 +1195,44 @@ 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:: privateMethodValue() -> string + + :rtype: string + + + .. py:method:: privatePropertyValue() -> string + + :rtype: string + + DoubleTrouble ^^^^^^^^^^^^^ From 4645e9695d12fcb453775e320453039edbdeee9c Mon Sep 17 00:00:00 2001 From: Romain Marcadier-Muller Date: Fri, 5 Oct 2018 11:56:35 +0200 Subject: [PATCH 2/5] Fix "consecutive blank lines are forbidden" --- packages/jsii-kernel/test/test.kernel.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/jsii-kernel/test/test.kernel.ts b/packages/jsii-kernel/test/test.kernel.ts index f07cd6b1e6..bd778be7d5 100644 --- a/packages/jsii-kernel/test/test.kernel.ts +++ b/packages/jsii-kernel/test/test.kernel.ts @@ -936,7 +936,6 @@ defineTest('overrides: skip overrides of private properties', async (test, sandb test.deepEqual(result.result, 'privateProperty'); }); - // ================================================================================================= const testNames: { [name: string]: boolean } = { }; From c8dcc98773e968d1f13ec965e453aa4cae0a2b9f Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 26 Sep 2018 10:52:50 +0300 Subject: [PATCH 3/5] fix(kernel): skip overrides of private methods/properties jsii-kernel will ignore override requests for methods/properties that resolve on the object but are not defined in the public API of the type. java runtime will not request overrides for methods/properties that use "private" accessibility. Fixes #244 --- packages/jsii-calc/lib/compliance.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index d9e4c6f729..d2456c438b 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -910,5 +910,3 @@ export class DoNotOverridePrivates { return this.privateProperty; } } - - From 648fdf989acf057d9590563abc433b59c2e0c19e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Fri, 5 Oct 2018 20:05:00 -0700 Subject: [PATCH 4/5] Add setter tests --- package-lock.json | 6 +++--- packages/jsii-calc/lib/compliance.ts | 4 ++++ packages/jsii-calc/test/assembly.jsii | 11 +++++++++++ .../amazon/jsii/testing/ComplianceTest.java | 15 ++++++++++++++- packages/jsii-kernel/lib/kernel.ts | 2 -- .../Amazon.JSII.Tests.CalculatorPackageId/.jsii | 11 +++++++++++ .../CalculatorNamespace/DoNotOverridePrivates.cs | 6 ++++++ .../tests/calculator/DoNotOverridePrivates.java | 4 ++++ .../test/expected.jsii-calc/sphinx/jsii-calc.rst | 6 ++++++ 9 files changed, 59 insertions(+), 6 deletions(-) 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 d2456c438b..c0e3f1451f 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -909,4 +909,8 @@ export class DoNotOverridePrivates { 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 c9274e0b83..92cf70ade9 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -1206,6 +1206,17 @@ }, "kind": "class", "methods": [ + { + "name": "changePrivatePropertyValue", + "parameters": [ + { + "name": "newValue", + "type": { + "primitive": "string" + } + } + ] + }, { "name": "privateMethodValue", "returns": { 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 0d8e94cbaf..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 @@ -888,9 +888,16 @@ public void doNotOverridePrivates_property_getter_public() { 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 @@ -899,11 +906,17 @@ public void doNotOverridePrivates_property_getter_private() { 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) { diff --git a/packages/jsii-kernel/lib/kernel.ts b/packages/jsii-kernel/lib/kernel.ts index e6958e9f60..a7f5241ba9 100644 --- a/packages/jsii-kernel/lib/kernel.ts +++ b/packages/jsii-kernel/lib/kernel.ts @@ -619,8 +619,6 @@ export class Kernel { }); } }); - } else if (!methodInfo && Object.keys(obj).includes(methodName)) { - throw new Error('boom'); } else { // sync method override (method info is not required) Object.defineProperty(obj, methodName, { 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 c9274e0b83..92cf70ade9 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 @@ -1206,6 +1206,17 @@ }, "kind": "class", "methods": [ + { + "name": "changePrivatePropertyValue", + "parameters": [ + { + "name": "newValue", + "type": { + "primitive": "string" + } + } + ] + }, { "name": "privateMethodValue", "returns": { 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 index 2d65408fc8..47256a259b 100644 --- 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 @@ -17,6 +17,12 @@ 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() { 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 index 0ca9d8f5ac..71a6033b87 100644 --- 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 @@ -11,6 +11,10 @@ public DoNotOverridePrivates() { 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); } 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 c8c0ee5f07..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 @@ -1256,6 +1256,12 @@ DoNotOverridePrivates + .. py:method:: changePrivatePropertyValue(newValue) + + :param newValue: + :type newValue: string + + .. py:method:: privateMethodValue() -> string :rtype: string From 1fa96dd8aeb27f1647c32aab77ca748776f791e9 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Fri, 5 Oct 2018 21:21:38 -0700 Subject: [PATCH 5/5] Update tests --- packages/jsii-calc/test/assembly.jsii | 2 +- .../dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 92cf70ade9..3ba8c034b9 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -3297,5 +3297,5 @@ } }, "version": "0.7.6", - "fingerprint": "eSNc0E/b0z3EKspPQoD/VEU1sfZLTkQJpTS8y7XqbJM=" + "fingerprint": "IrPnQp841TiCOiG/Z2z18s0K8pxTwuMglW1UJ2t1zsM=" } 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 92cf70ade9..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 @@ -3297,5 +3297,5 @@ } }, "version": "0.7.6", - "fingerprint": "eSNc0E/b0z3EKspPQoD/VEU1sfZLTkQJpTS8y7XqbJM=" + "fingerprint": "IrPnQp841TiCOiG/Z2z18s0K8pxTwuMglW1UJ2t1zsM=" }