Skip to content

Commit

Permalink
fix: type unions in variadic position leads to invalid code-gen
Browse files Browse the repository at this point in the history
The code-gen for type unions was incorrect, as the value is typically
array-typed, and the generated code hence needs to treat the value as
such.
  • Loading branch information
RomainMuller committed Aug 24, 2022
1 parent c73c2ee commit 70a806e
Show file tree
Hide file tree
Showing 14 changed files with 638 additions and 286 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,18 @@ public void NestedUnion()
}));
Assert.Equal("Expected argument unionProperty[0][\"bad\"] to be one of: Amazon.JSII.Tests.CalculatorNamespace.IStructA, Amazon.JSII.Tests.CalculatorNamespace.IStructB; received System.String (Parameter 'unionProperty')", exception3.Message);
}

[Fact(DisplayName = Prefix + nameof(Variadic))]
public void Variadic()
{
var exception1 = Assert.Throws<System.ArgumentException>(() =>
new VariadicTypeUnion(
new StructA{RequiredString = "present"},
1337.42
));
Assert.Equal("Expected argument union[1] to be one of: Amazon.JSII.Tests.CalculatorNamespace.IStructA, Amazon.JSII.Tests.CalculatorNamespace.IStructB; received System.Double (Parameter 'union')", exception1.Message);

Assert.NotNull(new VariadicTypeUnion());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@ func TestNestedUnion(t *testing.T) {
}()
}

func TestVariadic(t *testing.T) {
func() {
defer expectPanic(t, "parameter union[1] must be one of the allowed types: *StructA, *StructB; received 1337.42 (a float64)")
jsiicalc.NewVariadicTypeUnion(jsiicalc.StructA{RequiredString: jsii.String("present")}, 1337.42)
}()

// Should not raise
jsiicalc.NewVariadicTypeUnion()
}

func expectPanic(t *testing.T, expected string) {
if err := recover(); err != nil {
actual := fmt.Sprintf("%v", err)
Expand Down
16 changes: 16 additions & 0 deletions packages/@jsii/python-runtime/tests/test_runtime_type_checking.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,19 @@ def test_anonymous_object(self):

iface = jsii_calc.anonymous.UseOptions.provide("A")
assert jsii_calc.anonymous.UseOptions.consume(iface) == "A"

def test_nested_union(self):
with pytest.raises(
TypeError,
match=re.escape("type of argument union_property[0] must be one of (Mapping[str, Union[jsii_calc.StructA, Dict[str, Any], jsii_calc.StructB]], Sequence[Union[jsii_calc.StructA, Dict[str, Any], jsii_calc.StructB]]); got float instead")
):
jsii_calc.ClassWithNestedUnion([1337.42]) # type:ignore

def test_variadic(self):
with pytest.raises(
TypeError,
match=re.escape("type of argument union[1] must be one of (jsii_calc.StructA, jsii_calc.StructB); got float instead")
):
jsii_calc.VariadicTypeUnion(jsii_calc.StructA(required_string="present"), 1337.42) # type:ignore

jsii_calc.VariadicTypeUnion()
8 changes: 8 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3082,3 +3082,11 @@ export class ClassWithNestedUnion {
>,
) {}
}

export class VariadicTypeUnion {
public union: Array<StructA | StructB>;

public constructor(...union: Array<StructA | StructB>) {
this.union = union;
}
}
73 changes: 72 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -15428,6 +15428,77 @@
"name": "VariadicMethod",
"symbolId": "lib/compliance:VariadicMethod"
},
"jsii-calc.VariadicTypeUnion": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.VariadicTypeUnion",
"initializer": {
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3089
},
"parameters": [
{
"name": "union",
"type": {
"union": {
"types": [
{
"fqn": "jsii-calc.StructA"
},
{
"fqn": "jsii-calc.StructB"
}
]
}
},
"variadic": true
}
],
"variadic": true
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3086
},
"name": "VariadicTypeUnion",
"properties": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3087
},
"name": "union",
"type": {
"collection": {
"elementtype": {
"union": {
"types": [
{
"fqn": "jsii-calc.StructA"
},
{
"fqn": "jsii-calc.StructB"
}
]
}
},
"kind": "array"
}
}
}
],
"symbolId": "lib/compliance:VariadicTypeUnion"
},
"jsii-calc.VirtualMethodPlayground": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -18131,5 +18202,5 @@
}
},
"version": "3.20.120",
"fingerprint": "LBLJQQycukWu6zWQmp2/IbKS/Sfd+4e2zWrX+1KA+Aw="
"fingerprint": "Ze43eowG9ImRufT3MQ8yO+bW8JzOQlZIYtFsjpc960E="
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CollectionKind } from '@jsii/spec';
import { CodeMaker } from 'codemaker';
import { createHash } from 'crypto';
import { Parameter, TypeReference } from 'jsii-reflect';
Expand Down Expand Up @@ -27,7 +28,14 @@ export class ParameterValidator {
argName,
expr,
`${noMangle ? '' : 'argument '}{${argName}}`,
param.type,
param.variadic
? new TypeReference(param.system, {
collection: {
kind: CollectionKind.Array,
elementtype: param.type.spec!,
},
})
: param.type,
param.optional,
);
if (validation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,15 @@ export class ParameterValidator {
const descr = `parameter ${param.name}`;

const validations = new Array<Validation>();
if (!param.isOptional) {
if (!param.isOptional && !param.isVariadic) {
validations.push(Validation.nullCheck(expr, descr, param.reference));
}
const validation = Validation.forTypeMap(
expr,
descr,
param.reference.typeMap,
param.isVariadic
? { type: 'array', value: param.reference }
: param.reference.typeMap,
);
if (validation) {
validations.push(validation);
Expand Down Expand Up @@ -144,7 +146,9 @@ export class ParameterValidator {

public emitCall(code: CodeMaker): void {
const recv = this.receiver?.name ? `${this.receiver.name}.` : '';
const params = this.parameters.map((p) => p.name).join(', ');
const params = this.parameters
.map((p) => (p.isVariadic ? `&${p.name}` : p.name))
.join(', ');

code.openBlock(`if err := ${recv}${this.name}(${params}); err != nil`);
code.line(`panic(err)`);
Expand All @@ -162,7 +166,7 @@ export class ParameterValidator {
}${this.name}(${this.parameters
.map((p) =>
p.isVariadic
? `${p.name} []${p.reference.scopedReference(scope)}`
? `${p.name} *[]${p.reference.scopedReference(scope)}`
: p.toString(),
)
.join(', ')}) error`,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 70a806e

Please sign in to comment.