PSSA should have a new rule to check for properly used Process blocks

For some cases the formatter is changing code where it cannot run. Rules that trigger this are:

PSUseConsistentWhitespace.CheckParameter when $True
PSAvoidUsingCmdletAliases when $True

The source script triggering this bug is not valid, however

the PSAvoidUsingCmdletAliases: pwsh will execute this with no errors (It runs normally, it never assigns $x)
the CheckParameter: This code is broken, pwsh will not execute this

Possible Cause of `PSAvoidUsingCmdletAliases`

From the docs: If your function defines a Begin, Process or End block, all of your code must reside inside those blocks. No code will be recognized outside the blocks if any of the blocks are defined.

I think the cause is the script reaches a statement that is not inside a begin/process/end block -- so it assumes the function is a non-pipeline function. In that context, process is not a keyword but an identifier. It replaces process with Get-Process (which would the correct behavior if it was a normal function)

Invoke-ScriptAnalyzer has the same behavior.

Potential fix for `PSAvoidUsingCmdletAliases` ?

PSSA should have a rule to check for functions that have code outside begin/process/end blocks.
Can the parser test if there are any begin/process/end statements, then treat it as a pipeline function?
Can the formatter abort if errors occur? (Right now there's no errors on mutation)

Steps to reproduce

$script = @'
function Do-Stuff {
$x = 2
Process { }
}
'@
$scriptFixed = @'
function Do-Stuff {
#$x = 2
Process { }
}
'@

$settings = @{
    IncludeRules = @('PSUseCorrectCasing', 'PSUseConsistentIndentation')
    Rules        = @{
        PSAvoidUsingCmdletAliases = @{
            Enable = $false
        }
        PSUseConsistentIndentation = @{
            Enable          = $true
            Kind            = 'space'
            PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
            IndentationSize = 4
        }

    }
}

Invoke-Formatter -ScriptDefinition $script -Settings $settings
Invoke-Formatter -ScriptDefinition $scriptFixed -Settings $settings

Expected behavior

Indent and replace aliases.

function Do-Stuff {
    $x = 2
    Process { }
}

function Do-Stuff {
    #$x = 2
    Process { }
}

Actual behavior

Indent, and replaces keyword as if it was not in the keyword context.

function Do-Stuff {
    $x = 2
    Get-Process { }
}

function Do-Stuff {
    #$x = 2
    Process { }
}

Another example not using pipeline functions

Steps to reproduce

$Original = @'
Function BadCode {
    "stuff" | Format-CodeColor" 'ps1'

    $InputList | ForEach-Object {
    } | Select-Object -First 2
    | Join-String -sep ", " -OutputPrefix 'Results: '
}
'@

$settings = @{
    IncludeRules = @(
        "PSUseConsistentWhitespace"
    )
    Rules        = @{
        PSUseConsistentWhitespace = @{
            Enable         = $True
            CheckParameter = $false
        }
    }
}

$out1 = Invoke-Formatter -ScriptDefinition $Original -Settings $settings

$settings.rules.PSUseConsistentWhitespace.CheckParameter = $True
$out2 = Invoke-Formatter -ScriptDefinition $Original -Settings $settings

$out1
'+' * 30
$out2

if ($out1 -ne $out2) {
    Write-Error 'formatting does not match'
}

Expected behavior 2

Give an error, or don't mutate code

Actual behavior 2

Function BadCode {
    "`nPS> Top | Bot | Se" -OutputPrefix 'Results: '
}

Environment data

>> $PSVersionTable | ft

Name                           Value
----                           -----
PSVersion                      7.0.3
PSEdition                      Core
GitCommitId                    7.0.3
OS                             Microsoft Windows 10.0.19041
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

>> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.19.1

>> $psEditor | select EditorServicesVersion

EditorServicesVersion
---------------------
2.3.0.0

Meta: Should I have broken this into two issues? (I submitted on the wrong account)

Indent, and replaces keyword as if it was not in the keyword context.

This is behaving as expected.

PSAvoidUsingCmdletAliases also warns about and replaces implicit Get-prefix aliasing.

In the example you give, process is not parsed as a keyword. If you invoke your function, it will run the Get-Process command. PSScriptAnalyzer is doing its best to tell you this.

Importantly, the PowerShell parser (the same one that parses your script on execution) is what makes this distinction before PSScriptAnalyzer does any processing. By the time the script gets to PSScriptAnalyzer, there's a large structural distinction between a process block and a process keyword:

PS> {
>> process { }
>> }.Ast

Attributes         : {}
UsingStatements    : {}
ParamBlock         :
BeginBlock         :
ProcessBlock       : process { }
EndBlock           :
DynamicParamBlock  :
ScriptRequirements :
Extent             : {
                     process { }
                     }
Parent             : {
                     process { }
                     }

PS> {
>> write-host 'Hi'
>> process { }
>> }.Ast

Attributes         : {}
UsingStatements    : {}
ParamBlock         :
BeginBlock         :
ProcessBlock       :
EndBlock           : write-host 'Hi'
                     process { }
DynamicParamBlock  :
ScriptRequirements :
Extent             : {
                     write-host 'Hi'
                     process { }
                     }
Parent             : {
                     write-host 'Hi'
                     process { }
                     }

In the first case, a ScriptBlockAst object is produced with a ProcessBlock parameter populated. In the second case, ProcessBlock is null and EndBlock is now populated. So the structure of the AST has removed any ambiguity by the time PSScriptAnalyzer sees your script.

You can see this in the absence of PSScriptAnalyzer by trying to define a function like this directly in PowerShell. When process is used after other statements, it's parsed as a command name within the implicit end block (in a function), so you see a runtime error message here from Get-Process:

PS> function Test
>> {
>>     Write-Host "Hi"
>>     process { "Hi" }
>> }
PS> test
Hi
Get-Process:
Line |
   4 |      process { "Hi" }
     |              ~~~~~~~~
     | Cannot evaluate parameter 'Name' because its argument is specified as a script block and there is no input. A script block cannot be evaluated without input.

When process comes first, it defines a process block, and the parser will error if naked statements also occur outside of blocks:

PS> function Test
>> {
>>     process { "Hi" }
>>     write-host "Hi"
>> }
ParserError:
Line |
   4 |      write-host "Hi"
     |      ~~~~~~~~~~
     | unexpected token 'write-host', expected 'begin', 'process', 'end', or 'dynamicparam'.

So the formatting here has not mutated your code to change its meaning; it still does the same thing as the original script you wrote.

A remaining question is whether it's worth investing in a rule to warn about using commands that shadow keywords like this.

Rob Holt (@rjmholt)

Rob Holt (@rjmholt) This was resolved in #1638 . Do I need to do anything to close my ticket?

Navigation Menu

Repository navigation

PSSA should have a new rule to check for properly used Process blocks #1571

Possible Cause of `PSAvoidUsingCmdletAliases`

Potential fix for `PSAvoidUsingCmdletAliases` ?

Steps to reproduce

Expected behavior

Actual behavior

Another example not using pipeline functions

Steps to reproduce

Expected behavior 2

Actual behavior 2

Environment data

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

PSSA should have a new rule to check for properly used Process blocks #1571

Description

Possible Cause of PSAvoidUsingCmdletAliases

Potential fix for PSAvoidUsingCmdletAliases ?

Steps to reproduce

Expected behavior

Actual behavior

Another example not using pipeline functions

Steps to reproduce

Expected behavior 2

Actual behavior 2

Environment data

Activity

ninmonkey commented on Aug 17, 2020

rjmholt commented on Aug 18, 2020

ninmonkey commented on Sep 5, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Possible Cause of `PSAvoidUsingCmdletAliases`

Potential fix for `PSAvoidUsingCmdletAliases` ?