diff --git a/Rules/AvoidAssignmentToAutomaticVariable.cs b/Rules/AvoidAssignmentToAutomaticVariable.cs index b6b427440..1f2a784dd 100644 --- a/Rules/AvoidAssignmentToAutomaticVariable.cs +++ b/Rules/AvoidAssignmentToAutomaticVariable.cs @@ -33,6 +33,16 @@ public class AvoidAssignmentToAutomaticVariable : IScriptRule "IsCoreCLR", "IsLinux", "IsMacOS", "IsWindows" }; + private static readonly IReadOnlyList _writableAutomaticVariables = new List() + { + // Attempting to assign to any of those could cause issues, only in some special cases could assignment be by design + "_", "AllNodes", "Args", "ConsoleFilename", "Event", "EventArgs", "EventSubscriber", "ForEach", "Input", "Matches", "MyInvocation", + "NestedPromptLevel", "Profile", "PSBoundParameters", "PsCmdlet", "PSCommandPath", "PSDebugContext", + "PSItem", "PSScriptRoot", "PSSenderInfo", "Pwd", "PSCommandPath", "ReportErrorShowExceptionClass", + "ReportErrorShowInnerException", "ReportErrorShowSource", "ReportErrorShowStackTrace", "Sender", + "StackTrace", "This" + }; + /// /// Checks for assignment to automatic variables. /// The script's ast @@ -61,6 +71,12 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName), variableExpressionAst.Extent, GetName(), severity, fileName); } + + if (_writableAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase)) + { + yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToWritableAutomaticVariableError, variableName), + variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + } } IEnumerable parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true); @@ -79,12 +95,19 @@ public IEnumerable 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); } + + if (_writableAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase)) + { + yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToWritableAutomaticVariableError, variableName), + variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + } } } diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 8fbcbc368..b4e19e149 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -150,6 +150,15 @@ internal static string AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPow } } + /// + /// Looks up a localized string similar to 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.. + /// + internal static string AvoidAssignmentToWritableAutomaticVariableError { + get { + return ResourceManager.GetString("AvoidAssignmentToWritableAutomaticVariableError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid Using ComputerName Hardcoded. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c2bf6c943..973524259 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1104,6 +1104,9 @@ UseProcessBlockForPipelineCommand + + 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. + Use only 1 whitespace between parameter names or values. diff --git a/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 b/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 index e2a363da5..caf207b8c 100644 --- a/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 +++ b/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 @@ -9,29 +9,54 @@ Describe "AvoidAssignmentToAutomaticVariables" { $excpectedSeverityForAutomaticVariablesInPowerShell6 = 'Error' } - $testCases_ReadOnlyVariables = @( - @{ 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 + $testCases_AutomaticVariables = @( + @{ VariableName = '?'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'Error' ; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'ExecutionContext'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'false'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'Home'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'Host'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PID'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PSCulture'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PSEdition'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PSHome'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PSUICulture'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'PSVersionTable'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'ShellId'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + @{ VariableName = 'true'; ExpectedSeverity = 'Error'; IsReadOnly = $true } + # Variables introduced only in PowerShell 6+ 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 } + @{ VariableName = '_'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'AllNodes'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Args'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ConsoleFilename'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Event'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'EventArgs'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'EventSubscriber'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ForEach'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Input'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Matches'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'MyInvocation'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'NestedPromptLevel'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Profile'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'PSBoundParameters'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'PsCmdlet'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'PSCommandPath'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ReportErrorShowExceptionClass'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ReportErrorShowInnerException'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ReportErrorShowSource'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'ReportErrorShowStackTrace'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'Sender'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'StackTrace'; ExpectedSeverity = 'Warning' } + @{ VariableName = 'This'; ExpectedSeverity = 'Warning' } ) - It "Variable produces warning of Severity " -TestCases $testCases_ReadOnlyVariables { + $testCases_ReadOnlyAutomaticVariables = $testCases_AutomaticVariables | Where-Object { $_.IsReadonly } + + It "Variable produces warning of Severity " -TestCases $testCases_AutomaticVariables { param ($VariableName, $ExpectedSeverity) $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments @@ -40,7 +65,7 @@ Describe "AvoidAssignmentToAutomaticVariables" { $warnings.RuleName | Should -Be $ruleName } - It "Using Variable as parameter name produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables { + It "Using Variable as parameter name produces warning of Severity error" -TestCases $testCases_AutomaticVariables { param ($VariableName, $ExpectedSeverity) [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" @@ -49,7 +74,7 @@ Describe "AvoidAssignmentToAutomaticVariables" { $warnings.RuleName | Should -Be $ruleName } - It "Using Variable as parameter name in param block produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables { + It "Using Variable as parameter name in param block produces warning of Severity error" -TestCases $testCases_AutomaticVariables { param ($VariableName, $ExpectedSeverity) [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}" @@ -77,8 +102,8 @@ Describe "AvoidAssignmentToAutomaticVariables" { $warnings.Count | Should -Be 0 } - It "Setting Variable throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables { - param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr) + It "Setting Variable throws exception in applicable PowerShell version to verify the variables is read-only" -TestCases $testCases_ReadOnlyAutomaticVariables { + param ($VariableName, $ExpectedSeverity, $OnlyPresentInCoreClr, [bool] $IsReadOnly) if ($OnlyPresentInCoreClr -and !$IsCoreCLR) { @@ -86,7 +111,7 @@ Describe "AvoidAssignmentToAutomaticVariables" { 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. if ($VariableName -ne 'Error' -and ($VariableName -ne 'PSEdition' -and $PSVersionTable.PSVersion.Major -ne 4))