Fixed erroneous PSUseDeclaredVarsMoreThanAssignments for some globals variables (#2013)

* Fixed erroneous PSUseDeclaredVarsMoreThanAssignments for some globals.

* Added unit tests to cover global bug in UseDeclaredVarsMoreThanAssignments.

---------

Co-authored-by: Christoph Bergmeister <[email protected]>

Author: John-Leitch

Engine/Helper.cs

@@ -870,19 +870,13 @@ public bool IsUninitialized(VariableExpressionAst varAst, Ast ast)
         }
 
         /// <summary>
-        /// Returns true if varaible is either a global variable or an environment variable
+        /// Returns true if variable is either a global variable or an environment variable
         /// </summary>
         /// <param name="varAst"></param>
-        /// <param name="ast"></param>
         /// <returns></returns>
-        public bool IsVariableGlobalOrEnvironment(VariableExpressionAst varAst, Ast ast)
+        public bool IsVariableGlobalOrEnvironment(VariableExpressionAst varAst)
         {
-            if (!VariableAnalysisDictionary.ContainsKey(ast) || VariableAnalysisDictionary[ast] == null)
-            {
-                return false;
-            }
-
-            return VariableAnalysisDictionary[ast].IsGlobalOrEnvironment(varAst);
+            return VariableAnalysis.IsGlobalOrEnvironment(varAst);
         }
 
 

Engine/VariableAnalysis.cs

@@ -375,7 +375,7 @@ public bool IsUninitialized(VariableExpressionAst varTarget)
         /// </summary>
         /// <param name="varTarget"></param>
         /// <returns></returns>
-        public bool IsGlobalOrEnvironment(VariableExpressionAst varTarget)
+        public static bool IsGlobalOrEnvironment(VariableExpressionAst varTarget)
         {
             if (varTarget != null)
             {

Rules/UseDeclaredVarsMoreThanAssignments.cs

@@ -143,7 +143,7 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip
                 if (assignmentVarAst != null)
                 {
                     // Ignore if variable is global or environment variable or scope is drive qualified variable
-                    if (!Helper.Instance.IsVariableGlobalOrEnvironment(assignmentVarAst, scriptBlockAst)
+                    if (!Helper.Instance.IsVariableGlobalOrEnvironment(assignmentVarAst)
                         && !assignmentVarAst.VariablePath.IsScript
                         && assignmentVarAst.VariablePath.DriveName == null)
                     {

Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1

@@ -58,6 +58,48 @@ function MyFunc2() {
             Should -Be 0
         }
 
+        It "does not flag global variable" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$global:x=$null' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "does not flag global variable in block" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$global:x=$null;{$global:x=$null}' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "does not flag env variable" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$env:x=$null' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "does not flag env variable in block" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$env:x=$null;{$env:x=$null}' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "does not flag script variable" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$script:x=$null' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "does not flag script variable in block" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$script:x=$null;{$script:x=$null}' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 0
+        }
+
+        It "flags private variable" {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$private:x=$null' -IncludeRule $violationName  | `
+            Get-Count | `
+            Should -Be 1
+        }
+
         It "flags a variable that is defined twice but never used" {
             Invoke-ScriptAnalyzer -ScriptDefinition '$myvar=1;$myvar=2' -IncludeRule $violationName | `
             Get-Count | `