feat(dotnet): handling optional and variadic parameters

[assyadh]

Handling optional and variadic parameters in .NET

Emits the =null or params[] keywords when required in constructors or methods.

Ran pack.sh in the CDK with this change, and the S3 construct now looks better:

Optionals:

public Bucket(Amazon.CDK.Construct scope, string id, Amazon.CDK.AWS.S3.IBucketProps props = null): base(new DeputyProps(new object[]{scope, id, props})) { }

Making the C# call look like:

var bucket = new Bucket(this, "bucketName");

Rather than

var bucket = new Bucket(this, "bucketName", null);

Variadic:

Tested with null values, empty array, one value array, multiple values array.

// Array with no value in constructor params
var variadicClassNoParams = new VariadicMethod();

// Array with null value in constructor params
var variadicClassNullParams = new VariadicMethod(null);

// Array with one value in constructor params
var variadicClassOneParam = new VariadicMethod(1);

// Array with multiple values in constructor params
var variadicClassMultipleParams = new VariadicMethod(1, 2, 3, 4);

Fixes #153
Fixes #210

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[RomainMuller]

Should variadic arguments be defaulted to []?

[assyadh]

Should variadic arguments be defaulted to []?

In C#, a parameter array can not have a default value. It is by definition optional because it is passed as an empty array if not specified.

C# spec: http://download.microsoft.com/download/3/8/8/388e7205-bc10-4226-b2a8-75351c669b09/csharp%20language%20specification.doc

Section 7.4.1:

In particular, note that an empty array is created when there are zero arguments given for the parameter array.

This means no changes to the runtime should be required, as the parameter will be passed as an empty array.

[costleya]

Have we made sure this works with the current runtime? Have the compliance tests been updated to reflect this change?

[costleya]

The embedded if and else both set optionalKeyword to the same. Move this set to the parent if.

[assyadh]

Oops, this is a refactoring leftover. Removing

[costleya]

is this an else if?

[assyadh]

It looks like JSII is already doing the "mutual exclusion", but no harm in enforcing it with an if/else

[RomainMuller]

Yeah actually TypeScript won't allow you to use both the variadic syntax and the optional syntax at the same time... So if it's variadic, it cannot be optional, and vice-versa. But there's definitely no harm in checking again.

[assyadh]

Have we made sure this works with the current runtime? Have the compliance tests been updated to reflect this change?

Tested with the CDKHello app (the S3 bucket instruction used to require a useless new BucketProps(), not anymore), calling 'cdk-synth' works fine.

Tested with the ported EC2/ECS sample stack, which used to require useless new Ec2TaskDefinitionProps() and useless new RepositoryImageProps(), calling 'cdk-synth' works fine.

Looks like the variadic might need some runtime work. Looking into this and holding off the PR so far.

[assyadh]

This is ready to be reviewed/merged. tested with the CDK for Lambda and variadic layers.

Also added a full suite of compliance tests that run with the runtime with all sorts of tests around optionals and variadic params (nulls, empty, one value array, multiple values array etc..).

[RomainMuller]

Looking super good. Love the small enhancements to readability that you brought in there & the comments... Sweet sweet stuff!