diff --git a/Rules/AvoidAssignmentToAutomaticVariable.cs b/Rules/AvoidAssignmentToAutomaticVariable.cs index 5f7a859ee..6011300ef 100644 --- a/Rules/AvoidAssignmentToAutomaticVariable.cs +++ b/Rules/AvoidAssignmentToAutomaticVariable.cs @@ -22,10 +22,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class AvoidAssignmentToAutomaticVariable : IScriptRule { private static readonly IList _readOnlyAutomaticVariables = new List() - { - // 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 _readOnlyAutomaticVariablesIntroducedInVersion6_0 = new List() + { + // Attempting to assign to any of those read-only variable will result in an error at runtime + "IsCoreCLR", "IsLinux", "IsMacOS", "IsWindows" + }; /// /// Checks for assignment to automatic variables. @@ -47,6 +53,13 @@ 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); + } } IEnumerable parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true); @@ -55,12 +68,33 @@ public IEnumerable 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; } /// diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 7c0e53ec3..14a58591d 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -142,6 +142,15 @@ internal static string AvoidAssignmentToReadOnlyAutomaticVariableError { } } + /// + /// Looks up a localized string similar to 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.. + /// + internal static string AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error { + get { + return ResourceManager.GetString("AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid Using ComputerName Hardcoded. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 352d5ee3f..e941faff9 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1005,6 +1005,9 @@ AvoidAssignmentToAutomaticVariable + + 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. + '{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. diff --git a/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 b/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 index 79a161d61..55a47cbf0 100644 --- a/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 +++ b/Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1 @@ -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 '' produces warning of severity error" -TestCases $testCases_ReadOnlyVariables { - param ($VariableName) + It "Variable produces warning of Severity " -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 '' as parameter name produces warning of severity error" -TestCases $testCases_ReadOnlyVariables { - param ($VariableName) + It "Using Variable 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 '' as parameter name in param block produces warning of severity error" -TestCases $testCases_ReadOnlyVariables { - param ($VariableName) + It "Using Variable 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 '' throws exception to verify the variables is read-only" -TestCases $testCases_ReadOnlyVariables { - param ($VariableName) + It "Setting Variable 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.