Skip to content

Commit 5d529a3

Browse files
bergmeisterrjmholt
bergmeister
and
rjmholt
authored
Add more automatic variables to PSAvoidAssignmentToAutomaticVariables that are not read-only but should still not be assigned to in most cases (#1394)
* Add more automatic variables to PSAvoidAssignmentToAutomaticVariables that are not read-only but should still not be assigned to in most cases * Update Rules/AvoidAssignmentToAutomaticVariable.cs Co-Authored-By: Robert Holt <[email protected]> * rename variable to _writableAutomaticVariables Co-authored-by: Robert Holt <[email protected]>
1 parent b8fec67 commit 5d529a3

4 files changed

+81
-21
lines changed

‎Rules/AvoidAssignmentToAutomaticVariable.cs

+23
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ public class AvoidAssignmentToAutomaticVariable : IScriptRule
3333
"IsCoreCLR", "IsLinux", "IsMacOS", "IsWindows"
3434
};
3535

36+
private static readonly IReadOnlyList<string> _writableAutomaticVariables = new List<string>()
37+
{
38+
// Attempting to assign to any of those could cause issues, only in some special cases could assignment be by design
39+
"_", "AllNodes", "Args", "ConsoleFilename", "Event", "EventArgs", "EventSubscriber", "ForEach", "Input", "Matches", "MyInvocation",
40+
"NestedPromptLevel", "Profile", "PSBoundParameters", "PsCmdlet", "PSCommandPath", "PSDebugContext",
41+
"PSItem", "PSScriptRoot", "PSSenderInfo", "Pwd", "PSCommandPath", "ReportErrorShowExceptionClass",
42+
"ReportErrorShowInnerException", "ReportErrorShowSource", "ReportErrorShowStackTrace", "Sender",
43+
"StackTrace", "This"
44+
};
45+
3646
/// <summary>
3747
/// Checks for assignment to automatic variables.
3848
/// <param name="ast">The script's ast</param>
@@ -61,6 +71,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
6171
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
6272
variableExpressionAst.Extent, GetName(), severity, fileName);
6373
}
74+
75+
if (_writableAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
76+
{
77+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToWritableAutomaticVariableError, variableName),
78+
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
79+
}
6480
}
6581

6682
IEnumerable<Ast> parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);
@@ -79,12 +95,19 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7995
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
8096
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
8197
}
98+
8299
if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
83100
{
84101
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
85102
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
86103
variableExpressionAst.Extent, GetName(), severity, fileName);
87104
}
105+
106+
if (_writableAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
107+
{
108+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToWritableAutomaticVariableError, variableName),
109+
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
110+
}
88111
}
89112
}
90113

‎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
@@ -1104,6 +1104,9 @@
11041104
<data name="UseProcessBlockForPipelineCommandName" xml:space="preserve">
11051105
<value>UseProcessBlockForPipelineCommand</value>
11061106
</data>
1107+
<data name="AvoidAssignmentToWritableAutomaticVariableError" xml:space="preserve">
1108+
<value>The Variable '{0}' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.</value>
1109+
</data>
11071110
<data name="UseConsistentWhitespaceErrorSpaceBetweenParameter" xml:space="preserve">
11081111
<value>Use only 1 whitespace between parameter names or values.</value>
11091112
</data>

‎Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1

+46-21
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,54 @@ Describe "AvoidAssignmentToAutomaticVariables" {
99
$excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Error'
1010
}
1111

12-
$testCases_ReadOnlyVariables = @(
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
12+
$testCases_AutomaticVariables = @(
13+
@{ VariableName = '?'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
14+
@{ VariableName = 'Error' ; ExpectedSeverity = 'Error'; IsReadOnly = $true }
15+
@{ VariableName = 'ExecutionContext'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
16+
@{ VariableName = 'false'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
17+
@{ VariableName = 'Home'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
18+
@{ VariableName = 'Host'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
19+
@{ VariableName = 'PID'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
20+
@{ VariableName = 'PSCulture'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
21+
@{ VariableName = 'PSEdition'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
22+
@{ VariableName = 'PSHome'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
23+
@{ VariableName = 'PSUICulture'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
24+
@{ VariableName = 'PSVersionTable'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
25+
@{ VariableName = 'ShellId'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
26+
@{ VariableName = 'true'; ExpectedSeverity = 'Error'; IsReadOnly = $true }
27+
# Variables introduced only in PowerShell 6+ have a Severity of Warning only
2828
@{ VariableName = 'IsCoreCLR'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
2929
@{ VariableName = 'IsLinux'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
3030
@{ VariableName = 'IsMacOS'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
3131
@{ VariableName = 'IsWindows'; ExpectedSeverity = $excpectedSeverityForAutomaticVariablesInPowerShell6; OnlyPresentInCoreClr = $true }
32+
@{ VariableName = '_'; ExpectedSeverity = 'Warning' }
33+
@{ VariableName = 'AllNodes'; ExpectedSeverity = 'Warning' }
34+
@{ VariableName = 'Args'; ExpectedSeverity = 'Warning' }
35+
@{ VariableName = 'ConsoleFilename'; ExpectedSeverity = 'Warning' }
36+
@{ VariableName = 'Event'; ExpectedSeverity = 'Warning' }
37+
@{ VariableName = 'EventArgs'; ExpectedSeverity = 'Warning' }
38+
@{ VariableName = 'EventSubscriber'; ExpectedSeverity = 'Warning' }
39+
@{ VariableName = 'ForEach'; ExpectedSeverity = 'Warning' }
40+
@{ VariableName = 'Input'; ExpectedSeverity = 'Warning' }
41+
@{ VariableName = 'Matches'; ExpectedSeverity = 'Warning' }
42+
@{ VariableName = 'MyInvocation'; ExpectedSeverity = 'Warning' }
43+
@{ VariableName = 'NestedPromptLevel'; ExpectedSeverity = 'Warning' }
44+
@{ VariableName = 'Profile'; ExpectedSeverity = 'Warning' }
45+
@{ VariableName = 'PSBoundParameters'; ExpectedSeverity = 'Warning' }
46+
@{ VariableName = 'PsCmdlet'; ExpectedSeverity = 'Warning' }
47+
@{ VariableName = 'PSCommandPath'; ExpectedSeverity = 'Warning' }
48+
@{ VariableName = 'ReportErrorShowExceptionClass'; ExpectedSeverity = 'Warning' }
49+
@{ VariableName = 'ReportErrorShowInnerException'; ExpectedSeverity = 'Warning' }
50+
@{ VariableName = 'ReportErrorShowSource'; ExpectedSeverity = 'Warning' }
51+
@{ VariableName = 'ReportErrorShowStackTrace'; ExpectedSeverity = 'Warning' }
52+
@{ VariableName = 'Sender'; ExpectedSeverity = 'Warning' }
53+
@{ VariableName = 'StackTrace'; ExpectedSeverity = 'Warning' }
54+
@{ VariableName = 'This'; ExpectedSeverity = 'Warning' }
3255
)
3356

34-
It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_ReadOnlyVariables {
57+
$testCases_ReadOnlyAutomaticVariables = $testCases_AutomaticVariables | Where-Object { $_.IsReadonly }
58+
59+
It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
3560
param ($VariableName, $ExpectedSeverity)
3661

3762
$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments
@@ -40,7 +65,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
4065
$warnings.RuleName | Should -Be $ruleName
4166
}
4267

43-
It "Using Variable <VariableName> as parameter name produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables {
68+
It "Using Variable <VariableName> as parameter name produces warning of Severity error" -TestCases $testCases_AutomaticVariables {
4469
param ($VariableName, $ExpectedSeverity)
4570

4671
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" -ExcludeRule PSReviewUnusedParameter
@@ -49,7 +74,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
4974
$warnings.RuleName | Should -Be $ruleName
5075
}
5176

52-
It "Using Variable <VariableName> as parameter name in param block produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables {
77+
It "Using Variable <VariableName> as parameter name in param block produces warning of Severity error" -TestCases $testCases_AutomaticVariables {
5378
param ($VariableName, $ExpectedSeverity)
5479

5580
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}"
@@ -77,8 +102,8 @@ Describe "AvoidAssignmentToAutomaticVariables" {
77102
$warnings.Count | Should -Be 0
78103
}
79104

80-
It "Setting Variable <VariableName> throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables {
81-
param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr)
105+
It "Setting Variable <VariableName> throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyAutomaticVariables {
106+
param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr, [bool] $IsReadOnly)
82107

83108
if ($OnlyPresentInCoreClr -and !$IsCoreCLR)
84109
{

0 commit comments

Comments
 (0)