Skip to content

Commit 7c1ca74

Browse files
bergmeisterJamesWTruher
bergmeister
authored and
JamesWTruher
committed
Add warnings about read-only automatic variables introduced in PowerShell 6.0 (#917)
* Add warnings about readonly variables introduced in PowerShell 6.0 * use custom message for variables that only apply to ps 6.0 * Remove redundant quotes in IT block because Pester adds them now automatically * Make it throw an Error when PSSA is executed on PSCore * address pr comments * exclude PSUseDeclaredVarsMoreThanAssignments rule for tests
1 parent 2fc3911 commit 7c1ca74

4 files changed

+101
-35
lines changed

‎Rules/AvoidAssignmentToAutomaticVariable.cs

+39-5
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2222
public class AvoidAssignmentToAutomaticVariable : IScriptRule
2323
{
2424
private static readonly IList<string> _readOnlyAutomaticVariables = new List<string>()
25-
{
26-
// Attempting to assign to any of those read-only variable would result in an error at runtime
27-
"?", "true", "false", "Host", "PSCulture", "Error", "ExecutionContext", "Home", "PID", "PSEdition", "PSHome", "PSUICulture", "PSVersionTable", "ShellId"
28-
};
25+
{
26+
// Attempting to assign to any of those read-only variable would result in an error at runtime
27+
"?", "true", "false", "Host", "PSCulture", "Error", "ExecutionContext", "Home", "PID", "PSEdition", "PSHome", "PSUICulture", "PSVersionTable", "ShellId"
28+
};
29+
30+
private static readonly IList<string> _readOnlyAutomaticVariablesIntroducedInVersion6_0 = new List<string>()
31+
{
32+
// Attempting to assign to any of those read-only variable will result in an error at runtime
33+
"IsCoreCLR", "IsLinux", "IsMacOS", "IsWindows"
34+
};
2935

3036
/// <summary>
3137
/// Checks for assignment to automatic variables.
@@ -47,6 +53,13 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4753
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
4854
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
4955
}
56+
57+
if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
58+
{
59+
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
60+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
61+
variableExpressionAst.Extent, GetName(), severity, fileName);
62+
}
5063
}
5164

5265
IEnumerable<Ast> parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);
@@ -55,12 +68,33 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5568
var variableExpressionAst = parameterAst.Find(testAst => testAst is VariableExpressionAst, searchNestedScriptBlocks: false) as VariableExpressionAst;
5669
var variableName = variableExpressionAst.VariablePath.UserPath;
5770
// also check the parent to exclude parameter attributes such as '[Parameter(Mandatory=$true)]' where the read-only variable $true appears.
58-
if (_readOnlyAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase) && !(variableExpressionAst.Parent is NamedAttributeArgumentAst))
71+
if (variableExpressionAst.Parent is NamedAttributeArgumentAst)
72+
{
73+
continue;
74+
}
75+
76+
if (_readOnlyAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
5977
{
6078
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
6179
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
6280
}
81+
if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
82+
{
83+
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
84+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
85+
variableExpressionAst.Extent, GetName(), severity, fileName);
86+
}
87+
}
88+
}
89+
90+
private bool IsPowerShellVersion6OrGreater()
91+
{
92+
var psVersion = Helper.Instance.GetPSVersion();
93+
if (psVersion.Major >= 6)
94+
{
95+
return true;
6396
}
97+
return false;
6498
}
6599

66100
/// <summary>

‎Rules/Strings.Designer.cs

+9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rules/Strings.resx

+3
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,9 @@
10021002
<data name="AvoidAssignmentToAutomaticVariableName" xml:space="preserve">
10031003
<value>AvoidAssignmentToAutomaticVariable</value>
10041004
</data>
1005+
<data name="AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error" xml:space="preserve">
1006+
<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>
1007+
</data>
10051008
<data name="AvoidUsingCmdletAliasesMissingGetPrefixError" xml:space="preserve">
10061009
<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>
10071010
</data>

‎Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1

+50-30
Original file line numberDiff line numberDiff line change
@@ -3,55 +3,75 @@
33
Describe "AvoidAssignmentToAutomaticVariables" {
44
Context "ReadOnly Variables" {
55

6-
$readOnlyVariableSeverity = "Error"
6+
$excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Warning'
7+
if ($PSVersionTable.PSVersion.Major -ge 6)
8+
{
9+
$excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Error'
10+
}
11+
712
$testCases_ReadOnlyVariables = @(
8-
@{ VariableName = '?' }
9-
@{ VariableName = 'Error' }
10-
@{ VariableName = 'ExecutionContext' }
11-
@{ VariableName = 'false' }
12-
@{ VariableName = 'Home' }
13-
@{ VariableName = 'Host' }
14-
@{ VariableName = 'PID' }
15-
@{ VariableName = 'PSCulture' }
16-
@{ VariableName = 'PSEdition' }
17-
@{ VariableName = 'PSHome' }
18-
@{ VariableName = 'PSUICulture' }
19-
@{ VariableName = 'PSVersionTable' }
20-
@{ VariableName = 'ShellId' }
21-
@{ VariableName = 'true' }
13+
@{ VariableName = '?'; ExpectedSeverity = 'Error'; }
14+
@{ VariableName = 'Error' ; ExpectedSeverity = 'Error' }
15+
@{ VariableName = 'ExecutionContext'; ExpectedSeverity = 'Error' }
16+
@{ VariableName = 'false'; ExpectedSeverity = 'Error' }
17+
@{ VariableName = 'Home'; ExpectedSeverity = 'Error' }
18+
@{ VariableName = 'Host'; ExpectedSeverity = 'Error' }
19+
@{ VariableName = 'PID'; ExpectedSeverity = 'Error' }
20+
@{ VariableName = 'PSCulture'; ExpectedSeverity = 'Error' }
21+
@{ VariableName = 'PSEdition'; ExpectedSeverity = 'Error' }
22+
@{ VariableName = 'PSHome'; ExpectedSeverity = 'Error' }
23+
@{ VariableName = 'PSUICulture'; ExpectedSeverity = 'Error' }
24+
@{ VariableName = 'PSVersionTable'; ExpectedSeverity = 'Error' }
25+
@{ VariableName = 'ShellId'; ExpectedSeverity = 'Error' }
26+
@{ VariableName = 'true'; ExpectedSeverity = 'Error' }
27+
# Variables introuced only in PowerShell 6.0 have a Severity of Warning only
28+
@{ VariableName = 'IsCoreCLR'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
29+
@{ VariableName = 'IsLinux'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
30+
@{ VariableName = 'IsMacOS'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
31+
@{ VariableName = 'IsWindows'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
2232
)
2333

24-
It "Variable '<VariableName>' produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
25-
param ($VariableName)
34+
It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_ReadOnlyVariables {
35+
param ($VariableName, $ExpectedSeverity)
2636

27-
$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" | Where-Object { $_.RuleName -eq $ruleName }
37+
$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments
2838
$warnings.Count | Should -Be 1
29-
$warnings.Severity | Should -Be $readOnlyVariableSeverity
39+
$warnings.Severity | Should -Be $ExpectedSeverity
40+
$warnings.RuleName | Should -Be $ruleName
3041
}
3142

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

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

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

43-
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}" | Where-Object {$_.RuleName -eq $ruleName }
55+
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}"
4456
$warnings.Count | Should -Be 1
45-
$warnings.Severity | Should -Be $readOnlyVariableSeverity
57+
$warnings.Severity | Should -Be $ExpectedSeverity
58+
$warnings.RuleName | Should -Be $ruleName
4659
}
4760

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

53-
It "Setting Variable '<VariableName>' throws exception to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables {
54-
param ($VariableName)
66+
It "Setting Variable <VariableName> throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables {
67+
param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr)
68+
69+
if ($OnlyPresentInCoreClr -and !$IsCoreCLR)
70+
{
71+
# In this special case we expect it to not throw
72+
Set-Variable -Name $VariableName -Value 'foo'
73+
continue
74+
}
5575

5676
# Setting the $Error variable has the side effect of the ErrorVariable to contain only the exception message string, therefore exclude this case.
5777
# 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.

0 commit comments

Comments
 (0)