Skip to content

Commit

Permalink
feat(dotnet): handling optional and variadic parameters (#680)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Hamza Assyad authored and RomainMuller committed Aug 9, 2019
1 parent 2f40eeb commit e8b5a35
Show file tree
Hide file tree
Showing 21 changed files with 134 additions and 44 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Amazon.JSII.Runtime.Deputy;
using Amazon.JSII.Tests.CalculatorNamespace;
using CompositeOperation = Amazon.JSII.Tests.CalculatorNamespace.composition.CompositeOperation;
Expand Down Expand Up @@ -861,6 +862,56 @@ public void NullShouldBeTreatedAsUndefined()
obj.VerifyPropertyIsUndefined();
}

[Fact(DisplayName = Prefix + nameof(OptionalAndVariadicArgumentsTest))]
public void OptionalAndVariadicArgumentsTest()
{
// ctor
var objWithOptionalProvided = new NullShouldBeTreatedAsUndefined("param1", null);
var objWithoutOptionalProvided = new NullShouldBeTreatedAsUndefined("param1");

// method argument called with null value
objWithoutOptionalProvided.GiveMeUndefined(null);

// method argument called without null value
objWithoutOptionalProvided.GiveMeUndefined();

// 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);

// Variadic parameter with null passed
variadicClassNoParams.AsArray(Double.MinValue, null);

// Variadic parameter with default value used
variadicClassNoParams.AsArray(Double.MinValue);

var list = new List<double>();

// Variadic parameter with array with no value
variadicClassNoParams.AsArray(Double.MinValue, list.ToArray());

// Variadic parameter with array with one value
list.Add(1d);
variadicClassNoParams.AsArray(Double.MinValue, list.ToArray());

// Variadic parameter with array with multiple value
list.Add(2d);
list.Add(3d);
list.Add(4d);
list.Add(5d);
list.Add(6d);

variadicClassNoParams.AsArray(Double.MinValue, list.ToArray());
}

[Fact(DisplayName = Prefix + nameof(JsiiAgent))]
public void JsiiAgent()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,40 @@ static object[] ConvertArguments(Parameter[] parameters, params object[] argumen
throw new ArgumentException("Arguments do not match method parameters", nameof(arguments));
}

var cleanedArgs = new List<object>(arguments);
var cleanedParams = new List<Parameter>(parameters);

// Handling variadic parameters (null array, empty array, one value array, n values array..)
if (parameters.Length > 0 && parameters.Last().IsVariadic)
{
// Last parameter is variadic, let's explode the .NET attributes
Array variadicValues = arguments.Last() as Array;

// We remove the last argument (the variadic array);
cleanedArgs.RemoveAt(cleanedArgs.Count - 1);

// A null value could be passed as a params
if (variadicValues != null)
{
// We save the last parameter to backfill the parameters list
var lastParameter = cleanedParams.Last();

for (int i = 0; i < variadicValues.Length; i++)
{
// Backfill the arguments
cleanedArgs.Add(variadicValues.GetValue(i));

// Backfill the parameters if necessary, for a 1:1 mirror with the cleanedArgs
if (cleanedArgs.Count != cleanedParams.Count)
cleanedParams.Add(lastParameter);
}
}
}

IFrameworkToJsiiConverter converter = serviceProvider.GetRequiredService<IFrameworkToJsiiConverter>();
IReferenceMap referenceMap = serviceProvider.GetRequiredService<IReferenceMap>();

return parameters.Zip(arguments, (parameter, frameworkArgument) =>
return cleanedParams.Zip(cleanedArgs, (parameter, frameworkArgument) =>
{
if (!converter.TryConvert(parameter, referenceMap, frameworkArgument, out object jsiiArgument))
{
Expand Down
51 changes: 30 additions & 21 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,26 +229,20 @@ export class DotNetGenerator extends Generator {
this.code.openBlock(`public${inner}${absPrefix} class ${className}${implementsExpr}`);

// Compute the class parameters
// TODO: add the support for optional parameters
// Not achievable as of today and not supported by the current generator
// https://github.com/awslabs/jsii/issues/210
let parametersDefinition = '';
let parametersBase = '';
const initializer = cls.initializer;
if (initializer) {
this.dotnetDocGenerator.emitDocs(initializer);
this.dotnetRuntimeGenerator.emitDeprecatedAttributeIfNecessary(initializer);
if (initializer.parameters) {
parametersDefinition = this.renderParametersString(initializer.parameters);
for (const p of initializer.parameters) {
const pType = this.typeresolver.toDotNetType(p.type);
const isOptionalPrimitive = this.isOptionalPrimitive(p) ? '?' : '';
if (parametersDefinition !== '') {
parametersDefinition += ', ';
parametersBase += `${this.nameutils.convertParameterName(p.name)}`;
// If this is not the last parameter, append ,
if (initializer.parameters.indexOf(p) !== initializer.parameters.length - 1) {
parametersBase += ', ';
}
parametersDefinition += `${pType}${isOptionalPrimitive} ${this.nameutils.convertParameterName(p.name)}`;
parametersBase += `${this.nameutils.convertParameterName(p.name)}`;

}
}

Expand Down Expand Up @@ -443,18 +437,34 @@ export class DotNetGenerator extends Generator {
}
}

// TODO: I uncovered an issue with optional parameters and ordering them
// They are currently not supported by the generator.
// In C#, optional parameters need to be declared after required parameters
// We could make changes to the parameters ordering in the jsii model to support them
// But then an optional parameter becoming non optional would create a mess
// https://github.com/awslabs/jsii/issues/210
/**
* Renders method parameters string
*/
private renderMethodParameters(method: spec.Method): string {
return this.renderParametersString(method.parameters);
}

/**
* Renders parameters string for methods or constructors
*/
private renderParametersString(parameters: spec.Parameter[] | undefined): string {
const params = [];
if (method.parameters) {
for (const p of method.parameters) {
const isOptionalPrimitive = this.isOptionalPrimitive(p) ? '?' : '';
const st = `${this.typeresolver.toDotNetType(p.type)}${isOptionalPrimitive} ${this.nameutils.convertParameterName(p.name)}`;
if (parameters) {
for (const p of parameters) {
let optionalPrimitive = '';
let optionalKeyword = '';
let type = this.typeresolver.toDotNetType(p.type);
if (p.optional) {
optionalKeyword = ' = null';
if (this.isOptionalPrimitive(p)) {
optionalPrimitive = '?';

}
} else if (p.variadic) {
type = `params ${type}[]`;
}
const st =
`${type}${optionalPrimitive} ${this.nameutils.convertParameterName(p.name)}${optionalKeyword}`;
params.push(st);
}
}
Expand Down Expand Up @@ -637,7 +647,6 @@ export class DotNetGenerator extends Generator {
let isVirtualKeyWord = '';
// If the prop parent is a class
if (cls.kind === spec.TypeKind.Class) {

const implementedInBase = this.isMemberDefinedOnAncestor(cls as spec.ClassType, prop);
if (implementedInBase || datatype || proxy) {
// Override if the property is in a datatype or proxy class or declared in a parent class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class Calculator : Amazon.JSII.Tests.CalculatorNamespace.composition.Comp
/// <remarks>
/// stability: Experimental
/// </remarks>
public Calculator(Amazon.JSII.Tests.CalculatorNamespace.ICalculatorProps props): base(new DeputyProps(new object[]{props}))
public Calculator(Amazon.JSII.Tests.CalculatorNamespace.ICalculatorProps props = null): base(new DeputyProps(new object[]{props}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ protected DataRenderer(DeputyProps props): base(props)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "render", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"data\",\"optional\":true,\"type\":{\"fqn\":\"@scope/jsii-calc-lib.MyFirstStruct\"}}]")]
public virtual string Render(Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.IMyFirstStruct data)
public virtual string Render(Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.IMyFirstStruct data = null)
{
return InvokeInstanceMethod<string>(new object[]{data});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class DefaultedConstructorArgument : DeputyBase
/// <remarks>
/// stability: Experimental
/// </remarks>
public DefaultedConstructorArgument(double? arg1, string arg2, System.DateTime? arg3): base(new DeputyProps(new object[]{arg1, arg2, arg3}))
public DefaultedConstructorArgument(double? arg1 = null, string arg2 = null, System.DateTime? arg3 = null): base(new DeputyProps(new object[]{arg1, arg2, arg3}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class DeprecatedClass : DeputyBase
/// stability: Deprecated
/// </remarks>
[System.Obsolete("this constructor is \"just\" okay")]
public DeprecatedClass(string readonlyString, double? mutableNumber): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
public DeprecatedClass(string readonlyString, double? mutableNumber = null): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected DoNotRecognizeAnyAsOptional(DeputyProps props): base(props)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "method", parametersJson: "[{\"name\":\"_requiredAny\",\"type\":{\"primitive\":\"any\"}},{\"name\":\"_optionalAny\",\"optional\":true,\"type\":{\"primitive\":\"any\"}},{\"name\":\"_optionalString\",\"optional\":true,\"type\":{\"primitive\":\"string\"}}]")]
public virtual void Method(object requiredAny, object optionalAny, string optionalString)
public virtual void Method(object requiredAny, object optionalAny = null, string optionalString = null)
{
InvokeInstanceVoidMethod(new object[]{requiredAny, optionalAny, optionalString});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected DocumentedClass(DeputyProps props): base(props)
/// stability: Stable
/// </remarks>
[JsiiMethod(name: "greet", returnsJson: "{\"type\":{\"primitive\":\"number\"}}", parametersJson: "[{\"docs\":{\"summary\":\"The person to be greeted.\"},\"name\":\"greetee\",\"optional\":true,\"type\":{\"fqn\":\"jsii-calc.Greetee\"}}]")]
public virtual double Greet(Amazon.JSII.Tests.CalculatorNamespace.IGreetee greetee)
public virtual double Greet(Amazon.JSII.Tests.CalculatorNamespace.IGreetee greetee = null)
{
return InvokeInstanceMethod<double>(new object[]{greetee});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected DontComplainAboutVariadicAfterOptional(DeputyProps props): base(props)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "optionalAndVariadic", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"optional\",\"optional\":true,\"type\":{\"primitive\":\"string\"}},{\"name\":\"things\",\"type\":{\"primitive\":\"string\"},\"variadic\":true}]")]
public virtual string OptionalAndVariadic(string optional, string things)
public virtual string OptionalAndVariadic(string optional = null, params string[] things)
{
return InvokeInstanceMethod<string>(new object[]{optional, things});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class ExperimentalClass : DeputyBase
/// <remarks>
/// stability: Experimental
/// </remarks>
public ExperimentalClass(string readonlyString, double? mutableNumber): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
public ExperimentalClass(string readonlyString, double? mutableNumber = null): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ public interface IIInterfaceWithOptionalMethodArguments
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "hello", parametersJson: "[{\"name\":\"arg1\",\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg2\",\"optional\":true,\"type\":{\"primitive\":\"number\"}}]")]
void Hello(string arg1, double? arg2);
void Hello(string arg1, double? arg2 = null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ private IInterfaceWithOptionalMethodArgumentsProxy(ByRefValue reference): base(r
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "hello", parametersJson: "[{\"name\":\"arg1\",\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg2\",\"optional\":true,\"type\":{\"primitive\":\"number\"}}]")]
public void Hello(string arg1, double? arg2)
public void Hello(string arg1, double? arg2 = null)
{
InvokeInstanceVoidMethod(new object[]{arg1, arg2});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class NullShouldBeTreatedAsUndefined : DeputyBase
/// <remarks>
/// stability: Experimental
/// </remarks>
public NullShouldBeTreatedAsUndefined(string param1, object optional): base(new DeputyProps(new object[]{param1, optional}))
public NullShouldBeTreatedAsUndefined(string param1, object optional = null): base(new DeputyProps(new object[]{param1, optional}))
{
}

Expand All @@ -28,7 +28,7 @@ protected NullShouldBeTreatedAsUndefined(DeputyProps props): base(props)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "giveMeUndefined", parametersJson: "[{\"name\":\"value\",\"optional\":true,\"type\":{\"primitive\":\"any\"}}]")]
public virtual void GiveMeUndefined(object @value)
public virtual void GiveMeUndefined(object @value = null)
{
InvokeInstanceVoidMethod(new object[]{@value});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class OptionalConstructorArgument : DeputyBase
/// <remarks>
/// stability: Experimental
/// </remarks>
public OptionalConstructorArgument(double arg1, string arg2, System.DateTime? arg3): base(new DeputyProps(new object[]{arg1, arg2, arg3}))
public OptionalConstructorArgument(double arg1, string arg2, System.DateTime? arg3 = null): base(new DeputyProps(new object[]{arg1, arg2, arg3}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class OptionalStructConsumer : DeputyBase
/// <remarks>
/// stability: Experimental
/// </remarks>
public OptionalStructConsumer(Amazon.JSII.Tests.CalculatorNamespace.IOptionalStruct optionalStruct): base(new DeputyProps(new object[]{optionalStruct}))
public OptionalStructConsumer(Amazon.JSII.Tests.CalculatorNamespace.IOptionalStruct optionalStruct = null): base(new DeputyProps(new object[]{optionalStruct}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected RuntimeTypeChecking(DeputyProps props): base(props)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "methodWithDefaultedArguments", parametersJson: "[{\"name\":\"arg1\",\"optional\":true,\"type\":{\"primitive\":\"number\"}},{\"name\":\"arg2\",\"optional\":true,\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg3\",\"optional\":true,\"type\":{\"primitive\":\"date\"}}]")]
public virtual void MethodWithDefaultedArguments(double? arg1, string arg2, System.DateTime? arg3)
public virtual void MethodWithDefaultedArguments(double? arg1 = null, string arg2 = null, System.DateTime? arg3 = null)
{
InvokeInstanceVoidMethod(new object[]{arg1, arg2, arg3});
}
Expand All @@ -33,7 +33,7 @@ public virtual void MethodWithDefaultedArguments(double? arg1, string arg2, Syst
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "methodWithOptionalAnyArgument", parametersJson: "[{\"name\":\"arg\",\"optional\":true,\"type\":{\"primitive\":\"any\"}}]")]
public virtual void MethodWithOptionalAnyArgument(object arg)
public virtual void MethodWithOptionalAnyArgument(object arg = null)
{
InvokeInstanceVoidMethod(new object[]{arg});
}
Expand All @@ -43,7 +43,7 @@ public virtual void MethodWithOptionalAnyArgument(object arg)
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "methodWithOptionalArguments", parametersJson: "[{\"name\":\"arg1\",\"type\":{\"primitive\":\"number\"}},{\"name\":\"arg2\",\"type\":{\"primitive\":\"string\"}},{\"name\":\"arg3\",\"optional\":true,\"type\":{\"primitive\":\"date\"}}]")]
public virtual void MethodWithOptionalArguments(double arg1, string arg2, System.DateTime? arg3)
public virtual void MethodWithOptionalArguments(double arg1, string arg2, System.DateTime? arg3 = null)
{
InvokeInstanceVoidMethod(new object[]{arg1, arg2, arg3});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class StableClass : DeputyBase
/// <remarks>
/// stability: Stable
/// </remarks>
public StableClass(string readonlyString, double? mutableNumber): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
public StableClass(string readonlyString, double? mutableNumber = null): base(new DeputyProps(new object[]{readonlyString, mutableNumber}))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected StructPassing(DeputyProps props): base(props)
/// stability: External
/// </remarks>
[JsiiMethod(name: "howManyVarArgsDidIPass", returnsJson: "{\"type\":{\"primitive\":\"number\"}}", parametersJson: "[{\"name\":\"_positional\",\"type\":{\"primitive\":\"number\"}},{\"name\":\"inputs\",\"type\":{\"fqn\":\"jsii-calc.TopLevelStruct\"},\"variadic\":true}]")]
public static double HowManyVarArgsDidIPass(double positional, Amazon.JSII.Tests.CalculatorNamespace.ITopLevelStruct inputs)
public static double HowManyVarArgsDidIPass(double positional, params Amazon.JSII.Tests.CalculatorNamespace.ITopLevelStruct[] inputs)
{
return InvokeStaticMethod<double>(typeof(Amazon.JSII.Tests.CalculatorNamespace.StructPassing), new object[]{positional, inputs});
}
Expand Down
Loading

0 comments on commit e8b5a35

Please sign in to comment.