Repository navigation
Parameter binding problem with ValueFromRemainingArguments in PS functions #2035
Description
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,bbExpected 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,bbEnvironment 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
Activity
dlwyatt commentedon Aug 23, 2016
Here are the ParameterBinding traces from each scenario:
Function:
Cmdlet:
What's interesting to me is this bit from the cmdlet trace:
If I'm understanding this correctly, it looks like the binder is receiving a 1-element List, and the element happens to be a 2-element Object[] in this case. It tries (and fails, in the case of the cmdlet binding) to convert the List directly to a String[], but fails because the inner array (Object[]]) couldn't be directly converted to a String.
When we're doing a function's binding, the ArgumentTypeConverterAttribute gets involved, and it has no problem "casting" anything to a string. In this case, the string representation of the array @('aa','bb) is the string 'aa bb' (assuming a default value for the $OFS variable.)
In both cases, the array we actually passed as an argument to the command was wrapped in a single-element List, and the binder tried to do conversions on that List first, instead of acting directly on the values that were passed to the command. It's just that the ArgumentTypeConverterAttribute was successfully able to convert the List, and the code path that was taken for the cmdlet's binding was not, so it fell through to converting the list's element instead.
dlwyatt commentedon Aug 23, 2016
I haven't stepped through the code yet, but it seems logical that this List is what's being built up any time a ValueFromRemainingArguments parameter is in play. That would be logical, and I would expect it to be a 2-element List that contains strings if we had passed the values as separate arguments instead of a single array.
If that is what's happening, then what seems to be missing is some logic to check, when ValueFromRemainingArguments is in play, whether the List contains only a single element. If so, perhaps the list should be unwrapped before the parameter binder starts trying to do type conversion.
dlwyatt commentedon Aug 23, 2016
That does appear to be the case. Here's a trace of binding for the cmdlet with the array split into two string arguments:
dlwyatt commentedon Aug 23, 2016
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 commentedon Aug 24, 2016
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 commentedon Aug 24, 2016
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 ofdir "c:\"all the time. :)MarvTheRobot commentedon Aug 24, 2016
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 ofdir "c:\"all the time. :)You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#2035 (comment)
jpsnover commentedon Aug 28, 2016
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 commentedon Aug 28, 2016
Jeffrey Snover (@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 commentedon Aug 28, 2016
The current parameter binder is working as I expect too.
You can get the desired output by using splatting:
That said, it might be nice to have a convenient syntax for splatting unnamed/positional arguments that took an array like:
But to make it really convenient it should work on an array literal e.g.
Not exactly sure what the syntax should be since today this doesn't work with hashtable literals either.
dlwyatt commentedon Aug 28, 2016
I don't see how this can be expected behavior. Going back to the original example, giving the function a name this time:
Both of those calls should have identical values for the
$Extravariable inside the function.-Extrais the last positional parameter defined, and has theValueFromRemainingArgumentsattribute. 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 byValueFromRemainingArgumentsinstead.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 commentedon Aug 28, 2016
Let's start with the case where there is no
ValueFromRemainingArguments:That results in:
That was your initial expected output. But this doesn't accept extra args:
Do-Something Root Extra1 Extra2 Extra3. So you add inValueFromRemainingArgumentsto handle that case and voila, that case works:Unfortunately the original array case behavior has changed.
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:
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?
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 commentedon Aug 28, 2016
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')31 remaining items
joeyaiello commentedon May 25, 2017
@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 Dave Wyatt (@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.
Dave Wyatt (@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 commentedon May 25, 2017
No problem, I'll rebase.
dlwyatt commentedon May 25, 2017
What a non-event that was. No merge conflicts. :)
mklement0 commentedon Aug 21, 2017
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
ValueFromRemainingArgumentsattribute 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 A ValueFromRemainingArguments parameter by default invariably receives a [List[object]] collection rather than a scalar / PowerShell array ([object[]]), as appropriate #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.
ValueFromRemainingArgumentsis 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:
mklement0 commentedon Aug 21, 2017
Note that one side of the dichotomy is already enforced consistently (both in advanced functions and compiled cmdlets):
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.
Fixes PowerShell#2035
daxian-dbw commentedon Oct 13, 2017
Close vis PR #2038
Fix accidental wrapping of scalar Write-Output input