Skip to content

Commit 929ae14

Browse files
committed
New rule - ReviewUnusedParameter
1 parent ab69069 commit 929ae14

File tree

7 files changed

+329
-45
lines changed

7 files changed

+329
-45
lines changed

RuleDocumentation/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
|[ProvideCommentHelp](./ProvideCommentHelp.md) | Information | Yes |
4444
|[ReservedCmdletChar](./ReservedCmdletChar.md) | Error | |
4545
|[ReservedParams](./ReservedParams.md) | Error | |
46+
|[ReviewUnusedParameter](./ReviewUnusedParameter.md) | Warning | |
4647
|[ShouldProcess](./ShouldProcess.md) | Error | |
4748
|[UseApprovedVerbs](./UseApprovedVerbs.md) | Warning | |
4849
|[UseBOMForUnicodeEncodedFile](./UseBOMForUnicodeEncodedFile.md) | Warning | |
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# ReviewUnusedParameter
2+
3+
**Severity Level: Warning**
4+
5+
## Description
6+
7+
This rule identifies parameters declared in a script, scriptblock, or function scope that have not been used in that scope.
8+
9+
## How
10+
11+
Consider removing the unused parameter.
12+
13+
## Example
14+
15+
### Wrong
16+
17+
``` PowerShell
18+
function Test-Parameter
19+
{
20+
Param (
21+
$Parameter1,
22+
23+
# this parameter is never called in the function
24+
$Parameter2
25+
)
26+
27+
Get-Something $Parameter1
28+
}
29+
```
30+
31+
### Correct
32+
33+
``` PowerShell
34+
function Test-Parameter
35+
{
36+
Param (
37+
$Parameter1,
38+
39+
# now this parameter is being called in the same scope
40+
$Parameter2
41+
)
42+
43+
Get-Something $Parameter1 $Parameter2
44+
}
45+
```

Rules/ReviewUnusedParameter.cs

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Management.Automation.Language;
8+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
9+
#if !CORECLR
10+
using System.ComponentModel.Composition;
11+
#endif
12+
using System.Globalization;
13+
14+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
15+
{
16+
/// <summary>
17+
/// ReviewUnusedParameter: Check that all declared parameters are used in the script body.
18+
/// </summary>
19+
#if !CORECLR
20+
[Export(typeof(IScriptRule))]
21+
#endif
22+
public class ReviewUnusedParameter : IScriptRule
23+
{
24+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
25+
{
26+
if (ast == null)
27+
{
28+
throw new ArgumentNullException(Strings.NullAstErrorMessage);
29+
}
30+
31+
var scriptBlockAsts = ast.FindAll(x => x is ScriptBlockAst, true);
32+
if (scriptBlockAsts == null)
33+
{
34+
yield break;
35+
}
36+
37+
foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
38+
{
39+
// find all declared parameters
40+
IEnumerable<Ast> parameterAsts = scriptBlockAst.FindAll(a => a is ParameterAst, false);
41+
42+
// list all variables
43+
List<string> variables = scriptBlockAst.FindAll(a => a is VariableExpressionAst, false)
44+
.Cast<VariableExpressionAst>()
45+
.Select(v => v.VariablePath.ToString())
46+
.ToList();
47+
48+
foreach (ParameterAst parameterAst in parameterAsts)
49+
{
50+
// compare the list of variables to the parameter name
51+
// there should be at least two matches of the variable name since the parameter declaration counts as one
52+
int matchCount = variables
53+
.Where(x => x == parameterAst.Name.VariablePath.ToString())
54+
.Count();
55+
if (matchCount > 1)
56+
{
57+
continue;
58+
}
59+
60+
// all bets are off if the script uses PSBoundParameters
61+
if (variables.Contains("PSBoundParameters"))
62+
{
63+
continue;
64+
}
65+
66+
yield return new DiagnosticRecord(
67+
string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterError, parameterAst.Name.VariablePath.UserPath),
68+
parameterAst.Name.Extent,
69+
GetName(),
70+
DiagnosticSeverity.Warning,
71+
fileName,
72+
parameterAst.Name.VariablePath.UserPath
73+
);
74+
}
75+
}
76+
}
77+
78+
/// <summary>
79+
/// GetName: Retrieves the name of this rule.
80+
/// </summary>
81+
/// <returns>The name of this rule</returns>
82+
public string GetName()
83+
{
84+
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ReviewUnusedParameterName);
85+
}
86+
87+
/// <summary>
88+
/// GetCommonName: Retrieves the common name of this rule.
89+
/// </summary>
90+
/// <returns>The common name of this rule</returns>
91+
public string GetCommonName()
92+
{
93+
return string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterCommonName);
94+
}
95+
96+
/// <summary>
97+
/// GetDescription: Retrieves the description of this rule.
98+
/// </summary>
99+
/// <returns>The description of this rule</returns>
100+
public string GetDescription()
101+
{
102+
return string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterDescription);
103+
}
104+
105+
/// <summary>
106+
/// GetSourceType: Retrieves the type of the rule, builtin, managed or module.
107+
/// </summary>
108+
public SourceType GetSourceType()
109+
{
110+
return SourceType.Builtin;
111+
}
112+
113+
/// <summary>
114+
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
115+
/// </summary>
116+
/// <returns></returns>
117+
public RuleSeverity GetSeverity()
118+
{
119+
return RuleSeverity.Warning;
120+
}
121+
122+
/// <summary>
123+
/// GetSourceName: Retrieves the module/assembly name the rule is from.
124+
/// </summary>
125+
public string GetSourceName()
126+
{
127+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
128+
}
129+
}
130+
}

Rules/Strings.Designer.cs

Lines changed: 72 additions & 44 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,4 +1104,16 @@
11041104
<data name="UseProcessBlockForPipelineCommandName" xml:space="preserve">
11051105
<value>UseProcessBlockForPipelineCommand</value>
11061106
</data>
1107+
<data name="ReviewUnusedParameterCommonName" xml:space="preserve">
1108+
<value>ReviewUnusedParameter</value>
1109+
</data>
1110+
<data name="ReviewUnusedParameterDescription" xml:space="preserve">
1111+
<value>Ensure all parameters are used within the same script, scriptblock, or function where they are declared.</value>
1112+
</data>
1113+
<data name="ReviewUnusedParameterError" xml:space="preserve">
1114+
<value>The parameter {0} has been declared but not used. </value>
1115+
</data>
1116+
<data name="ReviewUnusedParameterName" xml:space="preserve">
1117+
<value>ReviewUnusedParameter</value>
1118+
</data>
11071119
</root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Describe "Test Name parameters" {
5959

6060
It "get Rules with no parameters supplied" {
6161
$defaultRules = Get-ScriptAnalyzerRule
62-
$expectedNumRules = 62
62+
$expectedNumRules = 63
6363
if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
6464
{
6565
# for PSv3 PSAvoidGlobalAliases is not shipped because

0 commit comments

Comments
 (0)