Merge pull request #13 from Lombiq/issue/OSOE-472

OSOE-472: Prevent the usage of PowerShell aliases

.github/workflows/static-code-analysis.yml

@@ -0,0 +1,38 @@
+name: Static Code Analysis
+
+# Runs for PRs opened for any branch, and pushes to the dev branch.
+on:
+  pull_request:
+  push:
+    branches:
+      - dev
+
+defaults:
+  run:
+    shell: pwsh
+
+jobs:
+  powershell-static-code-analysis:
+    name: PowerShell Static Code Analysis
+    strategy:
+      matrix:
+        machine-type: [ubuntu-latest, windows-latest]
+    runs-on: ${{ matrix.machine-type }}
+
+    steps:
+    - uses: actions/checkout@v3
+      with:
+        submodules: 'recursive'
+
+    - name: Windows PowerShell Static Code Analysis
+      run: |
+        if ($Env:RUNNER_OS -ne 'Windows') 
+        {
+            Write-Output "This step is only for Windows. The current runner is $Env:RUNNER_OS so it's skipped."
+            exit 0
+        }
+        powershell .\Lombiq.Analyzers.PowerShell\Invoke-Analyzer.ps1 -ForGitHubActions
+    
+    - name: PowerShell 7+ Static Code Analysis
+      if: always() # Even if the above fails, let's check both.
+      run: .\Lombiq.Analyzers.PowerShell\Invoke-Analyzer.ps1 -ForGitHubActions

.github/workflows/test-analysis-failure.yml

@@ -17,8 +17,9 @@ jobs:
       timeout-minutes: 30
       build-expected-code-analysis-errors: |
         MSB3073: The command exited with non-zero code.
+        PSAvoidUsingAutomaticVariableAlias: '$_' is an alias of '$PSItem'.
+        PSAvoidUsingCmdletAliases: '%' is an alias of 'ForEach-Object'.
         PSAvoidUsingEmptyCatchBlock: Empty catch block is used.
-        PSAvoidUsingCmdletAliases: 'echo' is an alias of 'Write-Output'.
         PSUseApprovedVerbs: The cmdlet 'Violate-Analyzers' uses an unapproved verb.
         PSUseSingularNouns: The cmdlet 'Violate-Analyzers' uses a plural noun.
 
@@ -31,7 +32,8 @@ jobs:
       timeout-minutes: 30
       build-expected-code-analysis-errors: |
         MSB3073: The command exited with non-zero code.
+        PSAvoidUsingAutomaticVariableAlias: '$_' is an alias of '$PSItem'.
+        PSAvoidUsingCmdletAliases: '%' is an alias of 'ForEach-Object'.
         PSAvoidUsingEmptyCatchBlock: Empty catch block is used.
-        PSAvoidUsingCmdletAliases: 'echo' is an alias of 'Write-Output'.
         PSUseApprovedVerbs: The cmdlet 'Violate-Analyzers' uses an unapproved verb.
         PSUseSingularNouns: The cmdlet 'Violate-Analyzers' uses a plural noun.

Lombiq.Analyzers.PowerShell.Tests/UnitTests/PowerShellAnalysisTests.cs

@@ -51,8 +51,9 @@ await _powerShell
 
     private static void MessageShouldContainViolationCodes(string message)
     {
-        message.ShouldContain("PSAvoidUsingEmptyCatchBlock");
+        message.ShouldContain("PSAvoidUsingAutomaticVariableAlias");
         message.ShouldContain("PSAvoidUsingCmdletAliases");
+        message.ShouldContain("PSAvoidUsingEmptyCatchBlock");
         message.ShouldContain("PSUseApprovedVerbs");
         message.ShouldContain("PSUseSingularNouns");
     }

Lombiq.Analyzers.PowerShell/Invoke-Analyzer.ps1

@@ -1,14 +1,16 @@
-param(
+param(
     $SettingsPath = (Join-Path (Split-Path -Parent $MyInvocation.MyCommand.Path) 'PSScriptAnalyzerSettings.psd1'),
     [Switch] $ForGitHubActions,
     [Switch] $ForMsBuild,
-    [Switch] $IncludeTestSolutions
+    [Switch] $IncludeTestSolutions,
+    [switch] $Fix
 )
 
-# This is like Get-ChildItem -Recurse -Include $IncludeFile | ? { $_.FullName -notlike "*\$ExcludeDirectory\*" } but
-# much faster. For example, this is relevant for ignoring node_modules.
+# This is like Get-ChildItem -Recurse -Include $IncludeFile | ? { $PSItem.FullName -notlike "*\$ExcludeDirectory\*" }
+# but much faster. For example, this is relevant for ignoring node_modules.
 # - Measure-Command { Find-Recursively -Path . -IncludeFile *.ps1 -ExcludeDirectory node_modules } => 3.83s
-# - Measure-Command { Get-ChildItem -Recurse -Force -Include $IncludeFile | ? { $_.FullName -notlike "*\$ExcludeDirectory\*" } } => 111.27s
+# - Measure-Command { Get-ChildItem -Recurse -Force -Include $IncludeFile | ? { $PSItem.FullName -notlike
+#   "*\$ExcludeDirectory\*" } } => 111.27s
 function Find-Recursively([string] $Path = '.', [string[]] $IncludeFile, [string] $ExcludeDirectory)
 {
     $ExcludeDirectory = $ExcludeDirectory.ToUpperInvariant()
@@ -25,7 +27,7 @@ function Find-Recursively([string] $Path = '.', [string[]] $IncludeFile, [string
         foreach ($child in (Get-ChildItem $Here.FullName -Force))
         {
             if ($child -is [System.IO.DirectoryInfo]) { Find-Inner $child }
-            elseif (($IncludeFile | ? { $child.name -like $_ }).Count) { $child }
+            elseif (($IncludeFile | Where-Object { $child.name -like $PSItem }).Count) { $child }
         }
     }
 
@@ -88,19 +90,32 @@ if ((Get-InstalledModule PSScriptAnalyzer -ErrorAction SilentlyContinue).Version
     }
 }
 
+$analyzerParameters = @{
+    "Settings"              = $SettingsPath.FullName
+    "CustomRulePath"        = Join-Path -Path (Split-Path $MyInvocation.MyCommand.Path -Parent) -ChildPath Rules
+    "RecurseCustomRulePath" = $true
+    "IncludeDefaultRules"   = $true
+    "Fix"                   = $Fix
+}
 $results = Find-Recursively -IncludeFile "*.ps1", "*.psm1", "*.psd1" -ExcludeDirectory node_modules |
-    ? { # Exclude /TestSolutions/Violate-Analyzers.ps1 and /TestSolutions/*/Violate-Analyzers.ps1
+    Where-Object { # Exclude /TestSolutions/Violate-Analyzers.ps1 and /TestSolutions/*/Violate-Analyzers.ps1
         $IncludeTestSolutions -or -not (
-            $_.Name -eq 'Violate-Analyzers.ps1' -and
-            ($_.Directory.Name -eq 'TestSolutions' -or $_.Directory.Parent.Name -eq 'TestSolutions')) } |
-    % { Invoke-ScriptAnalyzer $_.FullName -Settings $SettingsPath.FullName } |
+            $PSItem.Name -eq 'Violate-Analyzers.ps1' -and
+            ($PSItem.Directory.Name -eq 'TestSolutions' -or $PSItem.Directory.Parent.Name -eq 'TestSolutions')) } |
+    ForEach-Object { Invoke-ScriptAnalyzer -Path $PSItem.FullName @analyzerParameters } |
     # Only Warning and above (ignore "Information" type results).
-    ? { $_.Severity -ge [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticSeverity]::Warning }
+    Where-Object { $PSItem.Severity -ge [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticSeverity]::Warning }
 
 foreach ($result in $results)
 {
     $message = $result.RuleName + ": " + $result.Message
     Write-FileError -Path $result.ScriptPath -Line $result.Line -Column $result.Column $message
 }
 
-exit $results.Count
+# Exit code indicates the existence of analyzer violations instead of the number of violations, because exiting with
+# code 5 (when there are 5 violations) changes how MSBuild interprets the results and yields the error MSB3075 instead
+# of MSB3073 for some reason.
+if ($results.Count -ne 0)
+{
+    exit 1
+}

Lombiq.Analyzers.PowerShell/Lombiq.Analyzers.PowerShell.csproj

@@ -29,6 +29,7 @@
     <None Include="Lombiq.Analyzers.PowerShell.NuGet.props" Pack="true" PackagePath="build\Lombiq.Analyzers.PowerShell.props" />
     <None Include="Invoke-Analyzer.ps1" Pack="true" PackagePath="" />
     <None Include="PSScriptAnalyzerSettings.psd1" Pack="true" PackagePath="" />
+    <None Include="Rules\**" Pack="true" PackagePath="Rules\" />
   </ItemGroup>
 
 </Project>

Lombiq.Analyzers.PowerShell/PSScriptAnalyzerSettings.psd1

@@ -9,11 +9,4 @@
         # resolved.
         'PSReviewUnusedParameter'
     )
-    'Rules' =
-    @{
-        'PSAvoidUsingCmdletAliases' =
-        @{
-            'allowlist' = @('%', '?')
-        }
-    }
 }

Lombiq.Analyzers.PowerShell/Rules/Measure-AutomaticVariableAlias/Measure-AutomaticVariableAlias.psm1

@@ -0,0 +1,68 @@
+<#
+.SYNOPSIS
+    Detects the usages of the alias $_ of the automatic variable $PSItem and suggests to correct them.
+.DESCRIPTION
+    The full name of the automatic variable $PSItem should be used instead of its alias $_ for consistency.
+.EXAMPLE
+    Measure-AutomaticVariableAlias -Token $Token
+.INPUTS
+    [System.Management.Automation.Language.Token[]]
+.OUTPUTS
+    [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]]
+.NOTES
+    Copied (and modified) version of
+    https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1#L613.
+#>
+function Measure-AutomaticVariableAlias
+{
+    [CmdletBinding()]
+    [OutputType([Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord[]])]
+    Param
+    (
+        [Parameter(Mandatory = $true)]
+        [ValidateNotNullOrEmpty()]
+        [System.Management.Automation.Language.Token[]]
+        $Token
+    )
+
+    Process
+    {
+        $results = @()
+
+        try
+        {
+            # Filter down tokens to just variable tokens with the name "_".
+            foreach ($automaticVariableAliasToken in $Token | Where-Object { $PSItem.GetType().Name -eq "VariableToken" -and $PSItem.Name -eq "_" })
+            {
+                $correctionTypeName = "Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent"
+                $correctionExtent = New-Object -TypeName $correctionTypeName -ArgumentList @(
+                    $automaticVariableAliasToken.Extent.StartLineNumber
+                    $automaticVariableAliasToken.Extent.EndLineNumber
+                    $automaticVariableAliasToken.Extent.StartColumnNumber
+                    $automaticVariableAliasToken.Extent.EndColumnNumber
+                    '$PSItem'
+                    'Replaced the usage of the alias of the automatic variable ''$_'' with its full name ''$PSItem''.'
+                )
+
+                $suggestedCorrections = New-Object System.Collections.ObjectModel.Collection[$correctionTypeName]
+                $suggestedCorrections.add($correctionExtent) | Out-Null
+
+                $results += [Microsoft.Windows.Powershell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
+                    "Extent"               = $automaticVariableAliasToken.Extent
+                    "Message"              = '''$_'' is an alias of the automatic variable ''$PSItem''. Please consider using the full name of this' +
+                    ' variable for consistency.'
+                    "RuleName"             = "PSAvoidUsingAutomaticVariableAlias"
+                    "RuleSuppressionID"    = "PSAvoidUsingAutomaticVariableAlias"
+                    "Severity"             = "Warning"
+                    "SuggestedCorrections" = $suggestedCorrections
+                }
+            }
+
+            return $results
+        }
+        catch
+        {
+            $PSCmdlet.ThrowTerminatingError($PSItem)
+        }
+    }
+}

Readme.md

@@ -80,6 +80,11 @@ Occasionally there is good reason to ignore an analyzer warning. In this case ad
 
 Suppressing a specific line or range (like `#pragma warning disable` in C#) is not currently supported. See [the associated PSScriptAnalyzer issue](https://github.com/PowerShell/PSScriptAnalyzer/issues/849).
 
+### Implementing custom analyzer rules
+
+Check out [the official documentation](https://learn.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/create-custom-rule) for instructions and [CommunityAnalyzerRules](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1) for additional inspiration.
+Custom rule modules should be placed in the `Lombiq.Analyzers.PowerShell\Rules` folder to be automatically detected by the `Invoke-Analyzer` script.
+
 ## Contributing and support
 
 Bug reports, feature requests, comments, questions, code contributions and love letters are warmly welcome. You can send them to us via GitHub issues and pull requests. Please adhere to our [open-source guidelines](https://lombiq.com/open-source-guidelines) while doing so.

TestSolutions/Lombiq.Analyzers.PowerShell.PackageReference/Lombiq.Analyzers.PowerShell.PackageReference.csproj

@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Lombiq.Analyzers.PowerShell" Version="1.0.1" />
+    <PackageReference Include="Lombiq.Analyzers.PowerShell" Version="1.1.0-beta.1.osoe-472" />
   </ItemGroup>
 
   <Target Name="CopyFiles" BeforeTargets="PrepareForBuild">

TestSolutions/Violate-Analyzers.ps1

@@ -1,8 +1,8 @@
 function Violate-Analyzers()
 {
-    "This file is intended made to verify that PSScriptAnalyzer works and contains intentionally bad code."
+    "This file is intended to verify that PSScriptAnalyzer works and contains intentionally bad code."
 }
 
-try { echo Violate-Analyzers } catch { }
+try { Violate-Analyzers } catch { }
 
-Get-ChildItem . | % { "This is permitted by our settings file." }
+Get-ChildItem . | % { $_ }