Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 thread
iRon7 marked this conversation as resolved.
Outdated

## How

Use a hash table or similar dictionary type to store values with dynamic keys.
Comment thread
iRon7 marked this conversation as resolved.
Outdated

## 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 thread
iRon7 marked this conversation as resolved.
Outdated
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