From c9f97de464f07f1e69ea4fd70f6019339d7c6b55 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 14 Mar 2018 22:41:49 +0000 Subject: [PATCH 1/4] fix false positives such as '$a=@(); $b | foreach-object { $a+=$c}' because the rule misunderstood += as assignment --- Rules/UseDeclaredVarsMoreThanAssignments.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 32fb4c153..04938ccf7 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -180,13 +180,16 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip // 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; - if (varInAssignmentAsStatementAst != null && varAstAsAssignmentStatementAst != null) + if (varAstAsAssignmentStatementAst != null && varAstAsAssignmentStatementAst.Operator == TokenKind.Equals) { - inAssignment = varInAssignmentAsStatementAst.Left.Extent.Text.Equals(varAstAsAssignmentStatementAst.Left.Extent.Text, StringComparison.OrdinalIgnoreCase); - } - else - { - inAssignment = varInAssignment.Equals(varAst); + if (varInAssignmentAsStatementAst != null) + { + inAssignment = varInAssignmentAsStatementAst.Left.Extent.Text.Equals(varAstAsAssignmentStatementAst.Left.Extent.Text, StringComparison.OrdinalIgnoreCase); + } + else + { + inAssignment = varInAssignment.Equals(varAst); + } } } From 7daf5a4fdb9f639e8b4eb37e4d3f676c59ab9603 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 14 Mar 2018 22:46:52 +0000 Subject: [PATCH 2/4] add test (it turns out a regression happened for 'flags strongly typed variables' --- Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index daa766b06..14c6e135f 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -73,5 +73,10 @@ function MyFunc2() { It "returns no violations" { $noViolations.Count | Should -Be 0 } + + It "Does not flag += operator" { + $results = Invoke-ScriptAnalyzer -ScriptDefinition '$array=@(); $list | ForEach-Object { $array += $c }' | Where-Object { $_.RuleName -eq $violationName } + $results.Count | Should -Be 0 + } } } \ No newline at end of file From bc1c0079cd2dd3cc0c0c7dc35fc8c363cd325ce9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 14 Mar 2018 23:08:09 +0000 Subject: [PATCH 3/4] fix regression for strongly typed variables by being more selective about the if/else decision --- Rules/UseDeclaredVarsMoreThanAssignments.cs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 04938ccf7..2f7392510 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -180,17 +180,24 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip // 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; - if (varAstAsAssignmentStatementAst != null && varAstAsAssignmentStatementAst.Operator == TokenKind.Equals) + if (varAstAsAssignmentStatementAst != null) { - if (varInAssignmentAsStatementAst != null) + if (varAstAsAssignmentStatementAst.Operator == TokenKind.Equals) { - inAssignment = varInAssignmentAsStatementAst.Left.Extent.Text.Equals(varAstAsAssignmentStatementAst.Left.Extent.Text, StringComparison.OrdinalIgnoreCase); - } - else - { - inAssignment = varInAssignment.Equals(varAst); + if (varInAssignmentAsStatementAst != null) + { + inAssignment = varInAssignmentAsStatementAst.Left.Extent.Text.Equals(varAstAsAssignmentStatementAst.Left.Extent.Text, StringComparison.OrdinalIgnoreCase); + } + else + { + inAssignment = varInAssignment.Equals(varAst); + } } } + else + { + inAssignment = varInAssignment.Equals(varAst); + } } if (!inAssignment) From 077d0353bda9417a696d052ea7e17c3a4cd23fb0 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 16 Mar 2018 22:55:47 +0000 Subject: [PATCH 4/4] add additional test case for unassigned variable --- Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index 14c6e135f..4e4f43df7 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -78,5 +78,10 @@ function MyFunc2() { $results = Invoke-ScriptAnalyzer -ScriptDefinition '$array=@(); $list | ForEach-Object { $array += $c }' | Where-Object { $_.RuleName -eq $violationName } $results.Count | Should -Be 0 } + + It "Does not flag += operator when using unassigned variable" { + $results = Invoke-ScriptAnalyzer -ScriptDefinition '$list | ForEach-Object { $array += $c }' | Where-Object { $_.RuleName -eq $violationName } + $results.Count | Should -Be 0 + } } } \ No newline at end of file