Skip to content

Fix VariableAnalysis error with outer scope #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
25 changes: 25 additions & 0 deletions Engine/VariableAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 0 additions & 4 deletions Engine/VariableAnalysisBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 4 additions & 1 deletion Tests/Rules/AvoidGlobalOrUnitializedVars.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ while ($false) {
$a;

# $d may not be initialized too
$d;
$d;

# error must be raised here
Invoke-Command -ScriptBlock {$a; }
4 changes: 2 additions & 2 deletions Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
2 changes: 2 additions & 0 deletions Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down