Skip to content

Commit

Permalink
Shift dynamic positional parameters, fix #217
Browse files Browse the repository at this point in the history
  • Loading branch information
nightroman committed Mar 4, 2024
1 parent f8502cc commit b273813
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 51 deletions.
14 changes: 9 additions & 5 deletions Invoke-Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ if ($MyInvocation.InvocationName -eq '.') {return}
trap {*Die $_ 5}

$p = if ($_ = $PSCmdlet.SessionState.PSVariable.Get('*')) {if ($_.Description -eq 'IB') {$_.Value}}
$c, $r = $null
$c, $r, $a = $null
New-Variable * -Description IB ([PSCustomObject]@{
All = [System.Collections.Specialized.OrderedDictionary]([System.StringComparer]::OrdinalIgnoreCase)
Tasks = [System.Collections.Generic.List[object]]@()
Expand Down Expand Up @@ -116,10 +116,14 @@ if ($_.get_Count()) {
$c = 'Verbose', 'Debug', 'ErrorAction', 'WarningAction', 'ErrorVariable', 'WarningVariable', 'OutVariable', 'OutBuffer', 'PipelineVariable', 'InformationAction', 'InformationVariable', 'ProgressAction'
$r = 'Task', 'File', 'Result', 'Safe', 'Summary', 'WhatIf'
foreach($p in $_.get_Values()) {
if ($c -notcontains ($_ = $p.Name)) {
if ($r -contains $_) {throw "Script uses reserved parameter '$_'."}
${*}.DP.Add($_, (New-Object System.Management.Automation.RuntimeDefinedParameter $_, $p.ParameterType, $p.Attributes))
if ($c -contains ($_ = $p.Name)) {continue}
if ($r -contains $_) {throw "Script uses reserved parameter '$_'."}
foreach ($a in $p.Attributes) {
if ($a -is [System.Management.Automation.ParameterAttribute] -and $a.Position -ge 0) {
$a.Position += 2
}
}
${*}.DP.Add($_, (New-Object System.Management.Automation.RuntimeDefinedParameter $_, $p.ParameterType, $p.Attributes))
}
${*}.DP
}
Expand Down Expand Up @@ -663,7 +667,7 @@ foreach($_ in ${*}.DP.get_Values()) {
if (${*}.Q = $BuildTask -eq '?' -or $BuildTask -eq '??') {
$WhatIf = $true
}
Remove-Variable Task, File, Result, Safe, Summary, p, c, r
Remove-Variable Task, File, Result, Safe, Summary, p, c, r, a

${*}.Error = $null
try {
Expand Down
2 changes: 2 additions & 0 deletions Release-Notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## v5.10.6

Fix cryptic errors on unknown parameters, #217

Minor tweaks, new demo Tasks/Repeat2, etc.

`Show-BuildGraph.ps1`
Expand Down
46 changes: 0 additions & 46 deletions Tests/Fixed.test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -490,52 +490,6 @@ task PreserveCheckpoint {
Remove-Item z.build.ps1
}

# Issue #152 (1). Also test the bootstrapping scenario.
# This used to fail on the second call of z.ps1.
task DoNotMakeScriptParametersNamed {
# self-invoking build script
Set-Content z.ps1 {
param(
[Parameter()]$Tasks
)
if (!$MyInvocation.ScriptName.EndsWith('Invoke-Build.ps1')) {
return Invoke-Build $Tasks $MyInvocation.MyCommand.Path @PSBoundParameters
}
task Test {}
}

# invoke the script twice with the task name
./z.ps1 Test
./z.ps1 Test

remove z.ps1
}

<#
Issue #152 (2). Test positional parameters.
Potential problem:
- `Task` and `P1` both have position 0.
- `File` and `P2` both have position 1.
Fortunate current behaviour:
PowerShell somehow does what we expect, shift positions of P1, P2, P3.
This may change in the future, so let at least cover this by test.
Workarounds for the future:
- Use `[CmdletBinding(PositionalBinding=$false)]` (PowerShell v3+) to enforce named parameters.
- Set parameter positions explicitly starting with 2 (0, 1 are consumed by Task, File).
#>
task PositionalParameters {
Set-Content z.build.ps1 {
param($P1, $P2, $P3)
task Parameters {"$P1|$P2|$P3"}
}

# invoke the script with 5 positional parameters
($r = Invoke-Build Parameters z.build.ps1 v1 v2 v3)
assert ($r -contains 'v1|v2|v3')

remove z.build.ps1
}

# Issue #183, introduce $OriginalLocation, where the build starts.
task OriginalLocation {
Set-Location $HOME
Expand Down
49 changes: 49 additions & 0 deletions Tests/Issues/152-positional-script-parameters/152.test.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

# Issue #152 (1). Also test the bootstrapping scenario.
# This used to fail on the second call of z.ps1.
task DoNotMakeScriptParametersNamed {
# self-invoking build script
Set-Content z.ps1 {
param(
[Parameter()]$Tasks
)
if (!$MyInvocation.ScriptName.EndsWith('Invoke-Build.ps1')) {
return Invoke-Build $Tasks $MyInvocation.MyCommand.Path @PSBoundParameters
}
task Test {}
}

# invoke the script twice with the task name
./z.ps1 Test
./z.ps1 Test

remove z.ps1
}

<#
Issue #152 (2). Test positional parameters.
Potential problem:
- `Task` and `P1` both have position 0.
- `File` and `P2` both have position 1.
UPDATE: yes, it is a problem, see #217
Fortunate current behaviour:
PowerShell somehow does what we expect, shift positions of P1, P2, P3.
This may change in the future, so we cover this by test.
UPDATE: #217 now shifts positions but we keep the test anyway.
Workarounds for the future:
- Use `[CmdletBinding(PositionalBinding=$false)]` (PowerShell v3+) to enforce named parameters.
- Set parameter positions explicitly starting with 2 (0, 1 are consumed by Task, File).
UPDATE: The second is kind of used by #217 (shift +2)
#>
task PositionalParameters {
Set-Content z.build.ps1 {
param($P1, $P2, $P3)
task Parameters {"$P1|$P2|$P3"}
}

# invoke the script with 5 positional parameters
($r = Invoke-Build Parameters z.build.ps1 v1 v2 v3)
assert ($r -contains 'v1|v2|v3')

remove z.build.ps1
}
12 changes: 12 additions & 0 deletions Tests/Issues/217-bad-error-on-unknown-parameter/217.build.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<# https://github.com/nightroman/Invoke-Build/issues/217
#152 fixed one issue but added another, cryptic errors.
#217 shifts script parameters positions (+2) avoiding conflicts with IB Task and File.
#>

param(
$Param1,
$Param2,
$Param3
)

task .
32 changes: 32 additions & 0 deletions Tests/Issues/217-bad-error-on-unknown-parameter/217.test.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@

task UnknownParameter {
# parameter positions before IB
$a1, $a2, $a3 = Get-ParameterPosition
equals $a2 ($a1 + 1)
equals $a3 ($a2 + 1)

# call IB with an unknown parameter, this should fail
# and the error should be friendly, not some cryptic
try {
throw Invoke-Build -bar
}
catch {
equals "$_" "A parameter cannot be found that matches parameter name 'bar'."
}

# parameter positions after IB shifted +2 on every call
$b1, $b2, $b3 = Get-ParameterPosition
equals $b1 ($a1 + 2)
equals $b2 ($a2 + 2)
equals $b3 ($a3 + 2)
}

function Get-ParameterPosition {
foreach($p in (Get-Command "$BuildRoot\217.build.ps1").Parameters.Values) {
foreach ($a in $p.Attributes) {
if ($a -is [System.Management.Automation.ParameterAttribute]) {
$a.Position
}
}
}
}

0 comments on commit b273813

Please sign in to comment.