fix(jsii): Correctly ignore private properties from ctor (#531)

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.
aws · Jun 12, 2019 · e804cab · e804cab
1 parent 27d16c2
commit e804cab
Show file tree

Hide file tree

Showing 14 changed files with 255 additions and 5 deletions.
diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts
@@ -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!';
+    }
+}
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -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",
@@ -8604,5 +8648,5 @@
     }
   },
   "version": "0.11.2",
-  "fingerprint": "5vdOZenAtu9InhNSB0CQ4IjyHtR1CAttxJ+dpDOSCBE="
+  "fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8="
 }
diff --git a/...es/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii b/...es/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii
@@ -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",
@@ -8604,5 +8648,5 @@
     }
   },
   "version": "0.11.2",
-  "fingerprint": "5vdOZenAtu9InhNSB0CQ4IjyHtR1CAttxJ+dpDOSCBE="
+  "fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8="
 }
diff --git a/...ulatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/WithPrivatePropertyInConstructor.cs b/...ulatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/WithPrivatePropertyInConstructor.cs
@@ -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>();
+        }
+    }
+}
diff --git a/.../expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java b/.../expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/$Module.java
@@ -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);

diff --git a/...src/main/java/software/amazon/jsii/tests/calculator/WithPrivatePropertyInConstructor.java b/...src/main/java/software/amazon/jsii/tests/calculator/WithPrivatePropertyInConstructor.java
@@ -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);
+    }
+}
diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py b/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py
@@ -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.
@@ -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()
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
@@ -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
 ^^^^^^^^^^^

diff --git a/packages/jsii-reflect/test/classes.expected.txt b/packages/jsii-reflect/test/classes.expected.txt
@@ -83,4 +83,5 @@ Value
 VariadicMethod
 Very
 VirtualMethodPlayground
-VoidCallback
+VoidCallback
+WithPrivatePropertyInConstructor
diff --git a/packages/jsii-reflect/test/jsii-tree.test.all.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.all.expected.txt
@@ -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

diff --git a/packages/jsii-reflect/test/jsii-tree.test.inheritance.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.inheritance.expected.txt
@@ -106,6 +106,7 @@ assemblies
  │   ├── class VariadicMethod
  │   ├── class VirtualMethodPlayground
  │   ├── class VoidCallback
+ │   ├── class WithPrivatePropertyInConstructor
  │   ├─┬ class CompositeOperation
  │   │ └── base: Operation
  │   ├── interface CalculatorProps

diff --git a/packages/jsii-reflect/test/jsii-tree.test.members.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.members.expected.txt
@@ -542,6 +542,10 @@ assemblies
  │   │   ├── callMe() method
  │   │   ├── overrideMe() method
  │   │   └── methodWasCalled property
+ │   ├─┬ class WithPrivatePropertyInConstructor
+ │   │ └─┬ members
+ │   │   ├── <initializer>(privateField) initializer
+ │   │   └── success property
  │   ├─┬ class CompositeOperation
  │   │ └─┬ members
  │   │   ├── <initializer>() initializer

diff --git a/packages/jsii-reflect/test/jsii-tree.test.types.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.types.expected.txt
@@ -81,6 +81,7 @@ assemblies
  │   ├── class VariadicMethod
  │   ├── class VirtualMethodPlayground
  │   ├── class VoidCallback
+ │   ├── class WithPrivatePropertyInConstructor
  │   ├── class CompositeOperation
  │   ├── interface CalculatorProps
  │   ├── interface DeprecatedStruct

diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts
@@ -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));
           }
         }