From 0e873c28fe829b3ea4375a1cda17078ec3db0d02 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Tue, 21 Apr 2026 17:21:35 +0200 Subject: [PATCH 1/2] 1st commit --- Rules/AvoidDynamicVariableNames.cs | 87 +++++++++++++++++++++++++ Rules/Strings.resx | 12 ++++ docs/Rules/AvoidDynamicVariableNames.md | 51 +++++++++++++++ docs/Rules/README.md | 1 + 4 files changed, 151 insertions(+) create mode 100644 Rules/AvoidDynamicVariableNames.cs create mode 100644 docs/Rules/AvoidDynamicVariableNames.md diff --git a/Rules/AvoidDynamicVariableNames.cs b/Rules/AvoidDynamicVariableNames.cs new file mode 100644 index 000000000..a5a661a07 --- /dev/null +++ b/Rules/AvoidDynamicVariableNames.cs @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Management.Automation.Language; +using System.Linq; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that warns when reserved words are used as function names + /// + public class AvoidDynamicVariableNames : IScriptRule + { + /// + /// Analyzes the PowerShell AST for uses of reserved words as function names. + /// + /// The PowerShell Abstract Syntax Tree to analyze. + /// The name of the file being analyzed (for diagnostic reporting). + /// A collection of diagnostic records for each violation. + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Find all FunctionDefinitionAst in the Ast + var newVariableAsts = ast.FindAll(testAst => + testAst is CommandAst cmdAst && + ( + String.Equals(cmdAst.GetCommandName(), "New-Variable", StringComparison.OrdinalIgnoreCase) || + String.Equals(cmdAst.GetCommandName(), "Set-Variable", StringComparison.OrdinalIgnoreCase) + ), + true + ); + + foreach (CommandAst newVariableAst in newVariableAsts) + { + // Use StaticParameterBinder to reliably get parameter values + var bindingResult = StaticParameterBinder.BindCommand(newVariableAst, true); + + // Dynamic parameters return null for the ConstantValue property + if ( + bindingResult.BoundParameters.ContainsKey("Name") && + bindingResult.BoundParameters["Name"] == null + ) + { + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidDynamicVariableNamesError, + newVariableAst.Parent.Extent.Text), + newVariableAst.Parent.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName + ); + } + } + } + + public string GetCommonName() => Strings.AvoidDynamicVariableNamesCommonName; + + public string GetDescription() => Strings.AvoidDynamicVariableNamesDescription; + + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidDynamicVariableNamesName); + + public RuleSeverity GetSeverity() => RuleSeverity.Warning; + + public string GetSourceName() => Strings.SourceName; + + public SourceType GetSourceType() => SourceType.Builtin; + } +} \ No newline at end of file diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..429fb7c63 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -873,6 +873,18 @@ The type accelerator '{0}' is not available by default in PowerShell version '{1}' on platform '{2}' + + AvoidDynamicVariableNames + + + Avoid dynamic variable names + + + Do not dynamically create variable names in the general variable pool, this might introduce conflicts with other variables and is difficult to maintain. + + + Avoid dynamically creating variable names + Avoid global functions and aliases diff --git a/docs/Rules/AvoidDynamicVariableNames.md b/docs/Rules/AvoidDynamicVariableNames.md new file mode 100644 index 000000000..6dce6f83d --- /dev/null +++ b/docs/Rules/AvoidDynamicVariableNames.md @@ -0,0 +1,51 @@ +--- +description: Avoid dynamic variable names, instead use a hash table or similar dictionary type. +ms.date: 04/21/2026 +ms.topic: reference +title: AvoidDynamicVariableNames +--- +# AvoidDynamicVariableNames + +**Severity Level: Warning** + +## Description + +Do not dynamically create variable names in the general variable pool, this might introduce conflicts with other +variables and is difficult to maintain. + +## How + +Use a hash table or similar dictionary type to store values with dynamic keys. + +## Example + +### Wrong + +```powershell +'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + New-Variable -Name "My$_" -Value ($i++) +} +$MyTwo # returns 2 +``` + +### Correct + +```powershell +$My = @{} +'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + $My[$_] = $i++ +} +$My.Two # returns 2 +``` + +When it concerns a specific scope, option or visibility, put the concerned dictionary (hash table) in that +scope, option or visibility. In example, if the values should be read only and available in the script scope, +put the _hash table_ in the script scope and make it read only.: + +```powershell +New-Variable -Name My -Value @{} -Option ReadOnly -Scope Script +'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + $Script:My[$_] = $i++ +} +$Script:My.Two # returns 2 +``` \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..ee82f511f 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -34,6 +34,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidUsingConvertToSecureStringWithPlainText](./AvoidUsingConvertToSecureStringWithPlainText.md) | Error | Yes | | | [AvoidUsingDeprecatedManifestFields](./AvoidUsingDeprecatedManifestFields.md) | Warning | Yes | | | [AvoidUsingDoubleQuotesForConstantString](./AvoidUsingDoubleQuotesForConstantString.md) | Information | No | | +| [AvoidUsingDynamicVariableNames](./AvoidUsingDynamicVariableNames.md) | Warning | Yes | | | [AvoidUsingEmptyCatchBlock](./AvoidUsingEmptyCatchBlock.md) | Warning | Yes | | | [AvoidUsingInvokeExpression](./AvoidUsingInvokeExpression.md) | Warning | Yes | | | [AvoidUsingPlainTextForPassword](./AvoidUsingPlainTextForPassword.md) | Warning | Yes | | From b4aa1a7d2ccc6f06aedb4e059b5538120d5167dd Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 22 Apr 2026 12:21:36 +0200 Subject: [PATCH 2/2] Avoid dynamic variable names rule implementation and tests --- Rules/AvoidDynamicVariableNames.cs | 32 +++---- Rules/Strings.resx | 4 +- .../Rules/AvoidDynamicVariableNames.tests.ps1 | 86 +++++++++++++++++++ docs/Rules/AvoidDynamicVariableNames.md | 4 +- 4 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 Tests/Rules/AvoidDynamicVariableNames.tests.ps1 diff --git a/Rules/AvoidDynamicVariableNames.cs b/Rules/AvoidDynamicVariableNames.cs index a5a661a07..716069eb8 100644 --- a/Rules/AvoidDynamicVariableNames.cs +++ b/Rules/AvoidDynamicVariableNames.cs @@ -47,24 +47,26 @@ testAst is CommandAst cmdAst && { // Use StaticParameterBinder to reliably get parameter values var bindingResult = StaticParameterBinder.BindCommand(newVariableAst, true); - + if (!bindingResult.BoundParameters.ContainsKey("Name")) { continue; } + var nameBindingResult = bindingResult.BoundParameters["Name"]; // Dynamic parameters return null for the ConstantValue property - if ( - bindingResult.BoundParameters.ContainsKey("Name") && - bindingResult.BoundParameters["Name"] == null - ) + if (nameBindingResult.ConstantValue != null) { continue; } + string variableName = nameBindingResult.Value.ToString(); + if (variableName.StartsWith("\"") && variableName.EndsWith("\"")) { - yield return new DiagnosticRecord( - string.Format( - CultureInfo.CurrentCulture, - Strings.AvoidDynamicVariableNamesError, - newVariableAst.Parent.Extent.Text), - newVariableAst.Parent.Extent, - GetName(), - DiagnosticSeverity.Warning, - fileName - ); + variableName = variableName.Substring(1, variableName.Length - 2); } + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidDynamicVariableNamesError, + variableName), + newVariableAst.Parent.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName, + variableName + ); } } diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 429fb7c63..e493cfd91 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -880,10 +880,10 @@ Avoid dynamic variable names - Do not dynamically create variable names in the general variable pool, this might introduce conflicts with other variables and is difficult to maintain. + Do not create variables with a dynamic name, this might introduce conflicts with other variables and is difficult to maintain. - Avoid dynamically creating variable names + '{0}' is a dynamic variable name. Please, avoid creating variables with a dynamic name Avoid global functions and aliases diff --git a/Tests/Rules/AvoidDynamicVariableNames.tests.ps1 b/Tests/Rules/AvoidDynamicVariableNames.tests.ps1 new file mode 100644 index 000000000..19e0f33c6 --- /dev/null +++ b/Tests/Rules/AvoidDynamicVariableNames.tests.ps1 @@ -0,0 +1,86 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')] +param() + +BeforeAll { + $ruleName = "PSAvoidDynamicVariableNames" + $ruleMessage = "'{0}' is a dynamic variable name. Please, avoid creating variables with a dynamic name" +} + +Describe "AvoidDynamicVariableNames" { + Context "Violates" { + It "Basic dynamic variable name" { + $scriptDefinition = { New-Variable -Name $Test }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {New-Variable -Name $Test}.ToString() + $violations.Message | Should -Be ($ruleMessage -f '$Test') + } + It "Common dynamic variable iteration" { + $scriptDefinition = { + 'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + New-Variable -Name "My$_" -Value ($i++) + } + $MyTwo # returns 2 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {New-Variable -Name "My$_" -Value ($i++)}.ToString() + $violations.Message | Should -Be ($ruleMessage -f 'My$_') + } + } + + Context "Compliant" { + It "Common hash table population" { + $scriptDefinition = { + $My = @{} + 'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + $My[$_] = $i++ + } + $My.Two # returns 2 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Scoped hash table population" { + $scriptDefinition = { + New-Variable -Name My -Value @{} -Option ReadOnly -Scope Script + 'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + $Script:My[$_] = $i++ + } + $Script:My.Two # returns 2 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppressed" { + It "Basic dynamic variable name" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidDynamicVariableNames', '$Test', Justification = 'Test')] + Param() + New-Variable -Name $Test + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + It "Common dynamic variable iteration" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidDynamicVariableNames', 'My$_', Justification = 'Test')] + Param() + 'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process { + New-Variable -Name "My$_" -Value ($i++) + } + $MyTwo # returns 2 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidDynamicVariableNames.md b/docs/Rules/AvoidDynamicVariableNames.md index 6dce6f83d..05086fd6a 100644 --- a/docs/Rules/AvoidDynamicVariableNames.md +++ b/docs/Rules/AvoidDynamicVariableNames.md @@ -10,8 +10,8 @@ title: AvoidDynamicVariableNames ## Description -Do not dynamically create variable names in the general variable pool, this might introduce conflicts with other -variables and is difficult to maintain. +Do not create variables with a dynamic name, this might introduce conflicts with +other variables and is difficult to maintain. ## How