Skip to content

Commit

Permalink
fix(jsii): Defaulted parameters were not rendered as optional
Browse files Browse the repository at this point in the history
Parameters with a default value were not represented as optional in the assembly
documents due to an oversight when re-writing `jsii`. There was also no coverage
in the test corpus for this use-case, so I've added some.

Fixes #233
  • Loading branch information
RomainMuller committed Sep 17, 2018
1 parent b664805 commit 8208b67
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 4 deletions.
6 changes: 5 additions & 1 deletion packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ export class RuntimeTypeChecking {
public methodWithOptionalArguments(arg1: number, arg2: string, arg3?: Date) {
arg1; arg2; arg3;
}

public methodWithDefaultedArguments(arg1: number = 2, arg2: string, arg3: Date = new Date()) {
arg1; arg2; arg3;
}
}

export namespace DerivedClassHasNoProperties {
Expand Down Expand Up @@ -865,4 +869,4 @@ export class AbstractClassReturner {
abstractProperty: 'hello-abstract-property'
}
}
}
}
27 changes: 26 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -2270,6 +2270,31 @@
},
"kind": "class",
"methods": [
{
"name": "methodWithDefaultedArguments",
"parameters": [
{
"name": "arg1",
"type": {
"optional": true,
"primitive": "number"
}
},
{
"name": "arg2",
"type": {
"primitive": "string"
}
},
{
"name": "arg3",
"type": {
"optional": true,
"primitive": "date"
}
}
]
},
{
"docs": {
"comment": "Used to verify verification of number of method arguments."
Expand Down Expand Up @@ -3070,5 +3095,5 @@
}
},
"version": "0.7.5",
"fingerprint": "XrmsNUcNdYiHEC6BRVT5XoeVmYQZzjYgiu6MyibgOwk="
"fingerprint": "Ywp3EW+cv1wVMiKbYcuDGIc5pIYI3RQtcVzwVBMg0aM="
}
Original file line number Diff line number Diff line change
Expand Up @@ -2270,6 +2270,31 @@
},
"kind": "class",
"methods": [
{
"name": "methodWithDefaultedArguments",
"parameters": [
{
"name": "arg1",
"type": {
"optional": true,
"primitive": "number"
}
},
{
"name": "arg2",
"type": {
"primitive": "string"
}
},
{
"name": "arg3",
"type": {
"optional": true,
"primitive": "date"
}
}
]
},
{
"docs": {
"comment": "Used to verify verification of number of method arguments."
Expand Down Expand Up @@ -3070,5 +3095,5 @@
}
},
"version": "0.7.5",
"fingerprint": "XrmsNUcNdYiHEC6BRVT5XoeVmYQZzjYgiu6MyibgOwk="
"fingerprint": "Ywp3EW+cv1wVMiKbYcuDGIc5pIYI3RQtcVzwVBMg0aM="
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ protected RuntimeTypeChecking(DeputyProps props): base(props)
{
}

[JsiiMethod("methodWithDefaultedArguments", null, "[{\"name\":\"arg1\",\"type\":{\"primitive\":\"number\",\"optional\":true}},{\"name\":\"arg2\",\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg3\",\"type\":{\"primitive\":\"date\",\"optional\":true}}]")]
public virtual void MethodWithDefaultedArguments(double? arg1, string arg2, DateTime? arg3)
{
InvokeInstanceVoidMethod(new object[]{arg1, arg2, arg3});
}

/// <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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ public RuntimeTypeChecking() {
software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this);
}

public void methodWithDefaultedArguments(@javax.annotation.Nullable final java.lang.Number arg1, final java.lang.String arg2, @javax.annotation.Nullable final java.time.Instant arg3) {
this.jsiiCall("methodWithDefaultedArguments", Void.class, java.util.stream.Stream.concat(java.util.stream.Stream.concat(java.util.stream.Stream.of(arg1), java.util.stream.Stream.of(java.util.Objects.requireNonNull(arg2, "arg2 is required"))), java.util.stream.Stream.of(arg3)).toArray());
}

public void methodWithDefaultedArguments(@javax.annotation.Nullable final java.lang.Number arg1, final java.lang.String arg2) {
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());
}

/**
* Used to verify verification of number of method arguments.
*/
Expand Down
10 changes: 10 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 @@ -2417,6 +2417,16 @@ RuntimeTypeChecking
.. py:method:: methodWithDefaultedArguments([arg1, arg2, [arg3]])
:param arg1:
:type arg1: number or undefined
:param arg2:
:type arg2: string
:param arg3:
:type arg3: date 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
Original file line number Diff line number Diff line change
Expand Up @@ -618,14 +618,18 @@ export class Assembler implements Emitter {
if (LOG.isTraceEnabled()) {
LOG.trace(`Processing parameter: ${colors.cyan(paramSymbol.name)}`);
}
const paramDeclaration = paramSymbol.valueDeclaration as ts.ParameterDeclaration;
const parameter: spec.Parameter = {
name: paramSymbol.name,
type: await this._typeReference(this._typeChecker.getTypeAtLocation(paramSymbol.valueDeclaration), paramSymbol.valueDeclaration),
variadic: !!(paramSymbol.valueDeclaration as ts.ParameterDeclaration).dotDotDotToken
variadic: !!paramDeclaration.dotDotDotToken
};
if (parameter.variadic) {
parameter.type = (parameter.type as spec.CollectionTypeReference).collection.elementtype;
}
if (paramDeclaration.initializer != null) {
parameter.type.optional = true;
}
this._visitDocumentation(paramSymbol, parameter);
return parameter;
}
Expand Down

0 comments on commit 8208b67

Please sign in to comment.