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

Support variadic parameters for .NET #153

Closed
mpiroc opened this issue Aug 6, 2018 · 3 comments · Fixed by #680
Closed

Support variadic parameters for .NET #153

mpiroc opened this issue Aug 6, 2018 · 3 comments · Fixed by #680
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md language/dotnet Related to .NET bindings (C#, F#, ...) p0

Comments

@mpiroc
Copy link
Contributor

mpiroc commented Aug 6, 2018

This might be more involved that just adding the "params" keyword. Consider the following signature:

public void DoSomething(params object[] myParam)
{
}

// ...

object[] foo = // ...
// Because an object[] is also an object, the compiler treats foo as the first element
// of the params array, rather than the params array itself.
DoSomething(foo);

But in this case, there's no ambiguity:

public void DoSomething(params string[] myParam)
{
}

// ...

string[] foo = // ...

// The compiler treats foo as the params array, rather than the first element of the params array.
DoSomething(foo);

This might not matter, since generated code does not include any invocations of the generated methods. Just something to keep in mind.

@mpiroc mpiroc added language/dotnet Related to .NET bindings (C#, F#, ...) enhancement good first issue Related to contributions. See CONTRIBUTING.md labels Aug 11, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@fulghum fulghum added the p0 label Apr 2, 2019
@fulghum fulghum added the effort/medium Medium work item – a couple days of effort label Apr 22, 2019
@McDoit
Copy link

McDoit commented Apr 25, 2019

I guess i'm running into a related issue to this now in 0.29 of the C# CDK

AddActions of the PolicyStatement has the following signature
[JsiiMethod("addActions", "{\"type\":{\"fqn\":\"@aws-cdk/aws-iam.PolicyStatement\"}}", "[{\"name\":\"actions\",\"variadic\":true,\"type\":{\"primitive\":\"string\"}}]", false, false)]

But the generated C# method has the following signature, with a string
public virtual PolicyStatement AddActions(string actions)

Is there anyway i can create a correct string of a string params, or is this just something that need to be solved in this issue?

@bfeinb
Copy link

bfeinb commented Apr 25, 2019 via email

@McDoit
Copy link

McDoit commented Apr 26, 2019

This has been an issue for a while. The way I deal with this situation is to create an extension method to get the behavior I want. For my purposes I wanted to add multiple actions via a comma delimited set of actions as a single string, but you could easily go with an IEnumerable instead. The extension method I use is included below. It would be better to teach JSII how to deal with variable number of arguments in C#. Unfortunately I’m slammed on other projects at the moment, or I’d look at contributing something.

--Bonnie ///

/// Adds actions to a policy statement from a comma separated list of actions names as part of initializing IAM objects. /// /// What policy statement /// The names of the actions to add /// public static PolicyStatement AddActionsFromString(this PolicyStatement statement, string actions) { var policyStatement = statement; foreach (string action in actions.Split(", ")) { policyStatement = policyStatement.AddAction(action); } return policyStatement; } From: Erik Karlsson notifications@github.com Sent: Thursday, April 25, 2019 6:15 AM To: awslabs/jsii jsii@noreply.github.com Cc: Feinberg, Bonnie feinbb@amazon.com; Manual manual@noreply.github.com Subject: Re: [awslabs/jsii] Support variadic parameters for .NET (#153) I guess i'm running into a related issue to this now in 0.29 of the C# CDK AddActions of the PolicyStatement has the following signature [JsiiMethod("addActions", "{"type":{"fqn":"@aws-cdk/aws-iam.PolicyStatement"}}", "[{"name":"actions","variadic":true,"type":{"primitive":"string"}}]", false, false)] But the generated C# method has the following signature, with a string public virtual PolicyStatement AddActions(string actions) Is there anyway i can create a correct string of a string params, or is this just something that need to be solved in this issue? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#153 (comment)>, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AKCL62L6TAIWZTQYUNEVUXTPSGVGDANCNFSM4FOENHGQ.

Thank you @bfeinb

I tried something similar with a static method and looping over several actions, but then i get the following error in the app.Run() step:

Amazon.JSII.Runtime.JsiiException: 'While synthesizing mimer-ses-15/db-key/Resource: System.ArgumentException: Could not convert value 'System.Collections.Generic.Dictionary`2[System.String,System.Object]' with unrecognized type
Parameter name: value

Only reason i need it is beacuase of a related issue in the new 0.29, aws/aws-cdk#2375
I guess i will have to wait for that, or just create a new statement for every action instead

@RomainMuller RomainMuller added this to the .NET Support milestone Jul 30, 2019
RomainMuller pushed a commit that referenced this issue Aug 9, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md language/dotnet Related to .NET bindings (C#, F#, ...) p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants