Skip to content
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

Enhance ReviewUnusedParameter to bail out if $MyInvocation.BoundParameters encountered #1

Merged
merged 5 commits into from
Jun 11, 2020
Merged
Changes from 3 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
40 changes: 34 additions & 6 deletions Rules/ReviewUnusedParameter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)

foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
{
// bail out if PS bound parameter used.
if (scriptBlockAst.FindAll(IsBoundParametersReference, false).Any())
jegannathanmaniganadan marked this conversation as resolved.
Show resolved Hide resolved
{
continue;
}

// find all declared parameters
IEnumerable<Ast> parameterAsts = scriptBlockAst.FindAll(oneAst => oneAst is ParameterAst, false);

Expand All @@ -45,12 +51,6 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
.GroupBy(variableName => variableName, StringComparer.OrdinalIgnoreCase)
.ToDictionary(variableName => variableName.Key, variableName => variableName.Count(), StringComparer.OrdinalIgnoreCase);

// all bets are off if the script uses PSBoundParameters
if (variableCount.ContainsKey("PSBoundParameters"))
{
continue;
}

foreach (ParameterAst parameterAst in parameterAsts)
{
// there should be at least two usages of the variable since the parameter declaration counts as one
Expand All @@ -72,6 +72,34 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
}
}

/// <summary>
/// Checks for PS bound parameter reference.
/// </summary>
/// <param name="ast"></param>
jegannathanmaniganadan marked this conversation as resolved.
Show resolved Hide resolved
/// <returns></returns>
private static bool IsBoundParametersReference(Ast ast)
{
bool isBound = false;
// if $MyInvocation.BoundParameters or $PSCmdlet.MyInvocation.BoundParameters used.
if (ast is MemberExpressionAst memberAst
&& memberAst.Member is StringConstantExpressionAst memberStringAst
&& memberStringAst.Value.Equals("BoundParameters", StringComparison.OrdinalIgnoreCase))
{
// handle both regular and nested scenarios.
isBound = (memberAst.Expression is VariableExpressionAst veAst
&& veAst.VariablePath.UserPath.Equals("MyInvocation", StringComparison.OrdinalIgnoreCase))
|| (memberAst.Expression is MemberExpressionAst meAst
&& meAst.Expression is VariableExpressionAst veAstNested
&& veAstNested.VariablePath.UserPath.Equals("PSCmdlet", StringComparison.OrdinalIgnoreCase)
&& meAst.Member is StringConstantExpressionAst sceAstNested
&& sceAstNested.Value.Equals("MyInvocation", StringComparison.OrdinalIgnoreCase));
}

return isBound
|| ast is VariableExpressionAst variableAst
jegannathanmaniganadan marked this conversation as resolved.
Show resolved Hide resolved
&& variableAst.VariablePath.UserPath.Equals("PSBoundParameters", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// GetName: Retrieves the name of this rule.
/// </summary>
Expand Down