Fix S3878 FP: when a params argument is named

[afpirimoglu]

Description

S3878 should not be raised if the params argument is called by its name, which requires the value of it to be passed as an array. There are times params argument should be called by its name, as when there are other arguments with default values on them.

Repro steps

public void MethodWithParams(
    int intParam = 5, 
    string stringParam = "abc", 
    params double[] doubleArgs)
{
    // Does something
}

public void CallMethodWithParams()
{
    // To rely on the default value of stringParam in the call, one has to 
    // use name of doubleArgs params parameter when calling MethodWithParams. And when params parameters
    // are used by their name in the call, they have to be passes in as arrays. Otherwise code doesn't compile
    // Rule S3878 should not be raised when the params argument is being called by its name.
    this.MethodWithParams(intParam:5, doubleArgs:new double[] { 0.2, 0.3, });

    // another case, relies on both intParam and stringParam's default values
    // but provides values for doubleArgs
    this.MethodWithParams(doubleArgs: new double[] { 0.2, 0.3, });
}

Expected behavior

S3878 should not be raised when the method is calling the params argument by its name.

Actual behavior

S3878 does not make distinction on whether the params argument was being called by name or not.

Known workarounds

None

[mary-georgiou-sonarsource]

Hello @afpirimoglu,
I confirm this is a false positive.
I've added it to our backlog.

[afpirimoglu]

Taking a look at the repro code for this 6893, I am afraid it is not actually reproducing what I have described in this ticket.
This ticket is not about the first argument itself being an array. This ticket is about C# compiler requiring (forcing) developer to pass the whole params arguments as an array when the params argument is named in the call and when there is more than one argument in the params.

Especially when there is another argument of the method that has an optional value defined, if the caller wants to rely on that optional value of that argument (i.e. not provide that in the call), then the caller has to reference the params argument by name. And if the caller is passing more than one item in the params argument, then the C# compiler requires the caller to pass the params argument as an array. Otherwise code does not compile!

Expanding on the example code I had used in the description:

      public void MethodWithParams(
            string stringParam = "abc",  // Note that this is an optional parameter!!!!
            params double[] doubleArgs)
        {
            // Does something
        }
            // Need to call MethodWithParams and want to use the default value of stringParam
            // argument in the call, i.e. don't want to provide stringParam value in the call.
            // Then I have to use name of the params doubleArgs in the call, as below calls do.

            // This below call is fine, only one item in the params argument
            MethodWithParams(doubleArgs: 0.2);

            // This below commented line does not compile! Compiler gives error 
            // "CS8323 Named argument 'doubleArgs' is used out-of-position but is followed by an unnamed argument":
            // MethodWithParams(doubleArgs: 0.2, 0.3); 

            // Neither this below commented line compiles, it gives the same CS8323 C# compiler error:
            //MethodWithParams(0.2, 0.3); 

            // Only this below call compiles when the params arg being passed has more than one item in it,
            // and there is an optional parameter in the method signature.
            // But currently this below call gets Sonar warning S3878!
            MethodWithParams(doubleArgs: new double[] { 0.2, 0.3 });

Hope above helps to demonstrate the issue better.

And sometimes you want to specify the name of the argument in the call for better readibility, even if there was no other optional argument that compiler forces you to specify the name of the argument. A code example for it:

        public void MethodWithParams(
            double weight,  // Note this is not optional parameter
            params double[] dimensions,
        {
            // Does something
        }
// this call is ambiguous for the reader of the code:
MethodWithParams(0.4, 0.1, 0.2, 0.5)

// This below call is not ambiguous to the reader, since the parameter names are given.
// But this below call raises the S3878 warning! It should not have, because when the 
// params argument is named in the call, and if there is more than one item in it, 
// C# compiler forces that to be passed as an array!
MethodWithParams(weight: 0.4, dimensions: double[] {0.1, 0.2, 0.5});

// This does not compile!!
MethodWithParams(weight: 0.4, dimensions: 0.1, 0.2, 0.5);

[afpirimoglu]

In summary, if the params argument was passed by name in the call and the params argument value contained more than one item in it, S3878 should not be applied to it; whether there was an optional argument in the method signature or not.
Because when the params argument is called by its name and if it had more than one item in it, C# compiler forces it to be passed as an array.

Forgot to add: Thanks for looking into this!

[mary-georgiou-sonarsource]

Hello @afpirimoglu,

I'm a bit confused.
Why do you consider the repro not to represent what you just wrote?

Method(a: 1, argumentArray: new int[] { 1, 2 }); // Noncompliant FP

Maybe the Noncomliant in the comment is confusing? As this is practically for us an assertion that the rule is raising there.
That's why we add the FP notation afterward to indicate that this is a False positive issue.

I confirm that we don't want to raise an issue when you pass an array/ initialize an array for params in the case of a named argument.

[afpirimoglu]

`public class Repro6893
{
//Reproducer for #6893

public void Method(int a, params object[] argumentArray) { }


public void CallMethod()
{
    Method(a: 1, argumentArray: new int[] { 1, 2 }); // Noncompliant FP
}

}`
Hi @mary-georgiou-sonarsource ,
Copied above the code from repro. There the Method has its params argument type as object[] , but the call made to it is passing int[]. So that fits the 6894 issue.

Changing the Method params argument as int[] will separate it from 6894. Something like this below, which I also added there that when the params argument has only one element in it, compiler allows that being passed without an array even if it is called by name.

` public void MethodInt(int a, params int[] argumentArray) { }

    public void CallMethod()
    {
        MethodInt(a: 1, argumentArray: new int[] { 1 }); // Should raise S3878 because only one element in the params argument array
        MethodInt(a: 1, argumentArray: 1); // S3878 compliant call, when params argument has one element. When there is one element in the array compiler allows passing only that in the named params argument.
        MethodInt(a: 1, argumentArray: new int[] { 1, 2 }); // Currently S3878 is raised, when params argument has more than one element
        MethodInt(argumentArray: new int[] { 1, 2 }, a: 1); // This is not raising S3878, and it shouldn't. But above shouldn't also
    }`

Also looking at the repro test code, I found out if the arguments are out of order, Sonar already is not raising issue on named params argument call, which is the last MethodInt call I made here.