Skip to content

Commit 1b4a9b9

Browse files
jegannathanmaniganadanManigandan Jegannathan
and
Manigandan Jegannathan
authored
ReviewUnusedParameter: Do not trigger when MyInvocation.BoundParameters or PSCmdlet.MyInvocation.BoundParameters is used (#1520)
* Enhance ReviewUnusedParameter to bail out if $MyInvocation.BoundParmaeters encountered * Account for $PSCmdlet.MyInvocation.BoundParameters * Optmised the way we find bound parameters reference with in script block * Address review feedback * Remove unnecessary casting * - Label boolean parameter - Add pester tests Co-authored-by: Manigandan Jegannathan <jegannat@deshaw.com>
1 parent ca4221f commit 1b4a9b9

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

Rules/ReviewUnusedParameter.cs

+47-6
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3636

3737
foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
3838
{
39+
// bail out if PS bound parameter used.
40+
if (scriptBlockAst.Find(IsBoundParametersReference, searchNestedScriptBlocks: false) != null)
41+
{
42+
continue;
43+
}
44+
3945
// find all declared parameters
4046
IEnumerable<Ast> parameterAsts = scriptBlockAst.FindAll(oneAst => oneAst is ParameterAst, false);
4147

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

48-
// all bets are off if the script uses PSBoundParameters
49-
if (variableCount.ContainsKey("PSBoundParameters"))
50-
{
51-
continue;
52-
}
53-
5454
foreach (ParameterAst parameterAst in parameterAsts)
5555
{
5656
// there should be at least two usages of the variable since the parameter declaration counts as one
@@ -72,6 +72,47 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7272
}
7373
}
7474

75+
/// <summary>
76+
/// Checks for PS bound parameter reference.
77+
/// </summary>
78+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
79+
/// <returns>Boolean true indicating that given AST has PS bound parameter reference, otherwise false</returns>
80+
private static bool IsBoundParametersReference(Ast ast)
81+
{
82+
// $PSBoundParameters
83+
if (ast is VariableExpressionAst variableAst
84+
&& variableAst.VariablePath.UserPath.Equals("PSBoundParameters", StringComparison.OrdinalIgnoreCase))
85+
{
86+
return true;
87+
}
88+
89+
if (ast is MemberExpressionAst memberAst
90+
&& memberAst.Member is StringConstantExpressionAst memberStringAst
91+
&& memberStringAst.Value.Equals("BoundParameters", StringComparison.OrdinalIgnoreCase))
92+
{
93+
// $MyInvocation.BoundParameters
94+
if (memberAst.Expression is VariableExpressionAst veAst
95+
&& veAst.VariablePath.UserPath.Equals("MyInvocation", StringComparison.OrdinalIgnoreCase))
96+
{
97+
return true;
98+
}
99+
100+
// $PSCmdlet.MyInvocation.BoundParameters
101+
if (memberAst.Expression is MemberExpressionAst meAstNested)
102+
{
103+
if (meAstNested.Expression is VariableExpressionAst veAstNested
104+
&& veAstNested.VariablePath.UserPath.Equals("PSCmdlet", StringComparison.OrdinalIgnoreCase)
105+
&& meAstNested.Member is StringConstantExpressionAst sceAstNested
106+
&& sceAstNested.Value.Equals("MyInvocation", StringComparison.OrdinalIgnoreCase))
107+
{
108+
return true;
109+
}
110+
}
111+
}
112+
113+
return false;
114+
}
115+
75116
/// <summary>
76117
/// GetName: Retrieves the name of this rule.
77118
/// </summary>

Tests/Rules/ReviewUnusedParameter.tests.ps1

+12
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ Describe "ReviewUnusedParameter" {
5959
$Violations.Count | Should -Be 0
6060
}
6161

62+
It "has no violations when using MyInvocation.BoundParameters" {
63+
$ScriptDefinition = 'function Bound { param ($Param1) $splat = $MyInvocation.BoundParameters; Get-Foo @splat }'
64+
$Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
65+
$Violations.Count | Should -Be 0
66+
}
67+
68+
It "has no violations when using PSCmdlet.MyInvocation.BoundParameters" {
69+
$ScriptDefinition = 'function Bound { param ($Param1) $splat = $PSCmdlet.MyInvocation.BoundParameters; Get-Foo @splat }'
70+
$Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
71+
$Violations.Count | Should -Be 0
72+
}
73+
6274
It "has no violations when parameter is called in child scope" -skip {
6375
$ScriptDefinition = 'function foo { param ($Param1) function Child { $Param1 } }'
6476
$Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName

0 commit comments

Comments
 (0)