PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.87k stars 378 forks source link

Fixed erroneous PSUseDeclaredVarsMoreThanAssignments for some globals variables #2013

Open John-Leitch opened 4 months ago

John-Leitch commented 4 months ago

PR Summary

This PR fixes the following longstanding issue: https://github.com/PowerShell/PSScriptAnalyzer/issues/1300 (Global variables are erroneously reported as "never used")

The issue exists because Helper.IsVariableGlobalOrEnvironment uses an Ast to key into VariableAnalysisDictionary, calling VariableAnalysis.IsGlobalOrEnvironment on the value returned to determine whether or not the variable is a global. However, in some cases, the key does not exist in the dictionary, resulting in the method returning false. Fortunately, VariableAnalysis.IsGlobalOrEnvironment depends on no instance members, instead relying solely on the properties of VariableExpressionAst, which is passed in. As such, VariableAnalysis.IsGlobalOrEnvironment can be made static with no consequence, allowing Helper.IsVariableGlobalOrEnvironment to call the method without the dictionary operations.

Also of note, the current implementation is somewhat redundant, with UseDeclaredVarsMoreThanAssignments.AnalyzeScriptBlockAst checking VariablePath.DriveName separately, which is why the issue does not affect environment variables despite VariableAnalysis.IsGlobalOrEnvironment erroneously returning false in some cases.

PR Checklist

liamjpeters commented 4 months ago

Really clear explanation, thanks for getting to the root of the issue! 🙂 LGTM!