Skip to content

UseUsingScopeModifierInNewRunspaces: Fix ArgumentException when the same variable name is used in 2 different sessions. #1493

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 2 commits into from
May 18, 2020
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
33 changes: 18 additions & 15 deletions Rules/UseUsingScopeModifierInNewRunspaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private class SyntaxCompatibilityVisitor : AstVisitor
private static readonly IEnumerable<string> s_invokeCommandCmdletNamesAndAliases =
Helper.Instance.CmdletNameAndAliases("Invoke-Command");

private readonly Dictionary<string, Dictionary<string, VariableExpressionAst>> _varsDeclaredPerSession;
private readonly Dictionary<string, HashSet<string>> _varsDeclaredPerSession;

private readonly List<DiagnosticRecord> _diagnosticAccumulator;

Expand All @@ -127,7 +127,7 @@ private class SyntaxCompatibilityVisitor : AstVisitor
public SyntaxCompatibilityVisitor(UseUsingScopeModifierInNewRunspaces rule, string analyzedScriptPath)
{
_diagnosticAccumulator = new List<DiagnosticRecord>();
_varsDeclaredPerSession = new Dictionary<string, Dictionary<string, VariableExpressionAst>>();
_varsDeclaredPerSession = new Dictionary<string, HashSet<string>>();
_rule = rule;
_analyzedFilePath = analyzedScriptPath;
}
Expand Down Expand Up @@ -183,15 +183,15 @@ public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionA
return AstVisitAction.Continue;
}

IReadOnlyDictionary<string, VariableExpressionAst> varsInLocalAssignments = FindVarsInAssignmentAsts(scriptBlockExpressionAst);
HashSet<string> varsInLocalAssignments = FindVarsInAssignmentAsts(scriptBlockExpressionAst);
if (varsInLocalAssignments != null)
{
AddAssignedVarsToSession(sessionName, varsInLocalAssignments);
}

GenerateDiagnosticRecords(
FindNonAssignedNonUsingVarAsts(
scriptBlockExpressionAst,
scriptBlockExpressionAst,
GetAssignedVarsInSession(sessionName)));

return AstVisitAction.SkipChildren;
Expand All @@ -205,10 +205,10 @@ public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionA
/// Example: `$foo = "foo"` ==> the VariableExpressionAst for $foo is returned
/// </summary>
/// <param name="ast"></param>
private static IReadOnlyDictionary<string, VariableExpressionAst> FindVarsInAssignmentAsts(Ast ast)
private static HashSet<string> FindVarsInAssignmentAsts(Ast ast)
{
Dictionary<string, VariableExpressionAst> variableDictionary =
new Dictionary<string, VariableExpressionAst>();
HashSet<string> variableDictionary =
new HashSet<string>();

// Find all variables that are assigned within this ast
foreach (AssignmentStatementAst statementAst in ast.FindAll(IsAssignmentStatementAst, true))
Expand All @@ -217,11 +217,11 @@ private static IReadOnlyDictionary<string, VariableExpressionAst> FindVarsInAssi
{
string variableName = string.Format(variable.VariablePath.UserPath,
StringComparer.OrdinalIgnoreCase);
variableDictionary.Add(variableName, variable);
variableDictionary.Add(variableName);
}
};

return new ReadOnlyDictionary<string, VariableExpressionAst>(variableDictionary);
return variableDictionary;
}

/// <summary>
Expand Down Expand Up @@ -264,14 +264,14 @@ private static bool IsAssignmentStatementAst(Ast ast)
/// <param name="ast"></param>
/// <param name="varsInAssignments"></param>
private static IEnumerable<VariableExpressionAst> FindNonAssignedNonUsingVarAsts(
Ast ast, IReadOnlyDictionary<string, VariableExpressionAst> varsInAssignments)
Ast ast, HashSet<string> varsInAssignments)
{
// Find all variables that are not locally assigned, and don't have $using: scope modifier
foreach (VariableExpressionAst variable in ast.FindAll(IsNonUsingNonSpecialVariableExpressionAst, true))
{
var varName = string.Format(variable.VariablePath.UserPath, StringComparer.OrdinalIgnoreCase);

if (varsInAssignments.ContainsKey(varName))
if (varsInAssignments.Contains(varName))
{
yield break;
}
Expand Down Expand Up @@ -368,7 +368,7 @@ private static bool TryGetSessionNameFromInvokeCommand(CommandAst invokeCommandA
/// GetAssignedVarsInSession: Retrieves all previously declared vars for a given session (as in Invoke-Command -Session $session).
/// </summary>
/// <param name="sessionName"></param>
private IReadOnlyDictionary<string,VariableExpressionAst> GetAssignedVarsInSession(string sessionName)
private HashSet<string> GetAssignedVarsInSession(string sessionName)
{
return _varsDeclaredPerSession[sessionName];
}
Expand All @@ -378,16 +378,19 @@ private IReadOnlyDictionary<string,VariableExpressionAst> GetAssignedVarsInSessi
/// </summary>
/// <param name="sessionName"></param>
/// <param name="variablesToAdd"></param>
private void AddAssignedVarsToSession(string sessionName, IReadOnlyDictionary<string, VariableExpressionAst> variablesToAdd)
private void AddAssignedVarsToSession(string sessionName, HashSet<string> variablesToAdd)
{
if (!_varsDeclaredPerSession.ContainsKey(sessionName))
{
_varsDeclaredPerSession.Add(sessionName, new Dictionary<string, VariableExpressionAst>());
_varsDeclaredPerSession.Add(sessionName, new HashSet<string>());
}

foreach (var item in variablesToAdd)
{
_varsDeclaredPerSession[sessionName].Add(item.Key, item.Value);
if (!_varsDeclaredPerSession[sessionName].Contains(item))
{
_varsDeclaredPerSession[sessionName].Add(item);
}
}
}

Expand Down
21 changes: 21 additions & 0 deletions Tests/Rules/UseUsingScopeModifierInNewRunspaces.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,27 @@ Describe "UseUsingScopeModifierInNewRunspaces" {
}
}'
}
# Issue 1492: https://github.com/PowerShell/PSScriptAnalyzer/issues/1492
@{
Description = 'Does not throw when the same variable name is used in two different sessions'
ScriptBlock = @'
function Get-One{

Invoke-Command -Session $sourceRemoteSession {
$a = $sccmModule
foo $a
}
}

function Get-Two{

Invoke-Command -Session $sourceRemoteSession {
$a = $sccmModule
foo $a
}
}
'@
}
)
}

Expand Down