Coding conventions and PSScriptAnalyzer

PSScriptAnalyzer is a static analysis tool, it examines source code and flags issues based on configured rules. PSScriptAnalyzer comes allows a developer to define and inject their own rules using the CustomRulePath parameter. Custom rules might be used to apply an organisation-specific style to code, beyond the best-practice rules.

The rules used as examples in this article are available on GitHub/indented-automation.

About AST

Before defining rules a little background is required. PSScriptAnalyzer performs its analysis using the Abstract Syntax Tree, or AST. The AST describes the elements of a piece of code as a hierarchy (a tree).

Access the syntax tree in PowerShell is simple, each script block has an "AST" property.

{ Write-Host 'Hello world' }.Ast

Climbing the tree

The tree can be navigated by property name, working up through the properties of this AST object. For example, the parts which make up the Write-Host command are accessible:

{ Write-Host 'Hello world' }.Ast.
                             EndBlock.
                             Statements.
                             PipelineElements.
                             CommandElements

In the example above the elements are placed under the EndBlock property. This occurs because the End block is the default block without the declaration of an explicit block, such as begin or process.

Find and FindAll

AST can be searched using the Find or FindAll methods. A predicate must be defined to search the syntax tree. The predicate is defined as a script block in PowerShell. A single argument is passed in to the script block (when it is dynamically called), an AST element or token from the tree, the predicate should expect to work with this.

The predicate should return true or false which denotes whether or not a particular element will be returned. The simplest predicate is, therefore, one which always returns true. This has the effect of returning every node visited by the search.

$predicate = { $true }

The example below demonstrates a predicate which might be used to find a command within the syntax tree:

{ $args[0] -is [System.Management.Automation.Language.CommandAst] }

A param block may be declared within the predicate script block to give the argument a more meaningful name.

{
    param ( $ast )

    $ast -is [System.Management.Automation.Language.CommandAst]
}

The predicate is the first argument of both the Find and FindAll methods. The second argument, a boolean value, dictates whether or not the search should descend into nested script blocks.

The example below uses FindAll to search for all instances of Write-Host within a script block:

$scriptBlock = { Write-Host 'Hello ' -NoNewLine; Write-Host 'world' }
$predicate = {
    param ( $ast )

    $ast -is [System.Management.Automation.Language.CommandAst] -and 
    $ast.GetCommandName() -eq 'Write-Host'
}
$scriptBlock.Ast.FindAll($predicate, $true)

The second term in the predicate uses the GetCommandName method of CommandAst, limiting the results to the Write-Host command instead of any command.

The Find method returns the first match instead of all.

$scriptBlock = { Write-Host 'Hello ' -NoNewLine; Write-Host 'world' }
$predicate = {
    param ( $ast )

    $ast -is [System.Management.Automation.Language.CommandAst] -and 
    $ast.GetCommandName() -eq 'Write-Host'
}
$scriptBlock.Ast.Find($predicate, $true)

The next example introduces a (called) nested script block. Using the FindAll method with the second argument set to false will limit the results to the first command, the second is within a nested script block and therefore cannot be discovered.

$scriptBlock = {
    Write-Host 'Hello ' -NoNewLine
    & { Write-Host 'world' }
}
$predicate = {
    param ( $ast )

    $ast -is [System.Management.Automation.Language.CommandAst] -and 
    $ast.GetCommandName() -eq 'Write-Host'
}
$scriptBlock.Ast.FindAll($predicate, $false)

Script analyzer rules

A script analyzer rule has the following characteristics:

  • Defined as a function in a module.
  • The first parameter name ends with “ast” or “token”.
  • The first parameter should be an AST type. For example, ScriptBlockAst.

Each rule should expect to return an object of type Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord. This type is declared within the PSScriptAnalyzer module assembly, it will be available if the PSScriptAnalyzer module is imported. The DiagnosticRecord may be created based on a hashtable. The hashtable must include the following keys:

  • Message – A description of the problem.
  • Extent – Normally taken from the AST object, describes the region (start and end) of code which triggered the rule.
  • RuleName – The name of the rule function.
  • Severity – Information, Warning, or Error.

Rule template

The basic structure of a rule may be as defined as shown below:

function RuleName {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        [System.Management.Automation.Language.ScriptBlockAst]$ast
    )
    
    # Rule implementation
    [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
        Message = 'Message'
        Extent = $ast.Extent
        RuleName = $myinvocation.MyCommand.Name
        Severity = 'Warning'
    }
}

The OutputType attribute is used as an indication, but is not used or consumed by PSScriptAnalyzer.

Neither CmdletBinding or Parameter are necessary when defining a rule. The parameter does not need a mandatory flag, it does not need to accept pipeline input, and so on.

Naming rules

Rule names should be short and to the point. The rules included with ScriptAnalyzer are prefixed with “PS” by default, or “PSDSC” if the rule applies to a DSC resource. Maintaining this convention, or using a similarly short prefix seems sensible.

For example, a custom rule which seeks to avoid the use of empty begin, process, or end blocks might be called “PSAvoidEmptyNamedBlocks”.

The community rules within the PSScriptAnalyzer module has adopted a convention of prefixing rule names with verb “Measure”. On one hand this extends the verb-noun pairing convention to rules, on the other hand is not descriptive and lends nothing of value to the rule. Script analyzer rules are not subject to by-convention discovery techniques available when interacting with commands in PowerShell.

The AST type

PSScriptAnalyzer uses the type name of the parameter to determine which tokens should be passed to the rule for analysis. The AST type should therefore be as specific as possible.

A rule which tests the content of a single command should use the CommandAst type, as shown below:

function RuleName {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        [System.Management.Automation.Language.CommandAst]$ast
    )
}

A rule working with the characteristics of a function should use FunctionDefinitionAst, Named blocks a NamedBlockAst, and so on.

The possible AST types are listed under the System.Management.Automation.Language namespace document on MSDN.

Alternatively, a short script will reveal the available AST types:

[PowerShell].Assembly.
             GetTypes().
             Where{
                $_.Namespace -eq 'System.Management.Automation.Language' -and 
                $_.Name.EndsWith('Ast')
             }

Example rules

The following rules are used as working examples:

  • PSAvoidEmptyNamedBlocks
  • PSAvoidNestedFunctions
  • PSAvoidProcessWithoutPipeline
  • PSAvoidWriteErrorStop

Each rule represents a personal convention, not necessarily a best practice.

PSAvoidEmptyNamedBlocks

The PSAvoidEmptyNamedBlocks rule applies to any NamedBlockAst element, therefore the parameter type is defined as NamedBlockAst.

If the AST type is not known, it might be discovered by exploring AST.

{ process { } }.Ast.ProcessBlock.GetType()

The Statements property is used to evaluate whether or not the block is empty. This is shown by viewing the AST object, and with confirmation using Get-Member.

{ process { } }.Ast.ProcessBlock { process { } }.Ast.ProcessBlock |
    Get-Member Statements

The ReadOnlyCollection type exposes a Count property which can be used to assess whether or not the block contains statements.

function PSAvoidEmptyNamedBlocks {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        # The AST type accepted by this rule.
        [System.Management.Automation.Language.NamedBlockAst]$ast
    )
    
    # The condition which will trigger the rule.
    if ($ast.Statements.Count -eq 0) {
        # Generate a record for the named block
        [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
            Message = 'The {0} block is empty.' -f $ast.BlockKind
            Extent = $ast.Extent
            RuleName = $myinvocation.MyCommand.Name
            Severity = 'Warning'
        }
    }
}

The rule can be tested by passing an AST of the expected type. The first command will trigger the rule, the second will not.

PSAvoidEmptyNamedBlocks { process { } }.Ast.ProcessBlock
PSAvoidEmptyNamedBlocks { process { 'Statement' } }.Ast.ProcessBlock

White space (empty lines) will not be counted as a statement, this rule will be effective no matter how many empty lines appear in the process block.

PSAvoidNestedFunctions

Functions can be nested within another function. To find nested functions the AST type FunctionDefinitionAst is used as the starting point.

As with the named block, this type may be discovered by working with the AST tree.

{ function name { } }.Ast.EndBlock.Statements[0].GetType()

The following script block shows the scenario the rule must look for.

{ function name { function nested { 'Body' } 'Body' } }

The rule must perform a search on every function definition, in each case searching for another function within the body (of the current function). The FindAll method is used to execute the search.

function PSAvoidNestedFunctions {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        [System.Management.Automation.Language.FunctionDefinitionAst]$ast
    )
    
    # Create a search for nested functions
    $ast.Body.FindAll( {
        param ( $ast )
        
        $ast -is [System.Management.Automation.Language.FunctionDefinitionAst]
    }, $true ) | ForEach-Object {
        # For each nested function, generate a record
        [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
            Message = 'The function {0} contains the nested function {1}.' -f $ast.Name, $_.name
            Extent = $_.Extent
            RuleName = $myinvocation.MyCommand.Name
            Severity = 'Warning'
        }
    }
}

The rule can be tested by passing an AST of the expected type. The first command will trigger the rule, the second will not.

PSAvoidNestedFunctions { function name { function nested { } } }.Ast.EndBlock.Statements[0]
PSAvoidNestedFunctions { function name { } }.Ast.EndBlock.Statements[0]

PSAvoidProcessWithoutPipeline

A rule may combine information from more than one AST search. The starting point for this rule is a script block with a process block.

The second part of the search finds parameters which are configured to accept pipeline input.

If the process block is present, but none of the parameters accepts pipeline input a record is created.

This scenario is demonstrated by the following script. The use of the process block implies an intent to support an input pipeline, but support for the pipeline has not been defined.

function name {
    [CmdletBinding()]
    param (
        [String]$Name
    )
    
    process {
        Write-Verbose $Name
    }
}

The rule, therefore, accepts a ScriptBlockAst and tests different nodes beneath.

function PSAvoidProcessWithoutPipeline {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        [System.Management.Automation.Language.ScriptBlockAst]$ast
    )
    
    if ($null -ne $ast.ProcessBlock -and $ast.ParamBlock) {
        $attributeAst = $ast.ParamBlock.Find( {
            param ( $ast )
            
            $ast -is [AttributeAst] -and 
            $ast.TypeName.Name -eq 'Parameter' -and 
            $ast.NamedArguments.Where{
                $_.ArgumentName -in 'ValueFromPipeline', 'ValueFromPipelineByPropertyName' -and 
                $_.Argument.SafeGetValue() -eq $true
            } 
        }, $false )
        
        if (-not $attributeAst) {
            [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
                Message  = 'Process declared without an input pipeline'
                Extent   = $ast.ProcessBlock.Extent
                RuleName = $myinvocation.MyCommand.Name
                Severity = 'Warning'
            }
        }
    }
}

PSAvoidWriteErrorStop

This final example explores examining the arguments passed to a command. In this case the intent is to avoid using Write-Error to generate a terminating error. The rule does not test the ErrorActionPreference variable which may limit the effectiveness of the test.

As this rule focuses on a command, it expects to receive a CommandAst as the argument. The rule then filters to a specific command name before exploring the commands parameters.

The index (location) of the ErrorAction parameter is extracted and used to locate the argument for the parameter.

# A CommandAst
$ast = { Write-Error -EA 1 }.Ast.EndBlock.Statements[0].PipelineElements[0]
# Select the EA parameter
$parameter = $ast.CommandElements.Where{ $_.ParameterName -eq 'EA' }[0]
# Select the argument AST (a ConstantExpressionAst)
$argument = $ast.CommandElements[$ast.CommandElements.IndexOf($parameter) + 1]
# Argument value
$argument.SafeGetValue()

The SafeGetValue method will fail if the value is a variable, or sub-expression. This error is shown when running the example below:

{ $var = 12; $var }.Ast.
                    EndBlock.
                    Statements[1].
                    PipelineElements[0].
                    Expression.
                    SafeGetValue()

If the PS variable were in fact a constant, such as the varible “true”, SafeGetValue would return the value.

{ $true }.Ast.
          EndBlock.
          Statements[0].
          PipelineElements[0].
          Expression.
          SafeGetValue()

Evaluating the value of the argument for the ErrorAction parameter is left to the Enum.Parse static method. This allows conversion of the different ways “Stop” might be expressed as shown below.

[Enum]::Parse([System.Management.Automation.ActionPreference], 1)
[Enum]::Parse([System.Management.Automation.ActionPreference], "1")
[Enum]::Parse([System.Management.Automation.ActionPreference], "Stop")

The rule below incorporates each of these steps.

function PSAvoidWriteErrorStop {
    [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])]
    param (
        [System.Management.Automation.Language.ScriptBlockAst]$ast
    )
    
    if ($ast.GetCommandName() -eq 'Write-Error') 
        $parameter = $ast.CommandElements.Where{
            $_.ParameterName -like 'ErrorA*' -or 
            $_.ParameterName -eq 'EA'
        }[0]
        
        if ($parameter) {
            $argumentIndex = $ast.CommandElements.IndexOf($parameter) + 1
            $argument = $ast.CommandElements[$argumentIndex].SafeGetValue()
            
            if ([Enum]::Parse([ActionPreference], $argument) -eq 'Stop') {
                [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{
                    Message = 'Write-Error is used to create a terminating error. throw or $pscmdlet.ThrowTerminatingError should be used.'
                    Extent = $ast.Extent
                    RuleName = $myinvocation.MyCommand.Name
                    Severity = 'Warning'
                }
            }
        }
    }
}

Rule suppression and Extent

Individual rules may be suppressed by using the System.Diagnostics.CodeAnalysis.SuppressMessage type. The rule which should be supressed will is the first argument, the second (mandatory) argument is often an empty string.

The PSAvoidWriteErrorStop rule defined above might be suppressed in specific cases.

[System.Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidWriteErrorStop', '')]
param ( )

ScriptExtent vs InternalScriptExtent

PSScriptAnalyzer bases the descision to suppress a message on the StartOffset and EndOffset defined by the Extent. That is, a region of a script.

The Extent object exposed by AST is an InternalScriptExtent object, as shown by running the command below:

{ 'script' }.Ast.Extent.GetType()

The System.Management.Automation assembly includes a ScriptExtent
type. This type allows the creation of a user-defined Extent which can be returned in a diagnostic record.

An instance of the ScriptExtent may be created as follows:

using namespace System.Management.Automation.Language

$line = 'Write-Host "Hello world"'
$startPosition = [ScriptPosition]::new(
    'ScriptName.ps1',
    1,    # scriptLineNumber
    0,    # offsetInLine
    $line
)
$endPosition = [ScriptPosition]::new(
    'ScriptName.ps1',
    1,               # scriptLineNumber
    $line.Length,    # offsetInLine
    $line
)
[ScriptExtent]::new($startPosition, $endPosition)

Unfortunately, ScriptExtent differs from InternalScriptExtent in one important way: The StartOffset and EndOffset for ScriptExtent are hard-set to 0 (and cannot be modified).

Get-Member shows these cannot be changed:

PS> $extent = [ScriptExtent]::new($startPosition, $endPosition)
PS> $extent | Get-Member StartOffset

   TypeName: System.Management.Automation.Language.ScriptExtent

Name        MemberType Definition
----        ---------- ----------
StartOffset Property   int StartOffset {get;}

The output shows that the get accessor is available, but not set. Add-Member cannot override this because a change using Add-Member would only apply within PowerShell, PSScriptAnalyzer is a binary (compiled C#) module.

Making a better Extent

Tracking the problem back using a decomiler shows the problem is repeated in the ScriptPosition type.

To correct the problem, and allow rules with user-defined extents to be suppressed, a new Extent class must be created. This includes the creation of a supporting ScriptPosition class.

The classes below are based on the InternalScriptExtent code observed using ilspy. When defining a start and end position the offset should include any line break characters.

using System;
using System.Globalization;
using System.Management.Automation.Language;

namespace ScriptAnalyzer.PracticeAndStyle
{
	public sealed class ExtendedScriptPosition : IScriptPosition
	{
		private readonly int _offsetInLine;
		private readonly int _scriptLineNumber;
		private readonly string _scriptName;
		private readonly string _line;
		private readonly string _fullScript;
		private readonly int _offset;

		public string File
		{
			get
			{
				return this._scriptName;
			}
		}

		public int LineNumber
		{
			get
			{
				return this._scriptLineNumber;
			}
		}

		public int ColumnNumber
		{
			get
			{
				return this._offsetInLine;
			}
		}

		public int Offset
		{
			get
			{
				return this._offset;
			}
		}

		public string Line
		{
			get
			{
				return this._line;
			}
		}

		public ExtendedScriptPosition(string scriptName, int scriptLineNumber, int offsetInLine, string line, int offset)
		{
			this._scriptName = scriptName;
			this._scriptLineNumber = scriptLineNumber;
			this._offsetInLine = offsetInLine;
			this._offset = offset;
			if (string.IsNullOrEmpty(line))
			{
				this._line = string.Empty;
				return;
			}
			this._line = line;
		}

		public ExtendedScriptPosition(string scriptName, int scriptLineNumber, int offsetInLine, string line, int offset, string fullScript) : this(scriptName, scriptLineNumber, offsetInLine, line, offset)
		{
			this._fullScript = fullScript;
		}

		public string GetFullScript()
		{
			return this._fullScript;
		}
	}

	public sealed class ExtendedScriptExtent : IScriptExtent
	{
		private ExtendedScriptPosition _startPosition;
		private ExtendedScriptPosition _endPosition;

		public string File
		{
			get
			{
				return this._startPosition.File;
			}
		}

		public IScriptPosition StartScriptPosition
		{
			get
			{
				return this._startPosition;
			}
		}

		public IScriptPosition EndScriptPosition
		{
			get
			{
				return this._endPosition;
			}
		}

		public int StartLineNumber
		{
			get
			{
				return this._startPosition.LineNumber;
			}
		}

		public int StartColumnNumber
		{
			get
			{
				return this._startPosition.ColumnNumber;
			}
		}

		public int EndLineNumber
		{
			get
			{
				return this._endPosition.LineNumber;
			}
		}

		public int EndColumnNumber
		{
			get
			{
				return this._endPosition.ColumnNumber;
			}
		}

		public int StartOffset
		{
			get
			{
				return this._startPosition.Offset;
			}
		}

		public int EndOffset
		{
			get
			{
				return this._endPosition.Offset;
			}
		}

		public string Text
		{
			get
			{
				if (this.EndColumnNumber <= 0)
				{
					return string.Empty;
				}
				if (this.StartLineNumber == this.EndLineNumber)
				{
					return this._startPosition.Line.Substring(this._startPosition.ColumnNumber - 1, this._endPosition.ColumnNumber - this._startPosition.ColumnNumber);
				}
				return string.Format(CultureInfo.InvariantCulture, "{0}...{1}", new object[]
				{
					this._startPosition.Line.Substring(this._startPosition.ColumnNumber),
					this._endPosition.Line.Substring(0, this._endPosition.ColumnNumber)
				});
			}
		}

		public ExtendedScriptExtent(ExtendedScriptPosition startPosition, ExtendedScriptPosition endPosition)
		{
			this._startPosition = startPosition;
			this._endPosition = endPosition;
		}
	}
}

The class above may be used with "Add-Type -TypeDefinition", or compiled into a dll:

Add-Type -TypeDefinition $definition -OutputType Library -OutputAssembly C:\Temp\UserExtent.dll
Coding conventions and PSScriptAnalyzer
Share this