Skip to content

Commit

Permalink
fix(jsii): Optional any represented as required (#237)
Browse files Browse the repository at this point in the history
* 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

* Mark all `any` types as `optional`.
  • Loading branch information
RomainMuller authored Sep 19, 2018
1 parent 578bf9c commit 91074f3
Show file tree
Hide file tree
Showing 20 changed files with 215 additions and 64 deletions.
3 changes: 2 additions & 1 deletion packages/jsii-calc-base/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
},
"name": "typeName",
"returns": {
"optional": true,
"primitive": "any"
}
}
Expand Down Expand Up @@ -101,5 +102,5 @@
}
},
"version": "0.7.5",
"fingerprint": "Kzm7bNzxYO6frB7BwjqVBKhzTiCRwVG5aqgQrz7MOnE="
"fingerprint": "kazFxdH9DydCwjHvToTbXzqqXE0CytJ5qNmGO1NlGAM="
}
5 changes: 5 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
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
43 changes: 35 additions & 8 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "array"
Expand All @@ -388,18 +389,13 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "map"
}
}
},
{
"name": "anyProperty",
"type": {
"primitive": "any"
}
},
{
"name": "arrayProperty",
"type": {
Expand Down Expand Up @@ -521,6 +517,7 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "array"
Expand All @@ -532,15 +529,17 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "map"
}
}
},
{
"name": "unknownProperty",
"name": "anyProperty",
"type": {
"optional": true,
"primitive": "any"
}
},
Expand All @@ -550,6 +549,13 @@
"fqn": "jsii-calc.StringEnum",
"optional": true
}
},
{
"name": "unknownProperty",
"type": {
"optional": true,
"primitive": "any"
}
}
]
},
Expand Down Expand Up @@ -1152,6 +1158,14 @@
"optional": true
}
},
{
"abstract": true,
"name": "optionalAny",
"type": {
"optional": true,
"primitive": "any"
}
},
{
"abstract": true,
"name": "optionalArray",
Expand Down Expand Up @@ -2403,6 +2417,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 @@ -2854,6 +2880,7 @@
{
"name": "value",
"returns": {
"optional": true,
"primitive": "any"
}
}
Expand Down Expand Up @@ -3203,5 +3230,5 @@
}
},
"version": "0.7.5",
"fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8="
"fingerprint": "Zt3ElcP9k7ABYhwmP1xNZBni1sFj9iw0FZwWOe8n+L8="
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
},
"name": "typeName",
"returns": {
"optional": true,
"primitive": "any"
}
}
Expand Down Expand Up @@ -101,5 +102,5 @@
}
},
"version": "0.7.5",
"fingerprint": "Kzm7bNzxYO6frB7BwjqVBKhzTiCRwVG5aqgQrz7MOnE="
"fingerprint": "kazFxdH9DydCwjHvToTbXzqqXE0CytJ5qNmGO1NlGAM="
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ protected Base(DeputyProps props): base(props)
}

/// <returns>the name of the class (to verify native type names are created for derived classes).</returns>
[JsiiMethod("typeName", "{\"primitive\":\"any\"}", "[]")]
[JsiiMethod("typeName", "{\"primitive\":\"any\",\"optional\":true}", "[]")]
public virtual object TypeName()
{
return InvokeInstanceMethod<object>(new object[]{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ protected Base(final software.amazon.jsii.JsiiObject.InitializationMode mode) {
/**
* @return the name of the class (to verify native type names are created for derived classes).
*/
@javax.annotation.Nullable
public java.lang.Object typeName() {
return this.jsiiCall("typeName", java.lang.Object.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ Base
.. py:method:: typeName() -> any
:return: the name of the class (to verify native type names are created for derived classes).
:rtype: any
:rtype: any or undefined


BaseProps (interface)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "array"
Expand All @@ -388,18 +389,13 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "map"
}
}
},
{
"name": "anyProperty",
"type": {
"primitive": "any"
}
},
{
"name": "arrayProperty",
"type": {
Expand Down Expand Up @@ -521,6 +517,7 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "array"
Expand All @@ -532,15 +529,17 @@
"type": {
"collection": {
"elementtype": {
"optional": true,
"primitive": "any"
},
"kind": "map"
}
}
},
{
"name": "unknownProperty",
"name": "anyProperty",
"type": {
"optional": true,
"primitive": "any"
}
},
Expand All @@ -550,6 +549,13 @@
"fqn": "jsii-calc.StringEnum",
"optional": true
}
},
{
"name": "unknownProperty",
"type": {
"optional": true,
"primitive": "any"
}
}
]
},
Expand Down Expand Up @@ -1152,6 +1158,14 @@
"optional": true
}
},
{
"abstract": true,
"name": "optionalAny",
"type": {
"optional": true,
"primitive": "any"
}
},
{
"abstract": true,
"name": "optionalArray",
Expand Down Expand Up @@ -2403,6 +2417,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 @@ -2854,6 +2880,7 @@
{
"name": "value",
"returns": {
"optional": true,
"primitive": "any"
}
}
Expand Down Expand Up @@ -3203,5 +3230,5 @@
}
},
"version": "0.7.5",
"fingerprint": "PrLv57d+5ukv/bps1DvjB9DpM5DS6TpCEld13gQUTe8="
"fingerprint": "Zt3ElcP9k7ABYhwmP1xNZBni1sFj9iw0FZwWOe8n+L8="
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,20 @@ public virtual double EnumPropertyValue
get => GetInstanceProperty<double>();
}

[JsiiProperty("anyArrayProperty", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"any\"}}}")]
[JsiiProperty("anyArrayProperty", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"any\",\"optional\":true}}}")]
public virtual object[] AnyArrayProperty
{
get => GetInstanceProperty<object[]>();
set => SetInstanceProperty(value);
}

[JsiiProperty("anyMapProperty", "{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\"}}}")]
[JsiiProperty("anyMapProperty", "{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\",\"optional\":true}}}")]
public virtual IDictionary<string, object> AnyMapProperty
{
get => GetInstanceProperty<IDictionary<string, object>>();
set => SetInstanceProperty(value);
}

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

[JsiiProperty("arrayProperty", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}}}")]
public virtual string[] ArrayProperty
{
Expand Down Expand Up @@ -129,22 +122,22 @@ public virtual object UnionProperty
set => SetInstanceProperty(value);
}

[JsiiProperty("unknownArrayProperty", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"any\"}}}")]
[JsiiProperty("unknownArrayProperty", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"any\",\"optional\":true}}}")]
public virtual object[] UnknownArrayProperty
{
get => GetInstanceProperty<object[]>();
set => SetInstanceProperty(value);
}

[JsiiProperty("unknownMapProperty", "{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\"}}}")]
[JsiiProperty("unknownMapProperty", "{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\",\"optional\":true}}}")]
public virtual IDictionary<string, object> UnknownMapProperty
{
get => GetInstanceProperty<IDictionary<string, object>>();
set => SetInstanceProperty(value);
}

[JsiiProperty("unknownProperty", "{\"primitive\":\"any\"}")]
public virtual object UnknownProperty
[JsiiProperty("anyProperty", "{\"primitive\":\"any\",\"optional\":true}")]
public virtual object AnyProperty
{
get => GetInstanceProperty<object>();
set => SetInstanceProperty(value);
Expand All @@ -157,6 +150,13 @@ public virtual StringEnum OptionalEnumValue
set => SetInstanceProperty(value);
}

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

[JsiiMethod("enumMethod", "{\"fqn\":\"jsii-calc.StringEnum\"}", "[{\"name\":\"value\",\"type\":{\"fqn\":\"jsii-calc.StringEnum\"}}]")]
public virtual StringEnum EnumMethod(StringEnum value)
{
Expand Down
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
Loading

0 comments on commit 91074f3

Please sign in to comment.