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 = new HashSet<string>(Helper.Instance.CmdletNameAndAliases("New-Variable"), 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.Extent,
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, as 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>
</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: AvoidDynamicallyCreatingVariableNames
---
# AvoidDynamicallyCreatingVariableNames

**Severity Level: Information**

## Description

Do not create variables with a dynamic name, this might introduce conflicts with
other variables and is difficult to maintain.
Comment on lines +13 to +14
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Do not create variables with a dynamic name, this might introduce conflicts with
other variables and is difficult to maintain.
Don't create variables with dynamic names. It also makes the code difficult to understand and can
lead to unexpected behavior if the variable names are not unique or if they collide with existing
variables. A dynamic name is a name constructed using string concatenation or interpolation.
This rule checks for the use of `New-Variable` with a dynamic name.


## How

Use a hash table or similar dictionary type to store values with dynamic keys.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use a hash table or similar dictionary type to store values with dynamic keys.
Use a hash table or similar dictionary type to store values with dynamic keys. When you require a
specific scope, option, or visibility, put the dictionary (hashtable) in that scope and apply the
appropriate option or visibility.


## 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 a specific scope, option, or visibility is required, put the dictionary (hash table) in that
scope and apply the appropriate option or visibility. For 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
```
Comment on lines +41 to +51
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When a specific scope, option, or visibility is required, put the dictionary (hash table) in that
scope and apply the appropriate option or visibility. For 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
```
In this example, you want the values to be read-only and available in the script scope. Put the
hashtable 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
```
## References
- [New-Variable](xref:Microsoft.PowerShell.Utility.New-Variable)

1 change: 1 addition & 0 deletions docs/Rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The PSScriptAnalyzer contains the following rule definitions.
| [AvoidAssignmentToAutomaticVariable](./AvoidAssignmentToAutomaticVariable.md) | Warning | Yes | |
| [AvoidDefaultValueForMandatoryParameter](./AvoidDefaultValueForMandatoryParameter.md) | Warning | Yes | |
| [AvoidDefaultValueSwitchParameter](./AvoidDefaultValueSwitchParameter.md) | Warning | Yes | |
| [AvoidDynamicallyCreatingVariableNames](./AvoidDynamicallyCreatingVariableNames.md) | Information | Yes | |
| [AvoidExclaimOperator](./AvoidExclaimOperator.md) | Warning | No | |
| [AvoidGlobalAliases<sup>1</sup>](./AvoidGlobalAliases.md) | Warning | Yes | |
| [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | |
Expand Down
Loading