Skip to content

Commit

Permalink
fix(jsii): Correctly ignore private properties from ctor (#531)
Browse files Browse the repository at this point in the history
Whend eclared as part of a constructor argument declaration, private
properties would not be ignored as such, which could cause spurious JSII
errors, and made private APIs visible to other languages.
  • Loading branch information
RomainMuller authored Jun 12, 2019
1 parent 27d16c2 commit e804cab
Show file tree
Hide file tree
Showing 14 changed files with 255 additions and 5 deletions.
11 changes: 11 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1681,3 +1681,14 @@ export abstract class VoidCallback {
}
protected abstract overrideMe(): void;
}

/**
* Verifies that private property declarations in constructor arguments are hidden.
*/
export class WithPrivatePropertyInConstructor {
constructor(private readonly privateField: string = 'Success!') { }

public get success() {
return this.privateField === 'Success!';
}
}
46 changes: 45 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -8448,6 +8448,50 @@
}
]
},
"jsii-calc.WithPrivatePropertyInConstructor": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Verifies that private property declarations in constructor arguments are hidden."
},
"fqn": "jsii-calc.WithPrivatePropertyInConstructor",
"initializer": {
"docs": {
"stability": "experimental"
},
"parameters": [
{
"name": "privateField",
"optional": true,
"type": {
"primitive": "string"
}
}
]
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1688
},
"name": "WithPrivatePropertyInConstructor",
"properties": [
{
"docs": {
"stability": "experimental"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1691
},
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.composition.CompositeOperation": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -8604,5 +8648,5 @@
}
},
"version": "0.11.2",
"fingerprint": "5vdOZenAtu9InhNSB0CQ4IjyHtR1CAttxJ+dpDOSCBE="
"fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8="
}
Original file line number Diff line number Diff line change
Expand Up @@ -8448,6 +8448,50 @@
}
]
},
"jsii-calc.WithPrivatePropertyInConstructor": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Verifies that private property declarations in constructor arguments are hidden."
},
"fqn": "jsii-calc.WithPrivatePropertyInConstructor",
"initializer": {
"docs": {
"stability": "experimental"
},
"parameters": [
{
"name": "privateField",
"optional": true,
"type": {
"primitive": "string"
}
}
]
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1688
},
"name": "WithPrivatePropertyInConstructor",
"properties": [
{
"docs": {
"stability": "experimental"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1691
},
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.composition.CompositeOperation": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -8604,5 +8648,5 @@
}
},
"version": "0.11.2",
"fingerprint": "5vdOZenAtu9InhNSB0CQ4IjyHtR1CAttxJ+dpDOSCBE="
"fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Verifies that private property declarations in constructor arguments are hidden.</summary>
/// <remarks>stability: Experimental</remarks>
[JsiiClass(nativeType: typeof(WithPrivatePropertyInConstructor), fullyQualifiedName: "jsii-calc.WithPrivatePropertyInConstructor", parametersJson: "[{\"name\":\"privateField\",\"type\":{\"primitive\":\"string\"},\"optional\":true}]")]
public class WithPrivatePropertyInConstructor : DeputyBase
{
/// <remarks>stability: Experimental</remarks>
public WithPrivatePropertyInConstructor(string privateField): base(new DeputyProps(new object[]{privateField}))
{
}

protected WithPrivatePropertyInConstructor(ByRefValue reference): base(reference)
{
}

protected WithPrivatePropertyInConstructor(DeputyProps props): base(props)
{
}

/// <remarks>stability: Experimental</remarks>
[JsiiProperty(name: "success", typeJson: "{\"primitive\":\"boolean\"}")]
public virtual bool Success
{
get => GetInstanceProperty<bool>();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
case "jsii-calc.VariadicMethod": return software.amazon.jsii.tests.calculator.VariadicMethod.class;
case "jsii-calc.VirtualMethodPlayground": return software.amazon.jsii.tests.calculator.VirtualMethodPlayground.class;
case "jsii-calc.VoidCallback": return software.amazon.jsii.tests.calculator.VoidCallback.class;
case "jsii-calc.WithPrivatePropertyInConstructor": return software.amazon.jsii.tests.calculator.WithPrivatePropertyInConstructor.class;
case "jsii-calc.composition.CompositeOperation": return software.amazon.jsii.tests.calculator.composition.CompositeOperation.class;
case "jsii-calc.composition.CompositeOperation.CompositionStringStyle": return software.amazon.jsii.tests.calculator.composition.CompositeOperation.CompositionStringStyle.class;
default: throw new ClassNotFoundException("Unknown JSII type: " + fqn);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package software.amazon.jsii.tests.calculator;

/**
* Verifies that private property declarations in constructor arguments are hidden.
*
* EXPERIMENTAL
*/
@javax.annotation.Generated(value = "jsii-pacmak")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.$Module.class, fqn = "jsii-calc.WithPrivatePropertyInConstructor")
public class WithPrivatePropertyInConstructor extends software.amazon.jsii.JsiiObject {
protected WithPrivatePropertyInConstructor(final software.amazon.jsii.JsiiObject.InitializationMode mode) {
super(mode);
}
/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public WithPrivatePropertyInConstructor(@javax.annotation.Nullable final java.lang.String privateField) {
super(software.amazon.jsii.JsiiObject.InitializationMode.Jsii);
software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this, new Object[] { privateField });
}
/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public WithPrivatePropertyInConstructor() {
super(software.amazon.jsii.JsiiObject.InitializationMode.Jsii);
software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this);
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public java.lang.Boolean getSuccess() {
return this.jsiiGet("success", java.lang.Boolean.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5526,6 +5526,32 @@ def _override_me(self) -> None:
return jsii.invoke(self, "overrideMe", [])


class WithPrivatePropertyInConstructor(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.WithPrivatePropertyInConstructor"):
"""Verifies that private property declarations in constructor arguments are hidden.
Stability:
experimental
"""
def __init__(self, private_field: typing.Optional[str]=None) -> None:
"""
Arguments:
privateField: -
Stability:
experimental
"""
jsii.create(WithPrivatePropertyInConstructor, self, [private_field])

@property
@jsii.member(jsii_name="success")
def success(self) -> bool:
"""
Stability:
experimental
"""
return jsii.get(self, "success")


class composition:
class CompositeOperation(scope.jsii_calc_lib.Operation, metaclass=jsii.JSIIAbstractClass, jsii_type="jsii-calc.composition.CompositeOperation"):
"""Abstract operation composed from an expression of other operations.
Expand Down Expand Up @@ -5887,6 +5913,6 @@ def parts(self, value: typing.List[scope.jsii_calc_lib.Value]):
return jsii.set(self, "parts", value)


__all__ = ["AbstractClass", "AbstractClassBase", "AbstractClassReturner", "Add", "AllTypes", "AllTypesEnum", "AllowedMethodNames", "AsyncVirtualMethods", "AugmentableClass", "BinaryOperation", "Calculator", "CalculatorProps", "ClassThatImplementsTheInternalInterface", "ClassThatImplementsThePrivateInterface", "ClassWithDocs", "ClassWithMutableObjectLiteralProperty", "ClassWithPrivateConstructorAndAutomaticProperties", "ConstructorPassesThisOut", "Constructors", "ConsumersOfThisCrazyTypeSystem", "DefaultedConstructorArgument", "DeprecatedClass", "DeprecatedEnum", "DeprecatedStruct", "DerivedClassHasNoProperties", "DerivedStruct", "DoNotOverridePrivates", "DoNotRecognizeAnyAsOptional", "DocumentedClass", "DontComplainAboutVariadicAfterOptional", "DoubleTrouble", "EraseUndefinedHashValues", "EraseUndefinedHashValuesOptions", "ExperimentalClass", "ExperimentalEnum", "ExperimentalStruct", "ExportedBaseClass", "ExtendsInternalInterface", "GiveMeStructs", "Greetee", "GreetingAugmenter", "IAnotherPublicInterface", "IDeprecatedInterface", "IExperimentalInterface", "IExtendsPrivateInterface", "IFriendlier", "IFriendlyRandomGenerator", "IInterfaceImplementedByAbstractClass", "IInterfaceThatShouldNotBeADataType", "IInterfaceWithInternal", "IInterfaceWithMethods", "IInterfaceWithOptionalMethodArguments", "IInterfaceWithProperties", "IInterfaceWithPropertiesExtension", "IJSII417Derived", "IJSII417PublicBaseOfBase", "IJsii487External", "IJsii487External2", "IJsii496", "IMutableObjectLiteral", "INonInternalInterface", "IPrivatelyImplemented", "IPublicInterface", "IPublicInterface2", "IRandomNumberGenerator", "IReturnsNumber", "IStableInterface", "ImplementInternalInterface", "ImplementsInterfaceWithInternal", "ImplementsInterfaceWithInternalSubclass", "ImplementsPrivateInterface", "ImplictBaseOfBase", "InbetweenClass", "InterfaceInNamespaceIncludesClasses", "InterfaceInNamespaceOnlyInterface", "JSII417Derived", "JSII417PublicBaseOfBase", "JSObjectLiteralForInterface", "JSObjectLiteralToNative", "JSObjectLiteralToNativeClass", "JavaReservedWords", "Jsii487Derived", "Jsii496Derived", "JsiiAgent", "LoadBalancedFargateServiceProps", "Multiply", "Negate", "NodeStandardLibrary", "NullShouldBeTreatedAsUndefined", "NullShouldBeTreatedAsUndefinedData", "NumberGenerator", "ObjectRefsInCollections", "Old", "OptionalConstructorArgument", "OptionalStruct", "OptionalStructConsumer", "OverrideReturnsObject", "PartiallyInitializedThisConsumer", "Polymorphism", "Power", "PublicClass", "PythonReservedWords", "ReferenceEnumFromScopedPackage", "ReturnsPrivateImplementationOfInterface", "RuntimeTypeChecking", "SingleInstanceTwoTypes", "StableClass", "StableEnum", "StableStruct", "StaticContext", "Statics", "StringEnum", "StripInternal", "Sum", "SyncVirtualMethods", "Thrower", "UnaryOperation", "UnionProperties", "UseBundledDependency", "UseCalcBase", "UsesInterfaceWithProperties", "VariadicMethod", "VirtualMethodPlayground", "VoidCallback", "__jsii_assembly__", "composition"]
__all__ = ["AbstractClass", "AbstractClassBase", "AbstractClassReturner", "Add", "AllTypes", "AllTypesEnum", "AllowedMethodNames", "AsyncVirtualMethods", "AugmentableClass", "BinaryOperation", "Calculator", "CalculatorProps", "ClassThatImplementsTheInternalInterface", "ClassThatImplementsThePrivateInterface", "ClassWithDocs", "ClassWithMutableObjectLiteralProperty", "ClassWithPrivateConstructorAndAutomaticProperties", "ConstructorPassesThisOut", "Constructors", "ConsumersOfThisCrazyTypeSystem", "DefaultedConstructorArgument", "DeprecatedClass", "DeprecatedEnum", "DeprecatedStruct", "DerivedClassHasNoProperties", "DerivedStruct", "DoNotOverridePrivates", "DoNotRecognizeAnyAsOptional", "DocumentedClass", "DontComplainAboutVariadicAfterOptional", "DoubleTrouble", "EraseUndefinedHashValues", "EraseUndefinedHashValuesOptions", "ExperimentalClass", "ExperimentalEnum", "ExperimentalStruct", "ExportedBaseClass", "ExtendsInternalInterface", "GiveMeStructs", "Greetee", "GreetingAugmenter", "IAnotherPublicInterface", "IDeprecatedInterface", "IExperimentalInterface", "IExtendsPrivateInterface", "IFriendlier", "IFriendlyRandomGenerator", "IInterfaceImplementedByAbstractClass", "IInterfaceThatShouldNotBeADataType", "IInterfaceWithInternal", "IInterfaceWithMethods", "IInterfaceWithOptionalMethodArguments", "IInterfaceWithProperties", "IInterfaceWithPropertiesExtension", "IJSII417Derived", "IJSII417PublicBaseOfBase", "IJsii487External", "IJsii487External2", "IJsii496", "IMutableObjectLiteral", "INonInternalInterface", "IPrivatelyImplemented", "IPublicInterface", "IPublicInterface2", "IRandomNumberGenerator", "IReturnsNumber", "IStableInterface", "ImplementInternalInterface", "ImplementsInterfaceWithInternal", "ImplementsInterfaceWithInternalSubclass", "ImplementsPrivateInterface", "ImplictBaseOfBase", "InbetweenClass", "InterfaceInNamespaceIncludesClasses", "InterfaceInNamespaceOnlyInterface", "JSII417Derived", "JSII417PublicBaseOfBase", "JSObjectLiteralForInterface", "JSObjectLiteralToNative", "JSObjectLiteralToNativeClass", "JavaReservedWords", "Jsii487Derived", "Jsii496Derived", "JsiiAgent", "LoadBalancedFargateServiceProps", "Multiply", "Negate", "NodeStandardLibrary", "NullShouldBeTreatedAsUndefined", "NullShouldBeTreatedAsUndefinedData", "NumberGenerator", "ObjectRefsInCollections", "Old", "OptionalConstructorArgument", "OptionalStruct", "OptionalStructConsumer", "OverrideReturnsObject", "PartiallyInitializedThisConsumer", "Polymorphism", "Power", "PublicClass", "PythonReservedWords", "ReferenceEnumFromScopedPackage", "ReturnsPrivateImplementationOfInterface", "RuntimeTypeChecking", "SingleInstanceTwoTypes", "StableClass", "StableEnum", "StableStruct", "StaticContext", "Statics", "StringEnum", "StripInternal", "Sum", "SyncVirtualMethods", "Thrower", "UnaryOperation", "UnionProperties", "UseBundledDependency", "UseCalcBase", "UsesInterfaceWithProperties", "VariadicMethod", "VirtualMethodPlayground", "VoidCallback", "WithPrivatePropertyInConstructor", "__jsii_assembly__", "composition"]

publication.publish()
39 changes: 39 additions & 0 deletions packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6808,6 +6808,45 @@ VoidCallback
:type: boolean *(readonly)*


WithPrivatePropertyInConstructor
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

.. py:class:: WithPrivatePropertyInConstructor([privateField])
**Language-specific names:**

.. tabs::

.. code-tab:: c#

using Amazon.JSII.Tests.CalculatorNamespace;

.. code-tab:: java

import software.amazon.jsii.tests.calculator.WithPrivatePropertyInConstructor;

.. code-tab:: javascript

const { WithPrivatePropertyInConstructor } = require('jsii-calc');

.. code-tab:: typescript

import { WithPrivatePropertyInConstructor } from 'jsii-calc';



Verifies that private property declarations in constructor arguments are hidden.



:param privateField:
:type privateField: string

.. py:attribute:: success
:type: boolean *(readonly)*



composition
^^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion packages/jsii-reflect/test/classes.expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,5 @@ Value
VariadicMethod
Very
VirtualMethodPlayground
VoidCallback
VoidCallback
WithPrivatePropertyInConstructor
9 changes: 9 additions & 0 deletions packages/jsii-reflect/test/jsii-tree.test.all.expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,15 @@ assemblies
│ │ └─┬ methodWasCalled property
│ │ ├── immutable
│ │ └── type: boolean
│ ├─┬ class WithPrivatePropertyInConstructor
│ │ └─┬ members
│ │ ├─┬ <initializer>(privateField) initializer
│ │ │ └─┬ parameters
│ │ │ └─┬ privateField
│ │ │ └── type: Optional<string>
│ │ └─┬ success property
│ │ ├── immutable
│ │ └── type: boolean
│ ├─┬ class CompositeOperation
│ │ ├── base: Operation
│ │ └─┬ members
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ assemblies
│ ├── class VariadicMethod
│ ├── class VirtualMethodPlayground
│ ├── class VoidCallback
│ ├── class WithPrivatePropertyInConstructor
│ ├─┬ class CompositeOperation
│ │ └── base: Operation
│ ├── interface CalculatorProps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,10 @@ assemblies
│ │ ├── callMe() method
│ │ ├── overrideMe() method
│ │ └── methodWasCalled property
│ ├─┬ class WithPrivatePropertyInConstructor
│ │ └─┬ members
│ │ ├── <initializer>(privateField) initializer
│ │ └── success property
│ ├─┬ class CompositeOperation
│ │ └─┬ members
│ │ ├── <initializer>() initializer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ assemblies
│ ├── class VariadicMethod
│ ├── class VirtualMethodPlayground
│ ├── class VoidCallback
│ ├── class WithPrivatePropertyInConstructor
│ ├── class CompositeOperation
│ ├── interface CalculatorProps
│ ├── interface DeprecatedStruct
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ export class Assembler implements Emitter {
// Process constructor-based property declarations even if constructor is private
if (signature) {
for (const param of signature.getParameters()) {
if (ts.isParameterPropertyDeclaration(param.valueDeclaration)) {
if (ts.isParameterPropertyDeclaration(param.valueDeclaration) && !this._isPrivateOrInternal(param)) {
await this._visitProperty(param, jsiiType, ctx.replaceStability(jsiiType.docs && jsiiType.docs.stability));
}
}
Expand Down

0 comments on commit e804cab

Please sign in to comment.