Skip to content
Open
89 changes: 89 additions & 0 deletions Rules/AvoidDynamicallyCreatingVariableNames.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// 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.Linq;
using System.Management.Automation.Language;

#if !CORECLR
using System.ComponentModel.Composition;
#endif

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
{
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif

/// <summary>
/// Rule that informs the user when they create variables with dynamic names in the general variable scope.
/// This might lead to conflicts with other variables.
/// </summary>
public class AvoidDynamicallyCreatingVariableNames : IScriptRule
{
/// <summary>
/// Analyzes the PowerShell AST for uses of "New-Variable" command with a dynamic name argument.
/// </summary>
/// <param name="ast">The PowerShell Abstract Syntax Tree to analyze.</param>
/// <param name="fileName">The name of the file being analyzed (for diagnostic reporting).</param>
/// <returns>A collection of diagnostic records for each violation.</returns>

readonly HashSet<string> cmdList = Helper.Instance.CmdletNameAndAliases("New-Variable").ToHashSet(StringComparer.OrdinalIgnoreCase);
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);

// Find all "New-Variable" commands in the Ast
IEnumerable<CommandAst> newVariableAsts = ast.FindAll(testAst =>
testAst is CommandAst cmdAst &&
cmdList.Contains(cmdAst.GetCommandName()),
true
).Cast<CommandAst>();

foreach (CommandAst newVariableAst in newVariableAsts)
{
// 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 (nameBindingResult.ConstantValue != null) { continue; }
string variableName = nameBindingResult.Value.ToString();
Comment thread
iRon7 marked this conversation as resolved.
if (variableName.StartsWith("\"") && variableName.EndsWith("\""))
{
variableName = variableName.Substring(1, variableName.Length - 2);
}
yield return new DiagnosticRecord(
string.Format(
CultureInfo.CurrentCulture,
Strings.AvoidDynamicallyCreatingVariableNamesError,
variableName),
newVariableAst.Parent.Extent,
Comment thread
iRon7 marked this conversation as resolved.
Outdated
GetName(),
DiagnosticSeverity.Information,
fileName,
variableName
);
}
}

public string GetCommonName() => Strings.AvoidDynamicallyCreatingVariableNamesCommonName;

public string GetDescription() => Strings.AvoidDynamicallyCreatingVariableNamesDescription;

public string GetName() => string.Format(
CultureInfo.CurrentCulture,
Strings.NameSpaceFormat,
GetSourceName(),
Strings.AvoidDynamicallyCreatingVariableNamesName);

public RuleSeverity GetSeverity() => RuleSeverity.Information;

public string GetSourceName() => Strings.SourceName;

public SourceType GetSourceType() => SourceType.Builtin;
}
}
12 changes: 12 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,18 @@
<data name="UseCompatibleTypesTypeAcceleratorError" xml:space="preserve">
<value>The type accelerator '{0}' is not available by default in PowerShell version '{1}' on platform '{2}'</value>
</data>
<data name="AvoidDynamicallyCreatingVariableNamesName" xml:space="preserve">
<value>AvoidDynamicallyCreatingVariableNames</value>
</data>
<data name="AvoidDynamicallyCreatingVariableNamesCommonName" xml:space="preserve">
<value>Avoid dynamically creating variable names</value>
</data>
<data name="AvoidDynamicallyCreatingVariableNamesDescription" xml:space="preserve">
<value>Do not create variables with a dynamic name, this might introduce conflicts with other variables and is difficult to maintain.</value>
</data>
<data name="AvoidDynamicallyCreatingVariableNamesError" xml:space="preserve">
<value>'{0}' is a dynamic variable name. Please, avoid creating variables with a dynamic name</value>
Comment thread
iRon7 marked this conversation as resolved.
Outdated
</data>
<data name="AvoidGlobalFunctionsCommonName" xml:space="preserve">
<value>Avoid global functions and aliases</value>
</data>
Expand Down
141 changes: 141 additions & 0 deletions Tests/Rules/AvoidDynamicallyCreatingVariableNames.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')]
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingCmdletAliases', 'nv', Justification = 'For test purposes')]
param()

BeforeAll {
$ruleName = "PSAvoidDynamicallyCreatingVariableNames"
$ruleMessage = "'{0}' is a dynamic variable name. Please, avoid creating variables with a dynamic name"
}

Describe "AvoidDynamicallyCreatingVariableNames" {
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 Information
$violations.Extent.Text | Should -Be {New-Variable -Name $Test}.ToString()
$violations.Message | Should -Be ($ruleMessage -f '$Test')
}

It "Using alias" {
$scriptDefinition = { nv -Name $Test }.ToString()
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
$violations.Count | Should -Be 1
$violations.Severity | Should -Be Information
$violations.Extent.Text | Should -Be {nv -Name $Test}.ToString()
$violations.Message | Should -Be ($ruleMessage -f '$Test')
}

It "Using uppercase" {
$scriptDefinition = { NEW-VARIABLE -Name $Test }.ToString()
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
$violations.Count | Should -Be 1
$violations.Severity | Should -Be Information
$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 Information
$violations.Extent.Text | Should -Be {New-Variable -Name "My$_" -Value ($i++)}.ToString()
$violations.Message | Should -Be ($ruleMessage -f 'My$_')
}

It "Unquoted positional binding" {
$scriptDefinition = {
$myVarName = 'foo'
New-Variable $myVarName
}.ToString()
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
$violations.Count | Should -Be 1
$violations.Severity | Should -Be Information
$violations.Extent.Text | Should -Be {New-Variable $myVarName}.ToString()
$violations.Message | Should -Be ($ruleMessage -f '$myVarName')
}

It "Quoted positional binding" {
$scriptDefinition = {
'One', 'Two', 'Three' | ForEach-Object -Begin { $i = 1 } -Process {
New-Variable "My$_" ($i++)
}
$MyTwo # returns 2
}.ToString()
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
$violations.Count | Should -Be 1
$violations.Severity | Should -Be Information
$violations.Extent.Text | Should -Be {New-Variable "My$_" ($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
}

It "Verbatim (single quoted) name with dollar sign" {
$scriptDefinition = {
New-Variable -Name '$Sign1'
New-Variable -Name '$Sign2' -Value 'Dollar'
}.ToString()
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
$violations | Should -BeNullOrEmpty
}
}

Context "Suppressed" {
It "Basic dynamic variable name" {
$scriptDefinition = {
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidDynamicallyCreatingVariableNames', '$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('PSAvoidDynamicallyCreatingVariableNames', '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
}
}
}
51 changes: 51 additions & 0 deletions docs/Rules/AvoidDynamicallyCreatingVariableNames.md
Original file line number Diff line number Diff line change
@@ -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
Comment thread
iRon7 marked this conversation as resolved.
Outdated

**Severity Level: Information**

## Description

Do not create variables with a dynamic name, 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.:
Comment thread
iRon7 marked this conversation as resolved.
Outdated

```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
```
1 change: 1 addition & 0 deletions docs/Rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
| [AvoidDynamicallyCreatingVariableNames](./AvoidDynamicallyCreatingVariableNames.md) | Information | Yes | |
Comment thread
iRon7 marked this conversation as resolved.
Outdated
| [AvoidUsingEmptyCatchBlock](./AvoidUsingEmptyCatchBlock.md) | Warning | Yes | |
| [AvoidUsingInvokeExpression](./AvoidUsingInvokeExpression.md) | Warning | Yes | |
| [AvoidUsingPlainTextForPassword](./AvoidUsingPlainTextForPassword.md) | Warning | Yes | |
Expand Down
Loading