Skip to content

Add warnings about read-only automatic variables introduced in PowerShell 6.0 #917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 9, 2018
Merged

Add warnings about read-only automatic variables introduced in PowerShell 6.0 #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eb73985
Add warnings about readonly variables introduced in PowerShell 6.0
bergmeister Mar 3, 2018
178711c
use custom message for variables that only apply to ps 6.0
bergmeister Mar 3, 2018
c55f39c
Remove redundant quotes in IT block because Pester adds them now auto…
bergmeister Mar 3, 2018
5fda73d
Make it throw an Error when PSSA is executed on PSCore
bergmeister Mar 5, 2018
bbc8f3d
address pr comments
bergmeister Mar 23, 2018
abd4f61
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Mar 23, 2018
d81a5a8
exclude PSUseDeclaredVarsMoreThanAssignments rule for tests
bergmeister Mar 23, 2018
a5c023a
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Mar 30, 2018
ad56c52
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Apr 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions Rules/AvoidAssignmentToAutomaticVariable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
public class AvoidAssignmentToAutomaticVariable : IScriptRule
{
private static readonly IList<string> _readOnlyAutomaticVariables = new List<string>()
{
// Attempting to assign to any of those read-only variable would result in an error at runtime
"?", "true", "false", "Host", "PSCulture", "Error", "ExecutionContext", "Home", "PID", "PSEdition", "PSHome", "PSUICulture", "PSVersionTable", "ShellId"
};
{
// Attempting to assign to any of those read-only variable would result in an error at runtime
"?", "true", "false", "Host", "PSCulture", "Error", "ExecutionContext", "Home", "PID", "PSEdition", "PSHome", "PSUICulture", "PSVersionTable", "ShellId"
};

private static readonly IList<string> _readOnlyAutomaticVariablesIntroducedInVersion6_0 = new List<string>()
{
// Attempting to assign to any of those read-only variable will result in an error at runtime
"IsCoreCLR", "IsLinux", "IsMacOS", "IsWindows"
};

/// <summary>
/// Checks for assignment to automatic variables.
Expand All @@ -47,6 +53,13 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}

if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
{
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
variableExpressionAst.Extent, GetName(), severity, fileName);
}
}

IEnumerable<Ast> parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);
Expand All @@ -55,12 +68,33 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
var variableExpressionAst = parameterAst.Find(testAst => testAst is VariableExpressionAst, searchNestedScriptBlocks: false) as VariableExpressionAst;
var variableName = variableExpressionAst.VariablePath.UserPath;
// also check the parent to exclude parameter attributes such as '[Parameter(Mandatory=$true)]' where the read-only variable $true appears.
if (_readOnlyAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase) && !(variableExpressionAst.Parent is NamedAttributeArgumentAst))
if (variableExpressionAst.Parent is NamedAttributeArgumentAst)
{
continue;
}

if (_readOnlyAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
{
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
}
if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
{
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
variableExpressionAst.Extent, GetName(), severity, fileName);
}
}
}

private bool IsPowerShellVersion6OrGreater()
{
var psVersion = Helper.Instance.GetPSVersion();
if (psVersion.Major >= 6)
{
return true;
}
return false;
}

/// <summary>
Expand Down
9 changes: 9 additions & 0 deletions Rules/Strings.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Rules/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,9 @@
<data name="AvoidAssignmentToAutomaticVariableName" xml:space="preserve">
<value>AvoidAssignmentToAutomaticVariable</value>
</data>
<data name="AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error" xml:space="preserve">
<value>Starting from PowerShell 6.0, the Variable '{0}' cannot be assigned any more since it is a readonly automatic variable that is built into PowerShell, please use a different name.</value>
</data>
<data name="AvoidUsingCmdletAliasesMissingGetPrefixError" xml:space="preserve">
<value>'{0}' is implicitly aliasing '{1}' because it is missing the 'Get-' prefix. This can introduce possible problems and make scripts hard to maintain. Please consider changing command to its full name.</value>
</data>
Expand Down
80 changes: 50 additions & 30 deletions Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,55 +3,75 @@
Describe "AvoidAssignmentToAutomaticVariables" {
Context "ReadOnly Variables" {

$readOnlyVariableSeverity = "Error"
$excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Warning'
if ($PSVersionTable.PSVersion.Major -ge 6)
{
$excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Error'
}

$testCases_ReadOnlyVariables = @(
@{ VariableName = '?' }
@{ VariableName = 'Error' }
@{ VariableName = 'ExecutionContext' }
@{ VariableName = 'false' }
@{ VariableName = 'Home' }
@{ VariableName = 'Host' }
@{ VariableName = 'PID' }
@{ VariableName = 'PSCulture' }
@{ VariableName = 'PSEdition' }
@{ VariableName = 'PSHome' }
@{ VariableName = 'PSUICulture' }
@{ VariableName = 'PSVersionTable' }
@{ VariableName = 'ShellId' }
@{ VariableName = 'true' }
@{ VariableName = '?'; ExpectedSeverity = 'Error'; }
@{ VariableName = 'Error' ; ExpectedSeverity = 'Error' }
@{ VariableName = 'ExecutionContext'; ExpectedSeverity = 'Error' }
@{ VariableName = 'false'; ExpectedSeverity = 'Error' }
@{ VariableName = 'Home'; ExpectedSeverity = 'Error' }
@{ VariableName = 'Host'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PID'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PSCulture'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PSEdition'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PSHome'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PSUICulture'; ExpectedSeverity = 'Error' }
@{ VariableName = 'PSVersionTable'; ExpectedSeverity = 'Error' }
@{ VariableName = 'ShellId'; ExpectedSeverity = 'Error' }
@{ VariableName = 'true'; ExpectedSeverity = 'Error' }
# Variables introuced only in PowerShell 6.0 have a Severity of Warning only
@{ VariableName = 'IsCoreCLR'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
@{ VariableName = 'IsLinux'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
@{ VariableName = 'IsMacOS'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
@{ VariableName = 'IsWindows'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
)

It "Variable '<VariableName>' produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)
It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName, $ExpectedSeverity)

$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" | Where-Object { $_.RuleName -eq $ruleName }
$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments
$warnings.Count | Should -Be 1
$warnings.Severity | Should -Be $readOnlyVariableSeverity
$warnings.Severity | Should -Be $ExpectedSeverity
$warnings.RuleName | Should -Be $ruleName
}

It "Using Variable '<VariableName>' as parameter name produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)
It "Using Variable <VariableName> as parameter name produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName, $ExpectedSeverity)

[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" | Where-Object {$_.RuleName -eq $ruleName }
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}"
$warnings.Count | Should -Be 1
$warnings.Severity | Should -Be $readOnlyVariableSeverity
$warnings.Severity | Should -Be $ExpectedSeverity
$warnings.RuleName | Should -Be $ruleName
}

It "Using Variable '<VariableName>' as parameter name in param block produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)
It "Using Variable <VariableName> as parameter name in param block produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName, $ExpectedSeverity)

[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}" | Where-Object {$_.RuleName -eq $ruleName }
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}"
$warnings.Count | Should -Be 1
$warnings.Severity | Should -Be $readOnlyVariableSeverity
$warnings.Severity | Should -Be $ExpectedSeverity
$warnings.RuleName | Should -Be $ruleName
}

It "Does not flag parameter attributes" {
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'function foo{Param([Parameter(Mandatory=$true)]$param1)}' | Where-Object { $_.RuleName -eq $ruleName }
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'function foo{Param([Parameter(Mandatory=$true)]$param1)}'
$warnings.Count | Should -Be 0
}

It "Setting Variable '<VariableName>' throws exception to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)
It "Setting Variable <VariableName> throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr)

if ($OnlyPresentInCoreClr -and !$IsCoreCLR)
{
# In this special case we expect it to not throw
Set-Variable -Name $VariableName -Value 'foo'
continue
}

# Setting the $Error variable has the side effect of the ErrorVariable to contain only the exception message string, therefore exclude this case.
# For the library test in WMF 4, assigning a value $PSEdition does not seem to throw an error, therefore this special case is excluded as well.
Expand Down