Powershell: Parameter binding problem with ValueFromRemainingArguments in PS functions

[dlwyatt]

Steps to reproduce

Define a PowerShell function with an array parameter using the ValueFromRemainingArguments property of the Parameter attribute. Instead of sending multiple arguments, send that parameter a single array argument.

 & {
     param(
         [string]
         [Parameter(Position=0)]
         $Root,

         [string[]]
         [Parameter(Position=1, ValueFromRemainingArguments)]
         $Extra)
     $Extra.Count;
     for ($i = 0; $i -lt $Extra.Count; $i++)
     {
        "${i}: $($Extra[$i])"
     }
 } root aa,bb

Expected behavior

The array should be bound to the parameter just as you sent it, the same way it works for cmdlets. (The "ValueFromRemainingArguments" behavior isn't used, in this case, it should just bind like any other array parameter type.) The output of the above script block should be:

2
0: aa
1: bb

Actual behavior

PowerShell appears to be performing type conversion on the argument to treat the array as a single element of the parameter's array, instead of checking first to see if more arguments will be bound as "remaining arguments" first. The output of the above script block is currently:

1
0: aa bb

Additional information

To demonstrate that the behavior of cmdlets is different, you can use this code:

Add-Type -OutputAssembly $env:temp\testBinding.dll -TypeDefinition @'
    using System;
    using System.Management.Automation;

    [Cmdlet("Test", "Binding")]
    public class TestBindingCommand : PSCmdlet
    {
        [Parameter(Position = 0)]
        public string Root { get; set; }

        [Parameter(Position = 1, ValueFromRemainingArguments = true)]
        public string[] Extra { get; set; }

        protected override void ProcessRecord()
        {
            WriteObject(Extra.Length);
            for (int i = 0; i < Extra.Length; i++)
            {
                WriteObject(String.Format("{0}: {1}", i, Extra[i]));
            }
        }
    }
'@

Import-Module $env:temp\testBinding.dll

Test-Binding root aa,bb

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSEdition                      Core
PSVersion                      6.0.0-alpha
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
WSManStackVersion              3.0
GitCommitId                    v6.0.0-alpha.9-107-g203ace04c09dbbc1ac00d6b497849cb69cc919fb-dirty
PSRemotingProtocolVersion      2.3
CLRVersion
SerializationVersion           1.1.0.1
BuildVersion                   3.0.0.0

[dlwyatt]

That does appear to be the case. Here's a trace of binding for the cmdlet with the array split into two string arguments:

trace-command ParameterBinding -PSHost -Expression { Test-Binding root aa bb }

# snip

DEBUG: ParameterBinding Information: 0 :             Binding collection parameter Extra: argument type [List`1], parameter type [System.String[]], collection type Array, element type [System.String],
coerceElementType
DEBUG: ParameterBinding Information: 0 :             Arg is IList with 2 elements

[dlwyatt]

There is logic like this already (see https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/CmdletParameterBinderController.cs#L1696), but since the original call to BindParameter didn't fail, it doesn't run, in the case of a PowerShell function.

Will submit a fix shortly.

[MarvTheRobot]

Is this not down to how the string array is being passed in? Would "aa","bb" work any better.

I only ask as something similar happens to me in PowerShell data files in DSC. If I comma separate nodes, they end up as an array in the first element of an object array. When I remove the comma, I get an array as expected without being put into a containing array. --> ok so 'similar' is a very loose term ;)

Either way, it's a good bit of investigation and work. Perhaps I should stop being lazy and do similar.

[dlwyatt]

The quotation marks aren't important in this case. PowerShell's in argument parsing mode at that point, so unless a string contains spaces or certain other special syntax characters (such as commas), it's okay to pass without quotes. That's how you can do things like dir c:\ instead of dir "c:\" all the time. :)

[MarvTheRobot]

Ah, well THAT is a very good point.

On Wed, Aug 24, 2016 at 7:01 AM +0100, "Dave Wyatt" [email protected] wrote:

The quotation marks aren't important in this case. PowerShell's in argument parsing mode at that point, so unless a string contains spaces or certain other special syntax characters (such as commas), it's okay to pass without quotes. That's how you can do things like dir c:\ instead of dir "c:\" all the time. :)

You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/PowerShell/PowerShell/issues/2035#issuecomment-241965543

[jpsnover]

It is working exactly as I expect. Here is the example which best illustrates what is happening:
root aa bb,cc dd,ee,ff
3
0: aa
1: bb cc
2: dd ee ff

[dlwyatt]

@jpsnover : In that example, you're sidestepping the problem by passing in three arguments to the $Extra parameter. The code path that's causing a problem is when you just pass the parameter an array without using the "additional arguments" syntax sugar.

[rkeithhill]

The current parameter binder is working as I expect too.

You can get the desired output by using splatting:

$params = @{Extra = @('aa','bb')}
...
} root @params

That said, it might be nice to have a convenient syntax for splatting unnamed/positional arguments that took an array like:

$extra = 'aa','bb'
...
} root @extra

But to make it really convenient it should work on an array literal e.g.

} root @@('aa','bb')

Not exactly sure what the syntax should be since today this doesn't work with hashtable literals either.

[dlwyatt]

I don't see how this can be expected behavior. Going back to the original example, giving the function a name this time:

function Do-Something {
     param(
         [string]
         [Parameter(Position=0)]
         $Root,

         [string[]]
         [Parameter(Position=1, ValueFromRemainingArguments)]
         $Extra)

     $Extra.Count;
     for ($i = 0; $i -lt $Extra.Count; $i++)
     {
        "${i}: $($Extra[$i])"
     }
 }

Do-Something Root Extra1 Extra2 Extra3
Do-Something Root @('Extra1', 'Extra2', 'Extra3')

Both of those calls _should_ have identical values for the $Extra variable inside the function. -Extra is the last positional parameter defined, and has the ValueFromRemainingArguments attribute. I can either pass it the whole array in one go (same as any other parameter defined with an array object type), or I can use the syntax that is enabled by ValueFromRemainingArguments instead.

But they're not identical, because of the bug that's demonstrated here. If you look at the discussion for the PR to fix it, there are even more weird scenarios involving cmdlets like Write-Output. (Parameter binding for functions takes a different code path than cmdlets, and has different quirks in this case.)

[rkeithhill]

Let's start with the case where there is no ValueFromRemainingArguments:

function Do-Something {
     param(
         [string]
         [Parameter(Position=0)]
         $Root,

         [string[]]
         [Parameter(Position=1)]
         $Extra)

     $Extra.Count;
     for ($i = 0; $i -lt $Extra.Count; $i++)
     {
        "${i}: $($Extra[$i])"
     }
 }

That results in:

148> Do-Something Root 'Extra1', 'Extra2', 'Extra3'
3
0: Extra1
1: Extra2
2: Extra3

That was your initial expected output. But this doesn't accept extra args: Do-Something Root Extra1 Extra2 Extra3. So you add in ValueFromRemainingArguments to handle that case and voila, that case works:

151> Do-Something Root Extra1 Extra2 Extra3
3
0: Extra1
1: Extra2
2: Extra3

Unfortunately the original array case behavior has changed.

152> Do-Something Root Extra1, Extra2, Extra3
1
0: Extra1 Extra2 Extra3

I can see how that appears to be buggy and I guess it comes down to what the semantics are for ValueFromRemainingArguments. I think the semantics change pretty significantly for a parameter decorated with ValueFromRemainingArguments (why does that quote from Unforgiven come to mind - nevermind).

Obviously, for such a parameter, PS aggregates all remaining/unbound arguments into a single argument value (converting to array if necessary) to supply to the parameter. That appears to work as you'd expect for multiple space separated args. But for arrays the behavior is perhaps not expected. However, I think for the following case it is working as expected:

154> Do-Something Root Extra1,Extra2,Extra3 1,3 $false
3
0: Extra1 Extra2 Extra3
1: 1 3
2: False

I'm not sure what else PowerShell could do here but treat the first array as a "single" argument in this case. But what to do about the case where you have a single argument that is an array?

155> Do-Something Root Extra1,Extra2,Extra3
1
0: Extra1 Extra2 Extra3

Should PowerShell be clever and if the first (and only) argument is an array, assign it as sort of the splatted value of the parameter instead of treating it as a single value in the array or remaining args? I don't know. But I can see a case being made that if the user wanted those as mulitple args they would have specified them as multiple args. Not trying to be snarky but really just wondering if there could be a case where the user really wanted the first (and only) argument to be treated as a single argument (of type array) and this cleverness takes away that ability?

[dlwyatt]

For that, you'd behave just like any other array parameter where you wanted to send in a single-element array (such as -ArgumentList when calling New-Object for constructors that take an array): unary comma.

Do-Something ,('Extra1', 'Extra2', 'Extra3')

[iSazonov]

I agree with @rkeithhill the current parameter binder for ValueFromRemainingArguments is working as expected - its by design and no fix needed.

[dlwyatt]

Well, whatever. I don't see how anyone can agree with the binding behavior being different when using the syntax sugar, but fine. My recommendation will be for people to never use ValueFromRemainingArguments when writing functions due to the surprise factor.

[iSazonov]

There are fans of ValueFromRemainingArguments https://beatcracker.wordpress.com/2014/12/01/splatting-and-mandatory-parameters/

[lzybkr]

I'd like to leave this open - I do think there is a bug here. Ideally, ValueFromRemainingArguments should work the same when calling a function or cmdlet.

The following sample demonstrates multiple ways behavior differs:

Add-Type -PassThru @"
using System.Management.Automation;

[Cmdlet("Write", "ThingC")]
public class WriteThingCommand : PSCmdlet {
    [Parameter(Mandatory = true, Position = 0, ValueFromRemainingArguments = true)] public string[] Things { get; set; }

    override protected void ProcessRecord() {
        var sb = new System.Text.StringBuilder();
        sb.AppendFormat("  C`tLength: {0}", Things.Length);
        for (var i = 0; i < Things.Length; i++) {
            var t = new PSObject(Things[i]);
            sb.AppendFormat("`titem {0}: {1}", i, t.ToString());
        }
        System.Console.WriteLine(sb.ToString());
    }
}
"@ | Select -First 1 | % { Import-Module -Assembly $_.Assembly }

function Write-ThingF {
    param([Parameter(Position = 0, ValueFromRemainingArguments, Mandatory)][string[]]$Things)
    process {
        $sb = [System.Text.StringBuilder]::new()
        $null = $sb.AppendFormat("  F`tLength: {0}", $Things.Length)
        for ($i = 0; $i -lt $Things.Length; $i++) {
            $null = $sb.AppendFormat("`titem {0}: {1}", $i, [string]$Things[$i])
        }
        [Console]::WriteLine($sb.ToString())
    }
}

trap { [Console]::WriteLine("  error"); continue }

[Console]::WriteLine("Same")
Write-ThingF -Things 1,2
Write-ThingC -Things 1,2

[Console]::WriteLine("Differs")
Write-ThingF 1,2
Write-ThingC 1,2

[Console]::WriteLine("Same")
Write-ThingF 1 2
Write-ThingC 1 2

[Console]::WriteLine("Same")
Write-ThingF -Things 1 2
Write-ThingC -Things 1 2

[Console]::WriteLine("Same")
Write-ThingF -Things 1,2 3,4
Write-ThingC -Things 1,2 3,4

[Console]::WriteLine("Differs")
Write-ThingF 1,2 3,4
Write-ThingC 1,2 3,4

[Console]::WriteLine("Same")
Write-ThingF -Things 1 2 3,4
Write-ThingC -Things 1 2 3,4

[Console]::WriteLine("Differs")
Write-ThingF 1 2 3,4
Write-ThingC 1 2 3,4

and the output

Same
  F     Length: 2       item 0: 1       item 1: 2
  C     Length: 2       item 0: 1       item 1: 2
Differs
  F     Length: 1       item 0: 1 2
  C     Length: 2       item 0: 1       item 1: 2
Same
  F     Length: 2       item 0: 1       item 1: 2
  C     Length: 2       item 0: 1       item 1: 2
Same
  error
  error
Same
  error
  error
Differs
  F     Length: 2       item 0: 1 2     item 1: 3 4
  error
Same
  error
  error
Differs
  F     Length: 3       item 0: 1       item 1: 2       item 2: 3 4
  error

[iSazonov]

@lzybkr thank you for returning back to the origins of the discussion :-)
I expanded your example. It shows that there is no problem if the cmdlet created in the PS language, but the problem is in conjunction Add-Type C# | Import-Module
So if considered "F" and "C1" as expected behavior (this has been discussed previously here and in #2038), the fix should be to fix "C" (Add-Type C# | Import-Module). Do you agree?

Add-Type -PassThru @"
using System.Management.Automation;

[Cmdlet("Write", "ThingC")]
public class WriteThingCommand : PSCmdlet {
    [Parameter(Mandatory = true, Position = 0, ValueFromRemainingArguments = true)] public string[] Things { get; set; }

    override protected void ProcessRecord() {
        var sb = new System.Text.StringBuilder();
        sb.AppendFormat("  C `tLength: {0}", Things.Length);
        for (var i = 0; i < Things.Length; i++) {
            var t = new PSObject(Things[i]);
            sb.AppendFormat("`titem {0}: {1}", i, t.ToString());
        }
        System.Console.WriteLine(sb.ToString());
    }
}
"@ | Select -First 1 | % { Import-Module -Assembly $_.Assembly -Force }

function Write-ThingC1 {
    [CmdletBinding()]
    param([Parameter(Position = 0, ValueFromRemainingArguments, Mandatory)][string[]]$Things)
    process {
        $sb = [System.Text.StringBuilder]::new()
        $null = $sb.AppendFormat("  C1`tLength: {0}", $Things.Length)
        for ($i = 0; $i -lt $Things.Length; $i++) {
            $null = $sb.AppendFormat("`titem {0}: {1}", $i, [string]$Things[$i])
        }
        [Console]::WriteLine($sb.ToString())
    }
}


function Write-ThingF {
    param([Parameter(Position = 0, ValueFromRemainingArguments, Mandatory)][string[]]$Things)
    process {
        $sb = [System.Text.StringBuilder]::new()
        $null = $sb.AppendFormat("  F `tLength: {0}", $Things.Length)
        for ($i = 0; $i -lt $Things.Length; $i++) {
            $null = $sb.AppendFormat("`titem {0}: {1}", $i, [string]$Things[$i])
        }
        [Console]::WriteLine($sb.ToString())
    }
}

trap { [Console]::WriteLine("  error"); continue }

[Console]::WriteLine("Same: 1,2")
Write-ThingF -Things 1,2
Write-ThingC -Things 1,2
Write-ThingC1 -Things 1,2

[Console]::WriteLine("Differs: 1,2")
Write-ThingF 1,2
Write-ThingC 1,2
Write-ThingC1 1,2

[Console]::WriteLine("Same: 1 2")
Write-ThingF 1 2
Write-ThingC 1 2
Write-ThingC1 1 2

[Console]::WriteLine("Same: 1 2")
Write-ThingF -Things 1 2
Write-ThingC -Things 1 2
Write-ThingC1 -Things 1 2

[Console]::WriteLine("Same: 1,2 3,4")
Write-ThingF -Things 1,2 3,4
Write-ThingC -Things 1,2 3,4
Write-ThingC1 -Things 1,2 3,4

[Console]::WriteLine("Differs: 1,2 3,4")
Write-ThingF 1,2 3,4
Write-ThingC 1,2 3,4
Write-ThingC1 1,2 3,4

[Console]::WriteLine("Same: 1 2 3,4")
Write-ThingF -Things 1 2 3,4
Write-ThingC -Things 1 2 3,4
Write-ThingC1 -Things 1 2 3,4

[Console]::WriteLine("Differs: 1 2 3,4")
Write-ThingF 1 2 3,4
Write-ThingC 1 2 3,4
Write-ThingC1 1 2 3,4

and the output:

Same: 1,2
  F     Length: 2   item 0: 1   item 1: 2
  C     Length: 2   item 0: 1   item 1: 2
  C1    Length: 2   item 0: 1   item 1: 2
Differs: 1,2
  F     Length: 1   item 0: 1 2
  C     Length: 2   item 0: 1   item 1: 2
  C1    Length: 1   item 0: 1 2
Same: 1 2
  F     Length: 2   item 0: 1   item 1: 2
  C     Length: 2   item 0: 1   item 1: 2
  C1    Length: 2   item 0: 1   item 1: 2
Same: 1 2
  error
  error
  error
Same: 1,2 3,4
  error
  error
  error
Differs: 1,2 3,4
  F     Length: 2   item 0: 1 2 item 1: 3 4
  error
  C1    Length: 2   item 0: 1 2 item 1: 3 4
Same: 1 2 3,4
  error
  error
  error
Differs: 1 2 3,4
  F     Length: 3   item 0: 1   item 1: 2   item 2: 3 4
  error
  C1    Length: 3   item 0: 1   item 1: 2   item 2: 3 4

[lzybkr]

Your Write-ThingC1 is exactly equivalent to my Write-ThingF - [CmdletBinding()] is implied by the use of the [Parameter()].

To decide which behavior is correct, we need to understand the parameter binding algorithm. You can find the code here.

Roughly speaking, the algorithm is:

1. Bind named arguments
2. Bind positional arguments ignoring the ValueFromRemainingArguments parameter
3. Bind remaining arguments to ValueFromRemainingArguments parameter if any

So binding remaining arguments is a distinct step in the algorithm, expectations regarding binding positional arguments don't _necessarily_ apply.

We should probably consider how C# implements params. In C#, if the only argument starting the params` arguments is an array of the correct type, then C# will not create a new array to wrap the argument, otherwise C# expects 0 or more arguments of the array element type. C# does not convert the arguments like PowerShell would though.

I think this makes intuitive sense, but it gets a little messy in PowerShell because of how freely we convert values including arrays. But lets just assume that PowerShell should work like C#. If there is a single unbound parameter to bind to the ValueFromRemainingArguments and that argument is a collection, it seems like PowerShell should convert the elements of the collection to the expected element type of the parameter.

I realize that's a lot of words and might not be clear, so I'm suggesting that the correct behavior is what happens in the Write-ThingC case above - calling the binary cmdlet.

Note that based on his PR, @dlwyatt came to the same conclusion. His PR looks good to me, so there's just the concern about potential breakage.

[iSazonov]

@lzybkr Thanks for a lot of words! :blush: This is very useful for me!

I suspect that the root of the issue is the C# developer's view. :blush:
What about PHP? Fortran? Fort? PDP11 ASM?
And we need to look exclusively from the perspective of Powershell script writers experience.

In short, the issue is - a,b,c is one item or three?

In terms of Powershell formal syntax this is uniquely a single element.
a,b,c - is a array regardless of where it resides.

From PR #2038 Jeffrey Snover said:

the parser views this as 1 parameter - an array with 3 values

and

That seems like a bad thing because cmdlets with positional parameters are, and need to be, precise.
So a change like this would yield a unpredictable UX.

Thus the issue is a question about cosmetic change.
But what we get as a result? Performance? No. Simplicity? No. Better UX? No.
Powershell scripts writers use the current logic last 10 years as intuitive and "As is".

So we only get breaking change and the collapse of an indefinite number of scripts. Community will damn us :smile:

I can't approve still this change.

[dlwyatt]

In terms of Powershell formal syntax this is uniquely a single element.
a,b,c - is a array regardless of where it resides.

No, it's not, and that's where the confusion lies. If you pass a,b,c to a parameter that's defined as an array type, then the parameter will be an array with 3 elements, _not_ an array containing one element which happens to be another array.

The best example I can give for this is the -ArgumentList parameter of New-Object. It's an [Object[]] type, because New-Object can't know ahead of time what type of arguments the object's constructors might expect. Most of the time, this isn't a problem, _except_ when the constructor expects a single argument that is itself an array. For example, the [ipaddress] class lets you pass in an array of bytes:

# This will fail:

New-Object -TypeName ipaddress -ArgumentList 1,2,3,4

If you want to pass in the 4-element array as a single argument to a constructor, you use the leading unary comma (with some parentheses to make sure the parser knows what's going on):

New-Object -TypeName ipaddress -ArgumentList (,(1,2,3,4))

That's how it has always worked for any parameter that does _not_ include the buggy ValueFromRemainingArguments attribute.

[iSazonov]

I speak from the position of script writer: when I see a,b,c - it is an array. Because the comma is the array constructor. Further there are nuances...

And I agree that the different behavior of func and cmdlet is bad.

And do we have a complete set of tests to understand what will be broken by the change?

[SteveL-MSFT]

@PowerShell/powershell-committee reviewed this and agree having consistency in script and c# is the right thing as well getting an array instead of a string

[joeyaiello]

@PowerShell/powershell-maintainers given that this is a breaking change that we want to take, we should try to figure out what it takes to get @dlwyatt 's PR at #2038 merged. I know it's very stale at this point, but it would be useful, and we should deal with it or decide we're not going to take it.

@dlwyatt: if you're not up for doing a rebase against the next 9 months of code, please check the "Allow edits from maintainers" box on the PR (might need to close and reopen or something like that), so that we can come and give it a shot (I might end up doing this myself, but I think it would be a useful exercise).

[dlwyatt]

No problem, I'll rebase.

[dlwyatt]

What a non-event that was. No merge conflicts. :)

[mklement0]

I am very late to this party, but let me offer the following perspective:

• I think everyone agrees that it is important that both cmdlets and advanced functions exhibit the same behavior.

• I think that it is the _advanced functions_' current behavior that should be standardized.

The ValueFromRemainingArguments attribute is an _anomaly_ in the realm of PowerShell parameter parsing:
As the _plural_ in the name suggests, it _implicitly_ pertains to _multiple_ arguments at once - unlike any other parameter.

Therefore, the following dichotomy makes sense to me:

• If you opt into _regular_ PowerShell parsing - by way of using an _explicit parameter name_ on invocation, you _must_ use an _array_ to bind all remaining (not bound by regular parameters) arguments.
  (The fact that such arguments are currently collected as [List[object]] instances rather than as regular arrays is an - unfortunate - implementation detail that should be reconsidered - see #4625).

• Otherwise, _by omission of the parameter name_, you _implicitly opt into the anomaly_, which means that _multiple, separate_ arguments _implicitly_ bind to such a parameter as collection elements - whether these individual arguments happen to be arrays themselves or not.

  • Remember that an important use of ValueFromRemainingArguments is to enable scripts that are called from _outside_ of PowerShell (via -File) to receive _arrays_ - which cannot be done any other way and _requires_ that the parameter name be _omitted_.

In other words, the following equivalence make sense to me:

# Define a function with an unconstrained ValueFromRemainingArguments attribute
# that outputs the count of arguments bound.
Function Foo { param([Parameter(ValueFromRemainingArguments)] $Rest) $Rest.Count }

• Pass multiple arguments:

# Use positional arguments that are *individually* bound to $Rest.
> Foo a b
2

# Do the same by using -Rest explicitly, which requires *array syntax*.
> Foo -Rest a, b
2

• Pass an array as a single argument:

# Pass an array as the 1st and only argument.
> Foo a, b
1

# Do the same via -Rest, which requires a *nested array*:
> Foo -Rest (, ('a', 'b'))
1

[mklement0]

Note that _one_ side of the dichotomy is already enforced consistently (both in advanced functions and compiled cmdlets):

• If you use the parameter _name_, you must not specify _multiple_ arguments:

# Cmdlet:
# Fails, because only 'a' binds to -AdditionalChildPath, 
# leaving 'b' as an unrecognized extra positional argument.
> Join-Path parent child -AdditionalChildPath a b
Join-Path : A positional parameter cannot be found that accepts argument 'b'
...

# Advanced function: ditto.
> Function Foo { param([Parameter(ValueFromRemainingArguments)] $Rest) $Rest.Count }; Foo -Rest a b
Foo : A positional parameter cannot be found that accepts argument 'b'.
...

To offer a concise summary of behavior that I think makes sense, is conceptually simple, and therefore easy to remember for users:

• If you _omit the parameter name_, you may - and must - pass _multiple, individual arguments_.
  (If any of these individual arguments happens to be an array, it will bind to _one_ element of the collection assigned to the parameter.)

• If you _specify the parameter name_, you must pass the arguments _as an array_.

This is how advanced functions already work.
Now let's make cmdlets work the same way.

[daxian-dbw]

Close vis PR #2038