Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dotnet): handling optional and variadic parameters #680

Merged
merged 9 commits into from
Aug 9, 2019
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