From 74770c7aa391728c11c6890a0d827b6f26e8bb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Tue, 18 Sep 2018 08:44:13 +0200 Subject: [PATCH] fix(jsii): Optional `any` represented as required Optional parameters and properties typed `any` would be represented as required, despite the code unambiguously suggests the opposite. This is due to the fact that `any` implicitly covers `null` and `undefined`. This change fixes this by adding a specific provision for the question mark token in the declarations of those, and adds compliance test coverage for the same. Fixes #230 --- packages/jsii-calc/lib/compliance.ts | 5 +++ packages/jsii-calc/test/assembly.jsii | 22 +++++++++++- .../.jsii | 22 +++++++++++- .../CalculatorNamespace/DerivedStruct.cs | 7 ++++ .../CalculatorNamespace/DerivedStructProxy.cs | 7 ++++ .../CalculatorNamespace/IDerivedStruct.cs | 7 ++++ .../RuntimeTypeChecking.cs | 6 ++++ .../jsii/tests/calculator/DerivedStruct.java | 36 +++++++++++++++++++ .../tests/calculator/RuntimeTypeChecking.java | 8 +++++ .../expected.jsii-calc/sphinx/jsii-calc.rst | 11 ++++++ packages/jsii/lib/assembler.ts | 6 +++- 11 files changed, 134 insertions(+), 3 deletions(-) diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index 112921793d..09990ed8af 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -244,6 +244,10 @@ export class RuntimeTypeChecking { public methodWithDefaultedArguments(arg1: number = 2, arg2: string, arg3: Date = new Date()) { arg1; arg2; arg3; } + + public methodWithOptionalAnyArgument(arg?: any) { + arg; + } } export class OptionalConstructorArgument { @@ -479,6 +483,7 @@ export interface DerivedStruct extends MyFirstStruct { bool: boolean anotherRequired: Date optionalArray?: string[] + optionalAny?: any /** * This is optional. */ diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index a2c2a938a0..9436b03e32 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -1152,6 +1152,14 @@ "optional": true } }, + { + "abstract": true, + "name": "optionalAny", + "type": { + "optional": true, + "primitive": "any" + } + }, { "abstract": true, "name": "optionalArray", @@ -2403,6 +2411,18 @@ } ] }, + { + "name": "methodWithOptionalAnyArgument", + "parameters": [ + { + "name": "arg", + "type": { + "optional": true, + "primitive": "any" + } + } + ] + }, { "docs": { "comment": "Used to verify verification of number of method arguments." @@ -3203,5 +3223,5 @@ } }, "version": "0.7.5", - "fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8=" + "fingerprint": "E87+31Zrd9jEqf4+WTZ2u/A6uHS/HQFxOau1yxxuecM=" } 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 a2c2a938a0..9436b03e32 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 @@ -1152,6 +1152,14 @@ "optional": true } }, + { + "abstract": true, + "name": "optionalAny", + "type": { + "optional": true, + "primitive": "any" + } + }, { "abstract": true, "name": "optionalArray", @@ -2403,6 +2411,18 @@ } ] }, + { + "name": "methodWithOptionalAnyArgument", + "parameters": [ + { + "name": "arg", + "type": { + "optional": true, + "primitive": "any" + } + } + ] + }, { "docs": { "comment": "Used to verify verification of number of method arguments." @@ -3203,5 +3223,5 @@ } }, "version": "0.7.5", - "fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8=" + "fingerprint": "E87+31Zrd9jEqf4+WTZ2u/A6uHS/HQFxOau1yxxuecM=" } diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStruct.cs b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStruct.cs index 5a999f1a32..7795e279d1 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStruct.cs +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStruct.cs @@ -38,6 +38,13 @@ public IDictionary AnotherOptional set; } + [JsiiProperty("optionalAny", "{\"primitive\":\"any\",\"optional\":true}", true)] + public object OptionalAny + { + get; + set; + } + [JsiiProperty("optionalArray", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}", true)] public string[] OptionalArray { diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStructProxy.cs b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStructProxy.cs index 372ea6267f..59adc06210 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStructProxy.cs +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStructProxy.cs @@ -43,6 +43,13 @@ public virtual IDictionary AnotherOptional set => SetInstanceProperty(value); } + [JsiiProperty("optionalAny", "{\"primitive\":\"any\",\"optional\":true}")] + public virtual object OptionalAny + { + get => GetInstanceProperty(); + set => SetInstanceProperty(value); + } + [JsiiProperty("optionalArray", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}")] public virtual string[] OptionalArray { diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs index 58a290f26b..59503cab6f 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs @@ -39,6 +39,13 @@ IDictionary AnotherOptional set; } + [JsiiProperty("optionalAny", "{\"primitive\":\"any\",\"optional\":true}")] + object OptionalAny + { + get; + set; + } + [JsiiProperty("optionalArray", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}")] string[] OptionalArray { diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/RuntimeTypeChecking.cs b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/RuntimeTypeChecking.cs index 4f6a82c414..eb99aa7e3b 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/RuntimeTypeChecking.cs +++ b/packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/RuntimeTypeChecking.cs @@ -24,6 +24,12 @@ public virtual void MethodWithDefaultedArguments(double? arg1, string arg2, Date InvokeInstanceVoidMethod(new object[]{arg1, arg2, arg3}); } + [JsiiMethod("methodWithOptionalAnyArgument", null, "[{\"name\":\"arg\",\"type\":{\"primitive\":\"any\",\"optional\":true}}]")] + public virtual void MethodWithOptionalAnyArgument(object arg) + { + InvokeInstanceVoidMethod(new object[]{arg}); + } + /// Used to verify verification of number of method arguments. [JsiiMethod("methodWithOptionalArguments", null, "[{\"name\":\"arg1\",\"type\":{\"primitive\":\"number\"}},{\"name\":\"arg2\",\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg3\",\"type\":{\"primitive\":\"date\",\"optional\":true}}]")] public virtual void MethodWithOptionalArguments(double arg1, string arg2, DateTime? arg3) diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java index f118396065..580d693207 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java +++ b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java @@ -25,6 +25,8 @@ public interface DerivedStruct extends software.amazon.jsii.JsiiSerializable, so * This is optional. */ void setAnotherOptional(final java.util.Map value); + java.lang.Object getOptionalAny(); + void setOptionalAny(final java.lang.Object value); java.util.List getOptionalArray(); void setOptionalArray(final java.util.List value); @@ -45,6 +47,8 @@ final class Builder { @javax.annotation.Nullable private java.util.Map _anotherOptional; @javax.annotation.Nullable + private java.lang.Object _optionalAny; + @javax.annotation.Nullable private java.util.List _optionalArray; private java.lang.Number _anumber; private java.lang.String _astring; @@ -87,6 +91,15 @@ public Builder withAnotherOptional(@javax.annotation.Nullable final java.util.Ma this._anotherOptional = value; return this; } + /** + * Sets the value of OptionalAny + * @param value the value to be set + * @return {@code this} + */ + public Builder withOptionalAny(@javax.annotation.Nullable final java.lang.Object value) { + this._optionalAny = value; + return this; + } /** * Sets the value of OptionalArray * @param value the value to be set @@ -137,6 +150,8 @@ public DerivedStruct build() { @javax.annotation.Nullable private java.util.Map $anotherOptional = _anotherOptional; @javax.annotation.Nullable + private java.lang.Object $optionalAny = _optionalAny; + @javax.annotation.Nullable private java.util.List $optionalArray = _optionalArray; private java.lang.Number $anumber = java.util.Objects.requireNonNull(_anumber, "anumber is required"); private java.lang.String $astring = java.util.Objects.requireNonNull(_astring, "astring is required"); @@ -183,6 +198,16 @@ public void setAnotherOptional(@javax.annotation.Nullable final java.util.Map getOptionalArray() { return this.$optionalArray; @@ -288,6 +313,17 @@ public void setAnotherOptional(@javax.annotation.Nullable final java.util.Map getOptionalArray() { diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/RuntimeTypeChecking.java b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/RuntimeTypeChecking.java index b48aeca4a9..2f1e8cea29 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/RuntimeTypeChecking.java +++ b/packages/jsii-pacmak/test/expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/RuntimeTypeChecking.java @@ -19,6 +19,14 @@ public void methodWithDefaultedArguments(@javax.annotation.Nullable final java.l this.jsiiCall("methodWithDefaultedArguments", Void.class, java.util.stream.Stream.concat(java.util.stream.Stream.of(arg1), java.util.stream.Stream.of(java.util.Objects.requireNonNull(arg2, "arg2 is required"))).toArray()); } + public void methodWithOptionalAnyArgument(@javax.annotation.Nullable final java.lang.Object arg) { + this.jsiiCall("methodWithOptionalAnyArgument", Void.class, java.util.stream.Stream.of(arg).toArray()); + } + + public void methodWithOptionalAnyArgument() { + this.jsiiCall("methodWithOptionalAnyArgument", Void.class); + } + /** * Used to verify verification of number of method arguments. */ 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 97a8d61620..fd0df547f2 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 @@ -1009,6 +1009,11 @@ DerivedStruct (interface) :type: string => :py:class:`@scope/jsii-calc-lib.Value`\ or undefined *(abstract)* + .. py:attribute:: optionalAny + + :type: any or undefined *(abstract)* + + .. py:attribute:: optionalArray :type: string[] or undefined *(abstract)* @@ -2525,6 +2530,12 @@ RuntimeTypeChecking :type arg3: date or undefined + .. py:method:: methodWithOptionalAnyArgument([arg]) + + :param arg: + :type arg: any or undefined + + .. py:method:: methodWithOptionalArguments(arg1, arg2, [arg3]) Used to verify verification of number of method arguments. diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 1511018f80..21aeb54cd1 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -604,6 +604,10 @@ export class Assembler implements Emitter { property.immutable = (ts.getCombinedModifierFlags(signature) & ts.ModifierFlags.Readonly) !== 0; } + if (signature.questionToken) { + property.type.optional = true; + } + if (property.static && property.immutable && ts.isPropertyDeclaration(signature) && signature.initializer) { property.const = true; } @@ -627,7 +631,7 @@ export class Assembler implements Emitter { if (parameter.variadic) { parameter.type = (parameter.type as spec.CollectionTypeReference).collection.elementtype; } - if (paramDeclaration.initializer != null) { + if (paramDeclaration.initializer || paramDeclaration.questionToken) { parameter.type.optional = true; } this._visitDocumentation(paramSymbol, parameter);