8000 CLI: -File arguments don't recognize [bool] parameter values when passed as separate arguments · Issue #10838 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

CLI: -File arguments don't recognize [bool] parameter values when passed as separate arguments #10838

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.

Sign up for GitHub

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

Jump to bottom
Closed
mklement0 opened this issue Oct 18, 2019 · 15 comments
Closed

CLI: -File arguments don't recognize [bool] parameter values when passed as separate arguments #10838

mklement0 opened this issue Oct 18, 2019 · 15 comments
Labels
Hacktoberfest Potential candidate to participate in Hacktoberfest Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a Resolution-No Activity Issue has had no activity for 6 months or more Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@mklement0
Copy link
Contributor
mklement0 commented Oct 18, 2019 •
edited
Loading

In response to #4036, passing Booleans to [switch] parameters was fixed.

However, the problem persists for [bool] parameters if you pass the value as a separate argument (-param value), which is the typical syntax; by contrast, -param:value (the form that you must use with [switch] parameters) works fine (but using : and whitespace - -param: value - is also broken).

While [bool] parameters are rare, there's no reason for them not to be supported.

Additionally, the -param: value syntax form (: separator and whitespace) also doesn't work with [switch] parameters, only -param:value does (:, but no whitespace).

Also, given that [bool] parameters (but not [switch] parameters) also accepts numbers inside PowerShell - notably 1 for $true and 0 for $false (e.g., & { param([bool] $p) $p } 1 yields $true) - passing numbers should be supported too.

Steps to reproduce

Describe "-File with Booleans" {
  BeforeAll {
    Push-Location testdrive:\
    'param([bool] $foo) $foo' > testB.ps1
    'param([switch] $foo) $foo.IsPresent' > testS.ps1
  }

  # [switch] parameter

  It "[switch] param: `$true can be passed with ':' and *no* whitespace" {
    pwsh -noprofile -file ./testS.ps1 -foo:`$true | Should -Be 'True'
  }
  It "[switch] param: `$true can be passed with ':' *with* whitespace" {
    # Note the escaped $, so that '$true' is passed as a string.
    # However, it should work even without escaping, in which case 'True' is passed.
    pwsh -noprofile -file ./testS.ps1 -foo: `$true | Should -Be 'True'
  }


  # [bool] parameter

  It "[bool] param: `$true can be passed with ':' and *no* whitespace" {
    # Note the escaped $, so that '$true' is passed as a string.
    # However, it should work even without escaping, in which case 'True' is passed.
    pwsh -noprofile -file ./testB.ps1 -foo:`$true | Should -Be 'True'
  }

  It "[bool] param: `$true can be passed with ':' *with* whitespace" {
    pwsh -noprofile -file ./testB.ps1 -foo: `$true | Should -Be 'True'
  }

  # A [bool] parameter - unlike [switch] - *requires* an explicit argument, 
  # so you should be able to pass it with the customary value-as-separate
  # argument syntax.
  It "[bool] param: `$true can be passed as a *separate argument*" {
    pwsh -noprofile -file ./testB.ps1 -foo `$true | Should -Be 'True'
  }

  # A [bool] parameter - unlike [switch] - also supports *numbers* as arguments
  # where 0 is coerced to $false and 1 (any nonzero number) to $true
  It "[bool] param: 1 can be passed for $true" {
    pwsh -noprofile -file ./testB.ps1 -foo:1 | Should -Be 'True'
  }
  It "[bool] param: 0 can be passed for $false" {
    pwsh -noprofile -file ./testB.ps1 -foo:0 | Should -Be 'False'
  }

  AfterAll {
    Pop-Location
  }
}

Expected behavior

The test should pass.

Actual behavior

All tests except the ones with syntax form -foo:$true fail:

Expected $true, but got @('.../test.ps1 : Cannot process argument transformation on parameter 'foo'. 
Cannot convert value "System.String" to type "System.Boolean". Boolean parameters accept only Boolean values and numbers, such as $True, $False, 1 or 0.

Environment data

PowerShell Core 7.0.0-preview.4
@mklement0 mklement0 added the Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a label Oct 18, 2019
@mklement0 mklement0 changed the title CLI: -File arguments don't recognize [bool] parameters CLI: -File arguments don't recognize [bool] parameter values as separate arguments Oct 19, 2019
@mklement0 mklement0 changed the title CLI: -File arguments don't recognize [bool] parameter values as separate arguments CLI: -File arguments don't recognize [bool] parameter values when passed as separate arguments Oct 19, 2019
@iSazonov
Copy link
Collaborator

Should we escape $ in $true?

@mklement0
Copy link
Contributor Author

@iSazonov

Good point, but it makes no difference here, because the target pwsh instance should recognized both literal strings $true and True, the latter being what is passed without escaping.
For clarity I've added escaping and a comment to the OP.

Note that the tests do succeed - both with literal $true and True - if you use the -param:value syntax (: as separator, no whitespace):

# OK: literal '$true'
pwsh -noprofile -file ./test.ps1 -foo:`$true

# OK: literal 'True'
pwsh -noprofile -file ./test.ps1 -foo:$true

@iSazonov
Copy link
Collaborator

It seems the same as:

> [bool]'false'
True

@mklement0
Copy link
Contributor Author

No, it's not the same: we're talking about the CLI here, where all arguments are of necessity initially strings, and you need some way to pass a Boolean when you use -File.

The inability to pass a Boolean with -File - still a problem in Windows PowerShell - led to the fix in #4036, but that fix was incomplete, which is what this issue is about.

@iSazonov
Copy link
Collaborator

I see two way:

  • enhance a binder so that for -File option it tries to convert string arguments to parameter types
  • enhance a workaround for switch so that always convert $true and $false literals to boolean.

@mklement0
Copy link
Contributor Author
mklement0 commented Oct 20, 2019 •
edited
Loading

We don't need new parameter-parsing functionality, we just need to amend (fix) a previous fix:

That fix was to allow literal strings $true, true, $false, false to be converted to [bool] when bound to [switch] or - less commonly - [bool] parameters when the CLI is used with -File.

This now works in principle, except in particular syntax forms, which should obviously not be treated any different than its equivalent forms.

As stated, -foo:$false works, but neither -foo $false nor -foo: $false do - even though these are just syntax variations.

Note that -foo $false - whitespace-only separator - only applies to [bool] parameters, because with [switch] parameters you must use : (given that switch parameters never treat a separate argument as their value).

@SteveL-MSFT, you implemented the original fix in #4178 - does this make sense to you?

@iSazonov
Copy link
Collaborator

We don't need new parameter-parsing functionality,

If somebody run ps1 file in pwsh session and then do the same in command line (copy-paster) different behavior may surprise the user.

@mklement0
Copy link
Contributor Author
mklement0 commented Oct 20, 2019 •
edited
Loading

It may, but it shouldn't:

If you want the same behavior as inside PowerShell, invoke the CLI with -Command rather than -File.

It was a deliberate design decision to treat all -File arguments as literal strings, initially, before binding to the target script's parameters.

This doesn't preclude later conversions when binding to the script's parameters, such as from string to [int], but it does preclude passing arguments such as @{foo=1} and (1+2) and expecting them to be evaluated up front, as expressions as they would inside PowerShell.

If you disagree with this design, I suggest you create a new issue.

An exception to the design was deliberately put in place to allow binding strings $true, true, $false, false to [switch] and [bool] parameters when binding calling via -File (something that doesn't work inside PowerShell - try & { param([bool] $p) } '$true'), as it would otherwise be fundamentally impossible to pass a Boolean via -File.

The issue at hand is that this exception wasn't implemented comprehensively.

@mklement0
Copy link
Contributor Author

However, there is one problematic aspect about how the exception was implemented, @SteveL-MSFT, though it may not matter much in practice:

The Boolean-like strings are unconditionally converted to [bool] - even if the parameter they bind to is untyped:

The right behavior is to retain them as strings in that event, just like a "number string" such as 1 remains a string when bound to an untyped parameter via -File.

@iSazonov
Copy link
Collaborator

Conditional conversion means a binder enhancement which is likely unjustified complexity.

@iSazonov iSazonov added the WG-Engine core PowerShell engine, interpreter, and runtime label Oct 21, 2019
@SteveL-MSFT
Copy link
Member

@mklement0 seems like this is becoming a slippery slope. At some point, it seems that using -Command is a good enough workaround.

@mklement0
Copy link
Contributor Author

I'm personally fine with leaving the unconditional conversion mentioned in my previous comment be, but I don't think there's anything slippery about the issue as original reported:

Supporting a feature in only one syntax form, but not in others that should always result in the same behavior is a baffling inconsistency that is well worth fixing.

@BrucePay
Copy link
Collaborator

@mklement0 Passing commands to an executable - any executable - requires that the arguments be passed as literal strings because that's what the operating systems support. All type information is lost so trying to get the exact semantics of calling a PowerShell function from within PowerShell is tilting at windmills. As @SteveL-MSFT says, after a while you have to accept good enough. If there's something that just doesn't work, then it needs to get fixed (encoding scriptblocks, Steve's fix) but otherwise it just leads to playing wack-a-mole with bugs.

@mklement0
Copy link
Contributor Author

@BrucePay: To succinctly recap my previous comment:

All that is needed is to fix the incomplete fix that was implemented in #4178, as detailed in the OP.

@SteveL-MSFT SteveL-MSFT added Hacktoberfest Potential candidate to participate in Hacktoberfest Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors labels Oct 21, 2019
@mklement0 mklement0 mentioned this issue Mar 1, 2020
Closed
@iSazonov iSazonov mentioned this issue Mar 19, 2020
Merged
@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-No Activity Issue has had no activity for 6 months or more labels Nov 16, 2023
@adaviding
Copy link
adaviding commented Oct 17, 2024 •
edited
Loading

This is still an important issue. I hit it today. It still has not been addressed. I request that it be re-opened.

Using powershell 7.4.5 from bash prompt on Ubuntu 22.04:

pwsh script.ps1 false       # does not work, but it should
pwsh script.ps1 -foo false  # does not work, but it should
pwsh script.ps1 -foo:false  # works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Hacktoberfest Potential candidate to participate in Hacktoberfest Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a Resolution-No Activity Issue has had no activity for 6 months or more Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mklement0 @adaviding @BrucePay @SteveL-MSFT @iSazonov
0