Fix PSUseDeclaredVarsMoreThanAssignments to not give false positives when using += operator #935
Conversation
…ecause the rule misunderstood += as assignment
…bout the if/else decision
bergmeister
added
in progress
Area - PSUseDeclaredVarsMoreThanAssignments
and removed
in progress
labels
| @@ -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 } | |||
There was a problem hiding this comment.
what about the case where $a is not initialized? Does that fall into this rule or a different one?
There was a problem hiding this comment.
This is already covered because the way the algorithm works is that it first adds variable assignments to a list and then removes variables from this list when they are being used and at the end the left-over variables are basically the unused variables. In this case, += was not seen as a usage and therefore not being removed from this list. As a result, not initialising the variable does not affect the analysis because it already only looks for variable usages that are not in the list of assigned variables.
Using the += operator on an uninitialised variable might seem to be dangerous at first but it makes no problem in PowerShell at all therefore I do not think we should warn when doing that. It would only be a problem when using Set-StrictMode using version 1 or higher.
I added your test case though because it is always good to have more test cases moving forward.
…nalyzer into UseDeclaredVarsMoreThanAssignments_FixFalsePositives
PR Summary
Hooray, this is the fix for the most common false positive of PSSA.
The main issue was that it was treating
+=as assignment. There are more special cases that need to be looked at like e.g. if the scriptblock is in the context of dot-sourcing but this is a different story, therefore getting the false positives fixed first, will, make the overall experience in most cases much better since some people are excluding the rule due to the many false positives.Fixes #699
Fixes #827
Fixes #903 # Note to myself that other bugs in the comment of this issue should be extracted
Fixes PowerShell/vscode-powershell#776
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets. Please mark anything not applicable to this PRNA.WIP:to the beginning of the title and remove the prefix when the PR is ready