Merge pull request #60 from PowerShell/FixVariableAnalysisScope

Fix VariableAnalysis error with outer scope

Author: Quoc Truong

Engine/Helper.cs

@@ -700,7 +700,8 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst)
 
             VariableAnalysis previousOuter = OuterAnalysis;
 
-            // We already run variable analysis in these cases so check
+            // We already run variable analysis if the parent is a function so skip these.
+            // Otherwise, we have to do variable analysis using the outer scope variables.
             if (!(scriptBlockAst.Parent is FunctionDefinitionAst) && !(scriptBlockAst.Parent is FunctionMemberAst))
             {
                 OuterAnalysis = Helper.Instance.InitializeVariableAnalysisHelper(scriptBlockAst, OuterAnalysis);
@@ -726,8 +727,15 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst)
                 scriptBlockAst.EndBlock.Visit(this);
             }
 
+            VariableAnalysis innerAnalysis = OuterAnalysis;
             OuterAnalysis = previousOuter;
 
+            if (!(scriptBlockAst.Parent is FunctionDefinitionAst) && !(scriptBlockAst.Parent is FunctionMemberAst))
+            {
+                // Update the variable analysis of the outer script block
+                VariableAnalysis.UpdateOuterAnalysis(OuterAnalysis, innerAnalysis);
+            }
+
             return null;
         }
 
@@ -889,6 +897,13 @@ public object VisitCatchClause(CatchClauseAst catchClauseAst)
         /// <returns></returns>
         public object VisitCommand(CommandAst commandAst)
         {
+            if (commandAst == null) return null;
+
+            foreach (CommandElementAst ceAst in commandAst.CommandElements)
+            {
+                ceAst.Visit(this);
+            }
+
             return null;
         }
 
@@ -1219,12 +1234,19 @@ public object VisitParenExpression(ParenExpressionAst parenExpressionAst)
         }
 
         /// <summary>
-        /// Do nothing
+        /// Visit pipeline
         /// </summary>
         /// <param name="pipelineAst"></param>
         /// <returns></returns>
         public object VisitPipeline(PipelineAst pipelineAst)
         {
+            if (pipelineAst == null) return null;
+
+            foreach (var command in pipelineAst.PipelineElements)
+            {
+                command.Visit(this);
+            }
+
             return null;
         }
 

Engine/VariableAnalysis.cs

@@ -255,6 +255,31 @@ public void AnalyzeImpl(Ast ast, VariableAnalysis outerAnalysis)
             }
         }
 
+        /// <summary>
+        /// Updates the variablesdictionary of the outeranalysis based on that of the inneranalysis
+        /// </summary>
+        /// <param name="OuterAnalysis"></param>
+        /// <param name="InnerAnalysis"></param>
+        internal static void UpdateOuterAnalysis(VariableAnalysis OuterAnalysis, VariableAnalysis InnerAnalysis)
+        {
+            if (OuterAnalysis == null || InnerAnalysis == null)
+            {
+                return;
+            }
+
+            foreach (var key in InnerAnalysis.VariablesDictionary.Keys)
+            {
+                if (OuterAnalysis.VariablesDictionary.ContainsKey(key))
+                {
+                    OuterAnalysis.VariablesDictionary[key] = InnerAnalysis.VariablesDictionary[key];
+                }
+                else
+                {
+                    OuterAnalysis.VariablesDictionary.Add(key, InnerAnalysis.VariablesDictionary[key]);
+                }
+            }
+        }
+
         /// <summary>
         /// Return variableanalysisdetails for VarTarget.
         /// This function should only be called after Block.SparseSimpleConstants are called.

Engine/VariableAnalysisBase.cs

@@ -2957,10 +2957,6 @@ public object VisitScriptBlockExpression(ScriptBlockExpressionAst scriptBlockExp
         {
             // Don't recurse into the script block, it's variables are distinct from the script block
             // we're currently analyzing.
-            if (scriptBlockExpressionAst != null)
-            {
-                scriptBlockExpressionAst.ScriptBlock.Visit(this.Decorator);
-            }
             return null;
         }
 

Tests/Rules/AvoidGlobalOrUnitializedVars.ps1

@@ -19,4 +19,7 @@ while ($false) {
 $a;
 
 # $d may not be initialized too
-$d;
+$d;
+
+# error must be raised here
+Invoke-Command -ScriptBlock {$a; }

Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1

@@ -31,8 +31,8 @@ Describe "AvoidGlobalVars" {
 
 Describe "AvoidUnitializedVars" {
     Context "When there are violations" {
-        It "has 4 avoid using unitialized variable violations" {
-            $nonInitializedViolations.Count | Should Be 4
+        It "has 5 avoid using unitialized variable violations" {
+            $nonInitializedViolations.Count | Should Be 5
         }
 
         It "has the correct description message" {

Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1

@@ -22,6 +22,8 @@ stop-process 12,23 -ErrorVariable ev -ErrorAction SilentlyContinue
 if($null -ne $ev)
 {
     Write-host $ev[0]
+    # no error should be raised here
+    Invoke-Command {$b}
 }
 
 get-process notepad | tee-object -variable proc