Skip to content

Commit

Permalink
fix(jsii): Optional any represented as required
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@RomainMuller
RomainMuller committed Sep 18, 2018
1 parent 578bf9c commit 74770c7
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 3 deletions.
5 changes: 5 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -479,6 +483,7 @@ export interface DerivedStruct extends MyFirstStruct {
bool: boolean
anotherRequired: Date
optionalArray?: string[]
optionalAny?: any
/**
* This is optional.
*/
Expand Down
22 changes: 21 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,14 @@
"optional": true
}
},
{
"abstract": true,
"name": "optionalAny",
"type": {
"optional": true,
"primitive": "any"
}
},
{
"abstract": true,
"name": "optionalArray",
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -3203,5 +3223,5 @@
}
},
"version": "0.7.5",
"fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8="
"fingerprint": "E87+31Zrd9jEqf4+WTZ2u/A6uHS/HQFxOau1yxxuecM="
}
22 changes: 21 additions & 1 deletion ...es/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,14 @@
"optional": true
}
},
{
"abstract": true,
"name": "optionalAny",
"type": {
"optional": true,
"primitive": "any"
}
},
{
"abstract": true,
"name": "optionalArray",
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -3203,5 +3223,5 @@
}
},
"version": "0.7.5",
"fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8="
"fingerprint": "E87+31Zrd9jEqf4+WTZ2u/A6uHS/HQFxOau1yxxuecM="
}
7 changes: 7 additions & 0 deletions ...zon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStruct.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ public IDictionary<string, Value_> 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
{
Expand Down
7 changes: 7 additions & 0 deletions ...SII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DerivedStructProxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ public virtual IDictionary<string, Value_> AnotherOptional
set => SetInstanceProperty(value);
}

[JsiiProperty("optionalAny", "{\"primitive\":\"any\",\"optional\":true}")]
public virtual object OptionalAny
{
get => GetInstanceProperty<object>();
set => SetInstanceProperty(value);
}

[JsiiProperty("optionalArray", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}")]
public virtual string[] OptionalArray
{
Expand Down
7 changes: 7 additions & 0 deletions ...on.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ IDictionary<string, Value_> AnotherOptional
set;
}

[JsiiProperty("optionalAny", "{\"primitive\":\"any\",\"optional\":true}")]
object OptionalAny
{
get;
set;
}

[JsiiProperty("optionalArray", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}")]
string[] OptionalArray
{
Expand Down
6 changes: 6 additions & 0 deletions ...II.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/RuntimeTypeChecking.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}

/// <summary>Used to verify verification of number of method arguments.</summary>
[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)
Expand Down
36 changes: 36 additions & 0 deletions ...ted.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public interface DerivedStruct extends software.amazon.jsii.JsiiSerializable, so
* This is optional.
*/
void setAnotherOptional(final java.util.Map<java.lang.String, software.amazon.jsii.tests.calculator.lib.Value> value);
java.lang.Object getOptionalAny();
void setOptionalAny(final java.lang.Object value);
java.util.List<java.lang.String> getOptionalArray();
void setOptionalArray(final java.util.List<java.lang.String> value);

Expand All @@ -45,6 +47,8 @@ final class Builder {
@javax.annotation.Nullable
private java.util.Map<java.lang.String, software.amazon.jsii.tests.calculator.lib.Value> _anotherOptional;
@javax.annotation.Nullable
private java.lang.Object _optionalAny;
@javax.annotation.Nullable
private java.util.List<java.lang.String> _optionalArray;
private java.lang.Number _anumber;
private java.lang.String _astring;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -137,6 +150,8 @@ public DerivedStruct build() {
@javax.annotation.Nullable
private java.util.Map<java.lang.String, software.amazon.jsii.tests.calculator.lib.Value> $anotherOptional = _anotherOptional;
@javax.annotation.Nullable
private java.lang.Object $optionalAny = _optionalAny;
@javax.annotation.Nullable
private java.util.List<java.lang.String> $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");
Expand Down Expand Up @@ -183,6 +198,16 @@ public void setAnotherOptional(@javax.annotation.Nullable final java.util.Map<ja
this.$anotherOptional = value;
}

@Override
public java.lang.Object getOptionalAny() {
return this.$optionalAny;
}

@Override
public void setOptionalAny(@javax.annotation.Nullable final java.lang.Object value) {
this.$optionalAny = value;
}

@Override
public java.util.List<java.lang.String> getOptionalArray() {
return this.$optionalArray;
Expand Down Expand Up @@ -288,6 +313,17 @@ public void setAnotherOptional(@javax.annotation.Nullable final java.util.Map<ja
this.jsiiSet("anotherOptional", value);
}

@Override
@javax.annotation.Nullable
public java.lang.Object getOptionalAny() {
return this.jsiiGet("optionalAny", java.lang.Object.class);
}

@Override
public void setOptionalAny(@javax.annotation.Nullable final java.lang.Object value) {
this.jsiiSet("optionalAny", value);
}

@Override
@javax.annotation.Nullable
public java.util.List<java.lang.String> getOptionalArray() {
Expand Down
8 changes: 8 additions & 0 deletions ...ii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/RuntimeTypeChecking.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
11 changes: 11 additions & 0 deletions packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)*
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion packages/jsii/lib/assembler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down

0 comments on commit 74770c7

Please sign in to comment.