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

[suggested trap] should not use ValueFromPipeline with more than one param #9

Closed
jazzdelightsme opened this issue Apr 22, 2019 · 7 comments

Comments

@jazzdelightsme
Copy link

I didn't notice an existing trap covering pipeline input getting bound to multiple parameters. Repro:

function foo
{
    [CmdletBinding()]
    param( [Parameter( Mandatory = $false,
                       Position = 0,
                       ValueFromPipeline = $true )]
           [string[]] $P1,

           [Parameter( Mandatory = $false,
                       Position = 1,
                       ValueFromPipeline = $true )]
           [string[]] $P2
         )

    process
    {
        try
        {
            Write-Host "`$P1: $P1" -Fore Magenta
            Write-Host "`$P2: $P2" -Fore Magenta
        }
        finally { }
    }
}

'asdf' | foo

# Expected: input gets bound to one parameter...
# Actual: ...but it gets bound to /both/

# I guess the lesson is that you should never have ValueFromPipeline on more
# than one parameter in a given parameter set, but PS lets you do that.
@nightroman
Copy link
Owner

nightroman commented Apr 22, 2019

Thank you for the interesting case, I did not know about such a feature.
I would not call this a trap though... PowerShell works in the expected way,
i.e. exactly as the code is written.

This may be even useful in some scenarios. Nothing practical comes to my mind
yet but here is some contrived and weird but "maybe useful example":

# Both commands output:
# $P1: v1
# $P2: v2

'v2' | foo v1
'v1' | foo -p2 v2

In other words, I can choose what to pipe, P1 or P2.
It's kind of useful that PowerShell makes this possible.

@jazzdelightsme
Copy link
Author

My mental model of parameter binding includes the concept of "conservation of parameters" (like conservation of mass/energy in the physical world). One input "thing" gets bound to one parameter. I cannot think of any other binding scenario (I mean I think the only other possibilities are "positional" and "named") where a single input object gets "duplicated" to more than one parameter.

For example, consider positional binding. Suppose foo were declared like this instead:

function foo
{
    [CmdletBinding()]
    param( [Parameter( Mandatory = $false, Position = 0 )]
           [string] $P1,

           [Parameter( Mandatory = $false, Position = 0 )] # NOTE: same position!
           [string] $P2
         )

    process
    {
        try
        {
            Write-Host "`$P1: $P1" -Fore Magenta
            Write-Host "`$P2: $P2" -Fore Magenta
        }
        finally { }
    }
}

If foo 'asdf' were bound via your "exactly as written" rubric, then parameter binding should succeed, and both $P1 and $P2 should have the value 'v1'. But of course that's not what happens: when PS sees two parameters that both can be bound at position 0, in the same parameter set, it gives up (it does not even try to use types to try to break the tie, even). So why should pipeline binding work differently?

You could come up with an analogous situation for named binding, even: suppose you tried to write foo -P 'asdf': The parameter name is ambiguous (could match -P1 or -P2): so does PS assign 'asdf' to both? No, it fails binding.

In short, while one could argue that this is simply the design of pipeline binding, I found it unexpected, and therefore considered it a candidate for a "trap", but I'm not sure what rubric you use to decide if something qualifies.

@nightroman
Copy link
Owner

I like the case and I do not mind to add it to the collection.
But I am trying to understand the nature of the "trap".

I do not understand why # Expected: input gets bound to one parameter....
Do we expect PowerShell to ignore the second ValueFromPipeline = $true?
Should not we rather expect it to fail like in the second example?

@nightroman
Copy link
Owner

I'm not sure what rubric you use to decide if something qualifies [as a trap].

Something like sane code that does not work as expected.

@MikeShepard
Copy link

MikeShepard commented Apr 23, 2019 via email

@jazzdelightsme
Copy link
Author

Should not we rather expect it to fail like in the second example?

You're right; if it were to work as the other binding types, it should fail.

@nightroman
Copy link
Owner

I added the case to the collection, see ValueFromPipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants