Cleanup alias and missing SafeCommands (pester#1745)

* Added UnsafeCommands custom analyzer rule * Updated and renamed custom rules module * Updated PSScriptAnalyzer settings * Removed alias-usage and replaced unsafe commands * Removed temp debug code * Remove BOM * SafeCommands cleanup in /src/functions * Warn or suppress SafeCommand-rule * Apply suggestions from code review Co-authored-by: Jakub Jareš <[email protected]> * Fixed sort alias usage * typos * Safecommands in mock-wrapper * Revert BOM-removal in Mock.ps1 Co-authored-by: Jakub Jareš <[email protected]>
kimballjohnson · Oct 30, 2020 · f90f426 · f90f426
1 parent 3e902a0
commit f90f426
Show file tree

Hide file tree

Showing 23 changed files with 289 additions and 111 deletions.
diff --git a/PSScriptAnalyzerSettings.psd1 b/PSScriptAnalyzerSettings.psd1
@@ -0,0 +1,9 @@
+@{
+    Severity = @('Error','Warning')
+    IncludeDefaultRules = $true
+    CustomRulePath = './Pester.BuildAnalyzerRules'
+    ExcludeRules=@(
+        'PSUseShouldProcessForStateChangingFunctions'
+        'PSUseApprovedVerbs'
+    )
+}
diff --git a/Pester.BuildAnalyzerRules/Pester.BuildAnalyzerRules.psd1 b/Pester.BuildAnalyzerRules/Pester.BuildAnalyzerRules.psd1
@@ -0,0 +1,85 @@
+@{
+
+    # Script module or binary module file associated with this manifest.
+    RootModule        = 'Pester.BuildAnalyzerRules.psm1'
+
+    # Version number of this module.
+    ModuleVersion     = '0.0.1'
+
+    # ID used to uniquely identify this module
+    GUID              = '7e04f341-3ce0-4f3c-9177-1a7de9daaddf'
+
+    # Author of this module
+    Author            = 'Pester Team'
+
+    # Company or vendor of this module
+    CompanyName       = 'Pester'
+
+    # Copyright statement for this module
+    Copyright         = 'Copyright (c) 2020 by Pester Team, licensed under Apache 2.0 License.'
+
+    # Description of the functionality provided by this module
+    Description       = 'This module contains custom script analyzer rules used for validation during build of the Pester module.'
+
+    # Minimum version of the PowerShell engine required by this module
+    PowerShellVersion = '3.0'
+
+    # Functions to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no functions to export.
+    FunctionsToExport = 'Measure-*'
+
+    # Cmdlets to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no cmdlets to export.
+    CmdletsToExport   = @()
+
+    # Variables to export from this module
+    VariablesToExport = @()
+
+    # Aliases to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no aliases to export.
+    AliasesToExport   = @()
+
+    # List of all modules packaged with this module
+    # ModuleList = @()
+
+    # List of all files packaged with this module
+    # FileList = @()
+
+    # Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell.
+    PrivateData       = @{
+
+        PSData = @{
+
+            # Tags applied to this module. These help with module discovery in online galleries.
+            # Tags = @()
+
+            # A URL to the license for this module.
+            # LicenseUri = ''
+
+            # A URL to the main website for this project.
+            # ProjectUri = ''
+
+            # A URL to an icon representing this module.
+            # IconUri = ''
+
+            # ReleaseNotes of this module
+            # ReleaseNotes = ''
+
+            # Prerelease string of this module
+            # Prerelease = ''
+
+            # Flag to indicate whether the module requires explicit user acceptance for install/update/save
+            # RequireLicenseAcceptance = $false
+
+            # External dependent modules of this module
+            # ExternalModuleDependencies = @()
+
+        } # End of PSData hashtable
+
+    } # End of PrivateData hashtable
+
+    # HelpInfo URI of this module
+    # HelpInfoURI = ''
+
+    # Default prefix for commands exported from this module. Override the default prefix using Import-Module -Prefix.
+    # DefaultCommandPrefix = ''
+
+}
+
diff --git a/Pester.BuildAnalyzerRules/Pester.BuildAnalyzerRules.psm1 b/Pester.BuildAnalyzerRules/Pester.BuildAnalyzerRules.psm1
@@ -0,0 +1,76 @@
+# Get list of SafeCommands
+$SafeCommands = & { . "$PSScriptRoot/../src/functions/Pester.SafeCommands.ps1"; $Script:SafeCommands }
+# Workaround as RuleSuppressionID-based suppression is bugged. returns error.
+# Should be replaced with the following line when PSScriptAnalyzer is fixed. See Invoke-Pester
+# [Diagnostics.CodeAnalysis.SuppressMessageAttribute('Pester.BuildAnalyzerRules\Measure-SafeComands', 'Remove-Variable')]
+$IgnoreUnsafeCommands = @('Remove-Variable')
+function Measure-SafeComands {
+    <#
+    .SYNOPSIS
+    Should use $SafeCommand-variant of external function when available.
+    .DESCRIPTION
+    Pester module defines a $SafeCommands dictionary for external commands to avoid hijacking. To fix a violation of this rule, update the call to use SafeCoomands variant, ex. `& $SafeCommands['CommandName'] -Param1 Value1`.
+    .EXAMPLE
+    Measure-SafeComands -CommandAst $CommandAst
+    .INPUTS
+    [System.Management.Automation.Language.CommandAst]
+    .OUTPUTS
+    [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]]
+    .NOTES
+    None
+    #>
+    [CmdletBinding()]
+    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]])]
+    Param
+    (
+        [Parameter(Mandatory = $true)]
+        [ValidateNotNullOrEmpty()]
+        [System.Management.Automation.Language.CommandAst]
+        $CommandAst
+    )
+
+    Process {
+        $results = @()
+        try {
+            $commandName = $CommandAst.GetCommandName()
+
+            # If command exists in $SafeCommands, write error
+            if ($null -ne $commandName -and $commandName -in $SafeCommands.Keys -and $commandName -notin $IgnoreUnsafeCommands) {
+                foreach ($cmd in $CommandAst.CommandElements) {
+                    # Find extent for command name only
+                    if(($cmd -is [System.Management.Automation.Language.StringConstantExpressionAst]) -and $cmd.Value -eq $commandName) {
+
+                        #Define fix-action
+                        [int]$startLineNumber = $cmd.Extent.StartLineNumber
+                        [int]$endLineNumber = $cmd.Extent.EndLineNumber
+                        [int]$startColumnNumber = $cmd.Extent.StartColumnNumber
+                        [int]$endColumnNumber = $cmd.Extent.EndColumnNumber
+                        [string]$correction = "& `$SafeCommands['$commandName']"
+                        [string]$file = $MyInvocation.MyCommand.Definition
+                        [string]$description = 'Replacing with SafeCommands-type'
+                        $correctionExtent = New-Object 'Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent' $startLineNumber, $endLineNumber, $startColumnNumber, $endColumnNumber, $correction, $file, $description
+                        $suggestedCorrections = New-Object System.Collections.ObjectModel.Collection['Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent']
+                        $suggestedCorrections.add($correctionExtent) > $null
+
+                        # Output error
+                        $result = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
+                            'Message'              = "Unsafe call to '$commandName' found. $((Get-Help $MyInvocation.MyCommand.Name).Description.Text)"
+                            'Extent'               = $cmd.Extent
+                            'RuleName'             = $PSCmdlet.MyInvocation.InvocationName
+                            'Severity'             = 'Warning'
+                            'RuleSuppressionID'    = $commandName
+                            "SuggestedCorrections" = $suggestedCorrections
+                        }
+                        $results += $result
+                    }
+                }
+            }
+            return $results
+        }
+        catch {
+            $PSCmdlet.ThrowTerminatingError($PSItem)
+        }
+    }
+}
+
+Export-ModuleMember -Function 'Measure-*'
diff --git a/src/Format.psm1 b/src/Format.psm1
@@ -1,5 +1,6 @@
 # if -not build
-Import-Module "$PSScriptRoot/TypeClass.psm1" -DisableNameChecking
+. "$PSScriptRoot/functions/Pester.SafeCommands.ps1"
+& $SafeCommands['Import-Module'] "$PSScriptRoot/TypeClass.psm1" -DisableNameChecking
 # endif
 
 function Format-Collection ($Value, [switch]$Pretty) {
@@ -10,15 +11,15 @@ function Format-Collection ($Value, [switch]$Pretty) {
     }
     $count = $Value.Count
     $trimmed = $count  -gt $Limit
-    '@(' + (($Value | Select -First $Limit | % { Format-Nicely -Value $_ -Pretty:$Pretty }) -join $separator) + $(if ($trimmed) {', ...'}) + ')'
+    '@(' + (($Value | & $SafeCommands['Select-Object'] -First $Limit | & $SafeCommands['ForEach-Object'] { Format-Nicely -Value $_ -Pretty:$Pretty }) -join $separator) + $(if ($trimmed) {', ...'}) + ')'
 }
 
 function Format-Object ($Value, $Property, [switch]$Pretty) {
     if ($null -eq $Property) {
-        $Property = $Value.PSObject.Properties | Select-Object -ExpandProperty Name
+        $Property = $Value.PSObject.Properties | & $SafeCommands['Select-Object'] -ExpandProperty Name
     }
     $valueType = Get-ShortType $Value
-    $valueFormatted = ([string]([PSObject]$Value | Select-Object -Property $Property))
+    $valueFormatted = ([string]([PSObject]$Value | & $SafeCommands['Select-Object'] -Property $Property))
 
     if ($Pretty) {
         $margin = "    "
@@ -64,7 +65,7 @@ function Format-Hashtable ($Value) {
     $head = '@{'
     $tail = '}'
 
-    $entries = $Value.Keys | sort | foreach {
+    $entries = $Value.Keys | & $SafeCommands['Sort-Object'] | & $SafeCommands['ForEach-Object'] {
         $formattedValue = Format-Nicely $Value.$_
         "$_=$formattedValue" }
 
@@ -75,7 +76,7 @@ function Format-Dictionary ($Value) {
     $head = 'Dictionary{'
     $tail = '}'
 
-    $entries = $Value.Keys | sort | foreach {
+    $entries = $Value.Keys | & $SafeCommands['Sort-Object'] | & $SafeCommands['ForEach-Object'] {
         $formattedValue = Format-Nicely $Value.$_
         "$_=$formattedValue" }
 
@@ -139,9 +140,9 @@ function Format-Nicely ($Value, [switch]$Pretty) {
 function Sort-Property ($InputObject, [string[]]$SignificantProperties, $Limit = 4) {
 
     $properties = @($InputObject.PSObject.Properties |
-            where { $_.Name -notlike "_*"} |
-            select -expand Name |
-            sort)
+            & $SafeCommands['Where-Object'] { $_.Name -notlike "_*"} |
+            & $SafeCommands['Select-Object'] -expand Name |
+            & $SafeCommands['Sort-Object'])
     $significant = @()
     $rest = @()
     foreach ($p in $properties) {
@@ -154,7 +155,7 @@ function Sort-Property ($InputObject, [string[]]$SignificantProperties, $Limit =
     }
 
     #todo: I am assuming id, name properties, so I am just sorting the selected ones by name.
-    (@($significant | sort) + $rest) | Select -First $Limit
+    (@($significant | & $SafeCommands['Sort-Object']) + $rest) | & $SafeCommands['Select-Object'] -First $Limit
 
 }
 
@@ -189,7 +190,7 @@ function Format-Type ([Type]$Value) {
 }
 
 # if -not build
-Export-ModuleMember -Function @(
+& $SafeCommands['Export-ModuleMember'] -Function @(
     'Format-Collection'
     'Format-Object'
     'Format-Null'

diff --git a/src/Pester.RSpec.ps1 b/src/Pester.RSpec.ps1
@@ -17,11 +17,11 @@ function Find-File {
             }
 
             if ((& $script:SafeCommands['Test-Path'] $p)) {
-                $item = Get-Item $p
+                $item = & $SafeCommands['Get-Item'] $p
 
                 if ($item.PSIsContainer) {
                     # this is an existing directory search it for tests file
-                    Get-ChildItem -Recurse -Path $p -Filter "*$Extension" -File
+                    & $SafeCommands['Get-ChildItem'] -Recurse -Path $p -Filter "*$Extension" -File
                     continue
                 }
 
@@ -31,21 +31,21 @@ function Find-File {
                 }
 
                 if (".ps1" -ne $item.Extension) {
-                    Write-Error "Script path '$p' is not a ps1 file." -ErrorAction Stop
+                    & $SafeCommands['Write-Error'] "Script path '$p' is not a ps1 file." -ErrorAction Stop
                 }
 
                 # this is some file, we don't care if it is just a .ps1 file or .Tests.ps1 file
-                Add-Member -Name UnresolvedPath -Type NoteProperty -Value $p -InputObject $item
+                & $SafeCommands['Add-Member'] -Name UnresolvedPath -Type NoteProperty -Value $p -InputObject $item
                 $item
                 continue
             }
 
             # this is a path that does not exist so let's hope it is
             # a wildcarded path that will resolve to some files
-            Get-ChildItem -Recurse -Path $p -Filter "*$Extension" -File
+            & $SafeCommands['Get-ChildItem'] -Recurse -Path $p -Filter "*$Extension" -File
         }
 
-    Filter-Excluded -Files $files -ExcludePath $ExcludePath | where { $_ }
+    Filter-Excluded -Files $files -ExcludePath $ExcludePath | & $SafeCommands['Where-Object'] { $_ }
 }
 
 function Filter-Excluded ($Files, $ExcludePath) {

diff --git a/src/Pester.Runtime.psm1 b/src/Pester.Runtime.psm1
@@ -2109,7 +2109,7 @@ function PostProcess-ExecutedBlock {
                     $b.OwnPassedCount++
                 }
                 else {
-                    throw "Test '$($t.Name)' is in invalid state. $($t | Format-List -Force * | Out-String)"
+                    throw "Test '$($t.Name)' is in invalid state. $($t | Format-List -Force * | & $SafeCommands['Out-String'])"
                 }
             }
 
@@ -2183,7 +2183,7 @@ function Where-Failed {
         $Block
     )
 
-    $Block | View-Flat | where { $_.ShouldRun -and (-not $_.Executed -or -not $_.Passed) }
+    $Block | View-Flat | & $SafeCommands['Where-Object'] { $_.ShouldRun -and (-not $_.Executed -or -not $_.Passed) }
 }
 
 function View-Flat {
@@ -2340,7 +2340,7 @@ function New-BlockContainerObject {
 
     $type, $item = switch ($PSCmdlet.ParameterSetName) {
         "ScriptBlock" { "ScriptBlock", $ScriptBlock }
-        "Path" { "File", (Get-Item $Path) }
+        "Path" { "File", (& $SafeCommands['Get-Item'] $Path) }
         "File" { "File", $File }
         default { throw [System.ArgumentOutOfRangeException]"" }
     }
@@ -2423,7 +2423,7 @@ function Import-Dependency {
         $sb = {
             param ($p)
 
-            . $($p; Remove-Variable -Scope Local -Name p)
+            . $($p; & $SafeCommands['Remove-Variable'] -Scope Local -Name p)
         }
 
         $flags = [System.Reflection.BindingFlags]'Instance,NonPublic'
@@ -2548,7 +2548,7 @@ function ConvertTo-HumanTime {
 Reset-TestSuiteState
 
 # if -not build
-Export-ModuleMember -Function @(
+& $SafeCommands['Export-ModuleMember'] -Function @(
     # the core stuff I am mostly sure about
     'Reset-TestSuiteState'
     'New-Block'

diff --git a/src/Pester.Types.ps1 b/src/Pester.Types.ps1
@@ -1,6 +1,6 @@
 if ($PSVersionTable.PSVersion.Major -ge 6) {
-    Add-Type -Path "$PSScriptRoot/bin/netstandard2.0/Pester.dll"
+    & $SafeCommands['Add-Type'] -Path "$PSScriptRoot/bin/netstandard2.0/Pester.dll"
 }
 else {
-    Add-Type -Path "$PSScriptRoot/bin/net452/Pester.dll"
+    & $SafeCommands['Add-Type'] -Path "$PSScriptRoot/bin/net452/Pester.dll"
 }