Skip to content

Flag unused variables in multiple assignments in UseDeclaredVarsMoreThanAssignments #2227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ethan Aldrich (0x000NULL) wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Flag unused variables in multiple assignments in UseDeclaredVarsMoreThanAssignments #2227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53cc1aa
Flag unused variables in multiple assignments
0x000NULL Oct 18, 2026
f875f8f
Only treat assigned variables of a multiple assignment as writes
0x000NULL Oct 19, 2026
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
62 changes: 43 additions & 19 deletions Rules/UseDeclaredVarsMoreThanAssignments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,7 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip

foreach (AssignmentStatementAst assignmentAst in assignmentAsts)
{
// Only checks for the case where lhs is a variable. Ignore things like $foo.property
VariableExpressionAst assignmentVarAst = assignmentAst.Left as VariableExpressionAst;

if (assignmentVarAst == null)
{
// If the variable is declared in a strongly typed way, e.g. [string]$s = 'foo' then the type is ConvertExpressionAst.
// Therefore we need to the VariableExpressionAst from its Child property.
var assignmentVarAstAsConvertExpressionAst = assignmentAst.Left as ConvertExpressionAst;
if (assignmentVarAstAsConvertExpressionAst != null && assignmentVarAstAsConvertExpressionAst.Child != null)
{
assignmentVarAst = assignmentVarAstAsConvertExpressionAst.Child as VariableExpressionAst;
}
}

if (assignmentVarAst != null)
foreach (VariableExpressionAst assignmentVarAst in GetAssignedVariables(assignmentAst.Left))
{
// Ignore if variable is global or environment variable or scope is drive qualified variable
if (!Helper.Instance.IsVariableGlobalOrEnvironment(assignmentVarAst)
Expand All @@ -166,21 +152,21 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip

if (assignmentsDictionary_OrdinalIgnoreCase.ContainsKey(varKey))
{
varsInAssignment = assignmentsDictionary_OrdinalIgnoreCase[varKey].Left.FindAll(testAst => testAst is VariableExpressionAst, true);
varsInAssignment = GetAssignedVariables(assignmentsDictionary_OrdinalIgnoreCase[varKey].Left);

// Checks if this variableAst is part of the logged assignment
foreach (VariableExpressionAst varInAssignment in varsInAssignment)
{
// Try casting to AssignmentStatementAst to be able to catch case where a variable is assigned more than once (https://github.com/PowerShell/PSScriptAnalyzer/issues/833)
var varInAssignmentAsStatementAst = varInAssignment.Parent as AssignmentStatementAst;
var varAstAsAssignmentStatementAst = varAst.Parent as AssignmentStatementAst;
var varInAssignmentAsStatementAst = GetAssignmentStatementOfTarget(varInAssignment);
var varAstAsAssignmentStatementAst = GetAssignmentStatementOfTarget(varAst);
if (varAstAsAssignmentStatementAst != null)
{
if (varAstAsAssignmentStatementAst.Operator == TokenKind.Equals)
{
if (varInAssignmentAsStatementAst != null)
{
inAssignment = varInAssignmentAsStatementAst.Left.Extent.Text.Equals(varAstAsAssignmentStatementAst.Left.Extent.Text, StringComparison.OrdinalIgnoreCase);
inAssignment = varInAssignment.Extent.Text.Equals(varAst.Extent.Text, StringComparison.OrdinalIgnoreCase);
}
else
{
Expand All @@ -192,6 +178,12 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip
{
inAssignment = varInAssignment.Equals(varAst);
}

// A multiple assignment has several variables on the lhs, so stop at the first match
if (inAssignment)
{
break;
Comment on lines +182 to +185
}
}

if (!inAssignment)
Expand Down Expand Up @@ -222,6 +214,38 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip
}
}

/// <summary>
/// Returns the variables assigned by the lhs of an assignment, e.g. $a, [string]$s or each of $a, $b.
/// Ignores things like $foo.property and $foo[$i], including the variables they read.
/// </summary>
private static IEnumerable<VariableExpressionAst> GetAssignedVariables(ExpressionAst left)
{
// Assigning multiple variables, e.g. $a, $b = 1, 2, assigns each element of the array literal
IEnumerable<ExpressionAst> leftAsts = left is ArrayLiteralAst arrayLiteralAst
? arrayLiteralAst.Elements
: (IEnumerable<ExpressionAst>)new[] { left };

foreach (ExpressionAst leftAst in leftAsts)
{
// If the variable is declared in a strongly typed way, e.g. [string]$s = 'foo' then the type is ConvertExpressionAst.
// Therefore we need to the VariableExpressionAst from its Child property.
VariableExpressionAst variableAst = leftAst as VariableExpressionAst ?? (leftAst as ConvertExpressionAst)?.Child as VariableExpressionAst;
if (variableAst != null)
{
yield return variableAst;
}
}
}

/// <summary>
/// Returns the assignment whose lhs is this variable, or contains it as an element as in $a, $b = 1, 2.
/// </summary>
private static AssignmentStatementAst GetAssignmentStatementOfTarget(VariableExpressionAst variableAst)
{
return variableAst.Parent as AssignmentStatementAst
?? (variableAst.Parent as ArrayLiteralAst)?.Parent as AssignmentStatementAst;
}

/// <summary>
/// Detects variables retrieved by usage of Get-Variable and remove those
/// variables from the entries in <paramref name="assignmentsDictionary_OrdinalIgnoreCase"/>.
Expand Down
31 changes: 31 additions & 0 deletions Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,27 @@ function MyFunc2() {
$results | Get-Count | Should -Be 1
$results[0].Extent | Should -Be '$mySecondvar'
}

It "flags each unused variable of a multiple assignment" {
Invoke-ScriptAnalyzer -ScriptDefinition '$a, $b = $null' -IncludeRule $violationName | `
Get-Count | `
Should -Be 2
}

It "flags only the unused variable of a multiple assignment" {
$results = Invoke-ScriptAnalyzer -ScriptDefinition '$a, $b = 1, 2; $b' -IncludeRule $violationName
$results | Get-Count | Should -Be 1
$results[0].RuleSuppressionID | Should -Be 'a'
}

It "flags variables of a multiple assignment that are only assigned again" -TestCases @(
@{ ScriptDefinition = '$a, $b = 1, 2; $a = 3' }
@{ ScriptDefinition = '$a, $b = 1, 2; $a, $b = 3, 4' }
) {
param($ScriptDefinition)
$results = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $violationName
$results.RuleSuppressionID | Sort-Object | Should -Be @('a', 'b')
}
}

Context "When there are no violations" {
Expand All @@ -139,6 +160,16 @@ function MyFunc2() {
$results.Count | Should -Be 0
}

It "Does not flag variables of a multiple assignment that are used" {
$results = Invoke-ScriptAnalyzer -ScriptDefinition '$a, $b = 1, 2; $a + $b' -IncludeRule $violationName
$results.Count | Should -Be 0
}

It "Does not flag a variable of a multiple assignment that is used in an index of another target" {
$results = Invoke-ScriptAnalyzer -ScriptDefinition 'param($items); $index, $items[$index] = 0, 42' -IncludeRule $violationName
$results.Count | Should -Be 0
}

It "Does not flag drive qualified variables such as env" {
$results = Invoke-ScriptAnalyzer -ScriptDefinition '$env:foo = 1; function foo(){ $env:bar = 42 }'
$results.Count | Should -Be 0
Expand Down