Pester PR Build Pipeline should check for PsScriptAnalyzer errors

[asears]

1. General summary of the issue

Running PSScriptAnalyzer against the repo throws the following warnings.

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseDeclaredVarsMoreThanAssignment Warning      test.ps1   14    The variable 'ErrorView' is assigned but never used.
s
PSUseDeclaredVarsMoreThanAssignment Warning      test.ps1   63    The variable 'PesterPreference' is assigned but never used.
s
PSAvoidUsingWriteHost               Warning      test.ps1   40    File 'test.ps1' uses Write-Host. Avoid using Write-Host
                                                                  because it might not work in all hosts, does not work when
                                                                  there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      test.ps1   42    File 'test.ps1' uses Write-Host. Avoid using Write-Host
                                                                  because it might not work in all hosts, does not work when
                                                                  there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      test.ps1   53    File 'test.ps1' uses Write-Host. Avoid using Write-Host
                                                                  because it might not work in all hosts, does not work when
                                                                  there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingCmdletAliases           Warning      test.ps1   28    'foreach' is an alias of 'ForEach-Object'. Alias can
                                                                  introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Get-Planet 15    'foreach' is an alias of 'ForEach-Object'. Alias can
                                                 .ps1             introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Get-Planet 17    'where' is an alias of 'Where-Object'. Alias can introduce
                                                 .ps1             possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Get-Planet 104   'Select' is an alias of 'Select-Object'. Alias can
                                                 .Tests.ps1       introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSUseShouldProcessForStateChangingF Warning      chocolatey 40    Function 'Update-Directory' has verb that could change
unctions                                         Install.ps       system state. Therefore, the function has to support
                                                 1                'ShouldProcess'.
PSAvoidOverwritingBuiltInCmdlets    Warning      chocolatey 111   'Get-FileHash' is a cmdlet that is included with PowerShell
                                                 Install.ps       (version desktop-5.1.14393.206-windows) whose definition
                                                 1                should not be overridden
PSAvoidUsingPositionalParameters    Information  getNugetEx 10    Cmdlet 'Write-Host' has positional parameter. Please use
                                                 e.ps1            named parameters instead of positional parameters when
                                                                  calling a command.
PSAvoidUsingPositionalParameters    Information  getNugetEx 14    Cmdlet 'Write-Host' has positional parameter. Please use
                                                 e.ps1            named parameters instead of positional parameters when
                                                                  calling a command.
PSAvoidUsingPositionalParameters    Information  getNugetEx 21    Cmdlet 'Write-Host' has positional parameter. Please use
                                                 e.ps1            named parameters instead of positional parameters when
                                                                  calling a command.
PSAvoidUsingPositionalParameters    Information  getNugetEx 26    Cmdlet 'Write-Host' has positional parameter. Please use
                                                 e.ps1            named parameters instead of positional parameters when
                                                                  calling a command.
PSAvoidUsingWriteHost               Warning      getNugetEx 10    File 'getNugetExe.ps1' uses Write-Host. Avoid using
                                                 e.ps1            Write-Host because it might not work in all hosts, does not
                                                                  work when there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      getNugetEx 14    File 'getNugetExe.ps1' uses Write-Host. Avoid using
                                                 e.ps1            Write-Host because it might not work in all hosts, does not
                                                                  work when there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      getNugetEx 21    File 'getNugetExe.ps1' uses Write-Host. Avoid using
                                                 e.ps1            Write-Host because it might not work in all hosts, does not
                                                                  work when there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      getNugetEx 26    File 'getNugetExe.ps1' uses Write-Host. Avoid using
                                                 e.ps1            Write-Host because it might not work in all hosts, does not
                                                                  work when there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingCmdletAliases           Warning      getNugetEx 17    'select' is an alias of 'Select-Object'. Alias can
                                                 e.ps1            introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      getNugetEx 36    'select' is an alias of 'Select-Object'. Alias can
                                                 e.ps1            introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSReviewUnusedParameter             Warning      publishPSG 5     The parameter 'Path' has been declared but not used.
                                                 alleryPack
                                                 age.ps1
PSAvoidUsingWriteHost               Warning      publishPSG 19    File 'publishPSGalleryPackage.ps1' uses Write-Host. Avoid
                                                 alleryPack       using Write-Host because it might not work in all hosts,
                                                 age.ps1          does not work when there is no host, and (prior to PS 5.0)
                                                                  cannot be suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSAvoidUsingWriteHost               Warning      signModule 34    File 'signModule.ps1' uses Write-Host. Avoid using
                                                 .ps1             Write-Host because it might not work in all hosts, does not
                                                                  work when there is no host, and (prior to PS 5.0) cannot be
                                                                  suppressed, captured, or redirected. Instead, use
                                                                  Write-Output, Write-Verbose, or Write-Information.
PSUseApprovedVerbs                  Warning      Format.psm 139   The cmdlet 'Sort-Property' uses an unapproved verb.
                                                 1
PSAvoidUsingCmdletAliases           Warning      Format.psm 13    'Select' is an alias of 'Select-Object'. Alias can
                                                 1                introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 13    '%' is an alias of 'ForEach-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 67    'sort' is an alias of 'Sort-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 67    'foreach' is an alias of 'ForEach-Object'. Alias can
                                                 1                introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 78    'sort' is an alias of 'Sort-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 78    'foreach' is an alias of 'ForEach-Object'. Alias can
                                                 1                introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 142   'where' is an alias of 'Where-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 143   'select' is an alias of 'Select-Object'. Alias can
                                                 1                introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 144   'sort' is an alias of 'Sort-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 157   'sort' is an alias of 'Sort-Object'. Alias can introduce
                                                 1                possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Format.psm 157   'Select' is an alias of 'Select-Object'. Alias can
                                                 1                introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSUseSingularNouns                  Warning      Pester.ps1 237   The cmdlet 'Get-AssertionDynamicParams' uses a plural noun.
                                                                  A singular noun should be used instead.
PSUseSingularNouns                  Warning      Pester.ps1 959   The cmdlet 'Set-PesterStatistics' uses a plural noun. A
                                                                  singular noun should be used instead.
PSUseDeclaredVarsMoreThanAssignment Warning      Pester.ps1 333   The variable 'invokedViaInvokePester' is assigned but never
s                                                                 used.
PSUseDeclaredVarsMoreThanAssignment Warning      Pester.ps1 717   The variable 'parameters' is assigned but never used.
s
PSUseApprovedVerbs                  Warning      Pester.ps1 241   The cmdlet 'Has-Flag' uses an unapproved verb.
PSUseApprovedVerbs                  Warning      Pester.ps1 999   The cmdlet 'Contain-AnyStringLike' uses an unapproved verb.
PSUseProcessBlockForPipelineCommand Warning      Pester.ps1 249   Command accepts pipeline input but has not defined a
                                                                  process block.
PSAvoidUsingCmdletAliases           Warning      Pester.ps1 108   'Where' is an alias of 'Where-Object'. Alias can introduce
                                                                  possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Pester.ps1 135   'Where' is an alias of 'Where-Object'. Alias can introduce
                                                                  possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSAvoidUsingCmdletAliases           Warning      Pester.ps1 680   'foreach' is an alias of 'ForEach-Object'. Alias can
                                                                  introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSAvoidUsingCmdletAliases           Warning      Pester.ps1 688   'foreach' is an alias of 'ForEach-Object'. Alias can
                                                                  introduce possible problems and make scripts hard to
                                                                  maintain. Please consider changing alias to its full
                                                                  content.
PSUseOutputTypeCorrectly            Information  Pester.ps1 770   The cmdlet 'Invoke-Pester' returns an object of type
                                                                  'Pester.Run' but this type is not declared in the
                                                                  OutputType attribute.
PSUseShouldProcessForStateChangingF Warning      Pester.ps1 786   Function 'New-PesterOption' has verb that could change
unctions                                                          system state. Therefore, the function has to support
                                                                  'ShouldProcess'.
PSUseShouldProcessForStateChangingF Warning      Pester.ps1 959   Function 'Set-PesterStatistics' has verb that could change
unctions                                                          system state. Therefore, the function has to support
                                                                  'ShouldProcess'.
PSUseSingularNouns                  Warning      Pester.RSp 78    The cmdlet 'Add-RSpecTestObjectProperties' uses a plural
                                                 ec.ps1           noun. A singular noun should be used instead.
PSUseSingularNouns                  Warning      Pester.RSp 105   The cmdlet 'Add-RSpecBlockObjectProperties' uses a plural
                                                 ec.ps1           noun. A singular noun should be used instead.
PSUseSingularNouns                  Warning      Pester.RSp 244   The cmdlet 'Remove-RSpecNonPublicProperties' uses a plural
                                                 ec.ps1           noun. A singular noun should be used instead.
PSReviewUnusedParameter             Warning      Pester.RSp 333   The parameter 'i' has been declared but not used.
                                                 ec.ps1
PSUseOutputTypeCorrectly            Information  Pester.RSp 241   The cmdlet 'New-PesterConfiguration' returns an object of
                                                 ec.ps1           type 'PesterConfiguration' but this type is not declared in
                                                                  the OutputType attribute.
PSUseShouldProcessForStateChangingF Warning      Pester.RSp 237   Function 'New-PesterConfiguration' has verb that could
unctions                                         ec.ps1           change system state. Therefore, the function has to support
                                                                  'ShouldProcess'.
PSUseShouldProcessForStateChangingF Warning      Pester.RSp 244   Function 'Remove-RSpecNonPublicProperties' has verb that
unctions                                         ec.ps1           could change system state. Therefore, the function has to
                                                                  support 'ShouldProcess'.
PSAvoidUsingCmdletAliases           Warning      Pester.RSp 48    'where' is an alias of 'Where-Object'. Alias can introduce
                                                 ec.ps1           possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.
PSUseApprovedVerbs                  Warning      Pester.RSp 51    The cmdlet 'Filter-Excluded' uses an unapproved verb.
                                                 ec.ps1
PSUseApprovedVerbs                  Warning      Pester.RSp 113   The cmdlet 'PostProcess-RspecTestRun' uses an unapproved
                                                 ec.ps1           verb.
PSProvideCommentHelp                Information  Pester.Run 78    The cmdlet 'Find-Test' does not have a help comment.
                                                 time.psm1
PSProvideCommentHelp                Information  Pester.Run 746   The cmdlet 'Is-Discovery' does not have a help comment.
                                                 time.psm1
PSProvideCommentHelp                Information  Pester.Run 1000  The cmdlet 'Invoke-PluginStep' does not have a help comment.
                                                 time.psm1
PSProvideCommentHelp                Information  Pester.Run 1680  The cmdlet 'Invoke-Test' does not have a help comment.
                                                 time.psm1
PSProvideCommentHelp                Information  Pester.Run 2372  The cmdlet 'Recurse-Up' does not have a help comment.
                                                 time.psm1
PSUseDeclaredVarsMoreThanAssignment Warning      Pester.Run 1020  The variable 'sw' is assigned but never used.
s                                                time.psm1
PSUseDeclaredVarsMoreThanAssignment Warning      Pester.Run 1695  The variable 'PesterPreference' is assigned but never used.
s                                                time.psm1
PSReviewUnusedParameter             Warning      Pester.Run 893   The parameter 'SessionState' has been declared but not
                                                 time.psm1        used.
PSUseProcessBlockForPipelineCommand Warning      Pester.Run 2049  Command accepts pipeline input but has not defined a
                                                 time.psm1        process block.
PSAvoidUsingCmdletAliases           Warning      Pester.Run 2052  'where' is an alias of 'Where-Object'. Alias can introduce
                                                 time.psm1        possible problems and make scripts hard to maintain. Please
                                                                  consider changing alias to its full content.

2. Describe Your Environment

3. Expected Behavior

4.Current Behavior

5. Possible Solution

PSScriptAnalyzer and other tools can be added to the project and builds to improve code quality and protect main branch.
Warnings can be shown yet continue to allow build by using limited exclusions, until they are resolved and can then be used as build breakers.

Invoke-ScriptAnalyzer . -Recurse

# PSScriptAnalyzerSettings.psd1
@{
    Severity=@('Error','Warning')
    ExcludeRules=@('PSAvoidUsingCmdletAliases',
                'PSAvoidUsingWriteHost')
}

https://github.com/PowerShell/PSScriptAnalyzer

6. Context

[nohwnd]

Yeah will do, once I get the most pressing issues out of the way, as always :)

[asears]

Want to assign some of this to me? I have submitted draft PR which can be reviewed for TODO, FIXME. As part of the scans, can probably suppress certain warnings such as Write-Host and changes required to support PesterPreference Global variables.

Invoke-ScriptAnalyzer .  -Recurse -ExcludeRule PSAvoidUsingWriteHost,PSAvoidGlobalVars,PSUseDeclaredVarsMoreThanAssignments

There are probably some scoping bugs in both the Pester code and the PSScriptAnalyzer scans.

I purposely avoided the /src folder for now.

[fflaten]

Comments to questions made in #1700 .

2. As part of improving the help, could expand the function comments into proper synopsis comment-based help, ensure each function in this script has help or template for help.

Output.ps1 contains internal functions, not public. Help not mandatory, at least not a detailed one.

3. I have seen script analyzer complain about Write-Host, should it be deprecated in favour of another method? I didn't see it complain about this file. Is it still "considered harmful"? Would Write-Information or Write-Output be another option? I understand there's some limitations here due to terminal outputs, mostly curious.

Deprecating Write-Host is a breaking change and probably not part of the near future (I would guess). Write-Information was introduced in PowerShell 5.0 and using normal output would break PassThru (Pester result object). It should be ignored in the ScriptAnalyzer-definition (whenever it's going to be part of the repo) for now as the cmdlet usage is a key part of the framework atm.

[fflaten]

Small first step done in #1745.
Alias usage cleaned up in /src/*.

Not part of CI yet as "style guide" needs to be defined to know which rules to exclude globally vs suppress per function. Would also need to decide search-path to test against. Src is published code and most important.

[fflaten]

Had some fun with this today to see how it could integrate in current azure pipeline since that was preferred.

@nohwnd: Any issue with installing https://marketplace.visualstudio.com/items?itemName=sariftools.scans in devops org?

If not, then we can run the PSSA-result through ConvertToSARIF + a few modifications and get this report with links directly to source (links only say filename though):

Not as fancy (and currently noisy) as github code analysis (below):

[nohwnd]

I don't see any problem installing it. I also don't see any problem having Github action that would build just to get the analyzer warnings into the output. Might be even easier to remove it later / disable it if it is too overwhelming.

AzDo is used here primarily because I can deploy hosted agents that run older versions of powershell. But this task we probably want to run just against ps7, because otherwise we get multiple copies of every warning.

[fflaten]

I got both working and don't really have a preference tbh. AzDo integration is more silent/hidden and only there if you want to look at it.

The github action workflow would be more in your face. Both in PRs (that's probably a good thing), but also by showing the new 174 security alerts in the repo 😄

If the security alerts didn't scare you, I can submit a PR for github first and should it be a problem then we can move it later.

[nohwnd]

How many of those alerts are about a variable being unused because it is in defined in BeforeAll, and used in It? :) Or in other words, how much unnecessary pollution we will have to add to the code to drive the alerts to 0, vs how many actual issues it found? (Rough counts of course, is it 50:50, or better?).

[fflaten]

No unused variables, but 26 unused parameters. Only scanning src so no noise from tests (unlike vscode).

There are "77 *-Objects cmdlets used" (custom rule due to being slow) and 30 plural nouns in function names. Could easily disable some rules if we don't care.

[fflaten]

Current state:

Count Name
----- ----
   77 Pester.BuildAnalyzerRules\Measure-ObjectCmdlets
   11 Pester.BuildAnalyzerRules\Measure-SafeCommands
    4 PSAvoidAssignmentToAutomaticVariable
    1 PSAvoidGlobalVars
    1 PSAvoidOverwritingBuiltInCmdlets
    2 PSAvoidUsingCmdletAliases
    7 PSAvoidUsingEmptyCatchBlock
    2 PSAvoidUsingWriteHost
    1 PSPossibleIncorrectComparisonWithNull
   26 PSReviewUnusedParameter
   10 PSUseDeclaredVarsMoreThanAssignments
    9 PSUseProcessBlockForPipelineCommand
   34 PSUseSingularNouns

[nohwnd]

These look like good violations to fix, please go ahead and PR the github pipeline 👍

[fflaten]

These look like good violations to fix, please go ahead and PR the github pipeline 👍

Did it two days ago 😁

[nohwnd]

You are too fast 😁