Skip to content

Commit a848091

Browse files
raghushantharaghushantha
committed
Merge pull request #174 from GoodOlClint/PSProvideDefaultParameterValue
Add PSProvideDefaultParameterValue Rule
2 parents 9539455 + 276f464 commit a848091

11 files changed

+267
-17
lines changed

‎Rules/AvoidUninitializedVariable.cs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,24 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5252

5353
IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true);
5454
IEnumerable<Ast> funcMemberAsts = ast.FindAll(item => item is FunctionMemberAst, true);
55-
56-
// Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
57-
bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
58-
59-
List<string> targetResourcesFunctions = new List<string>( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
60-
55+
6156
foreach (FunctionDefinitionAst funcAst in funcAsts)
6257
{
6358
// Finds all VariableExpressionAst.
6459
IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
6560

6661
HashSet<string> paramVariables = new HashSet<string>();
6762

68-
if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
63+
// don't raise the rules for variables in the param block.
64+
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
6965
{
70-
// don't raise the rules for variables in the param block.
71-
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
72-
{
73-
paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
74-
}
66+
paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
67+
}
68+
69+
//don't raise the rules for parameters outside the param block
70+
if(funcAst.Parameters != null)
71+
{
72+
paramVariables.UnionWith(funcAst.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
7573
}
7674

7775
// Iterates all VariableExpressionAst and check the command name.

‎Rules/ProvideDefaultParameterValue.cs

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
//
2+
// Copyright (c) Microsoft Corporation.
3+
//
4+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
5+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
6+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
7+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
8+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
9+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
10+
// THE SOFTWARE.
11+
//
12+
13+
using System;
14+
using System.Linq;
15+
using System.Collections.Generic;
16+
using System.Management.Automation.Language;
17+
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
18+
using System.ComponentModel.Composition;
19+
using System.Globalization;
20+
21+
namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
22+
{
23+
/// <summary>
24+
/// ProvideDefaultParameterValue: Check if any uninitialized variable is used.
25+
/// </summary>
26+
[Export(typeof(IScriptRule))]
27+
public class ProvideDefaultParameterValue : IScriptRule
28+
{
29+
/// <summary>
30+
/// AnalyzeScript: Check if any uninitialized variable is used.
31+
/// </summary>
32+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
33+
{
34+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
35+
36+
// Finds all functionAst
37+
IEnumerable<Ast> functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);
38+
39+
// Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
40+
bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
41+
42+
List<string> targetResourcesFunctions = new List<string>(new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
43+
44+
45+
foreach (FunctionDefinitionAst funcAst in functionAsts)
46+
{
47+
// Finds all ParamAsts.
48+
IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
49+
50+
// Iterrates all ParamAsts and check if their names are on the list.
51+
52+
HashSet<string> dscVariables = new HashSet<string>();
53+
if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
54+
{
55+
// don't raise the rules for variables in the param block.
56+
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
57+
{
58+
dscVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
59+
}
60+
}
61+
// only raise the rules for variables in the param block.
62+
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
63+
{
64+
foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
65+
{
66+
if (Helper.Instance.IsUninitialized(paramAst.Name, funcAst) && !dscVariables.Contains(paramAst.Name.VariablePath.UserPath))
67+
{
68+
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueError, paramAst.Name.VariablePath.UserPath),
69+
paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
70+
}
71+
}
72+
}
73+
74+
if (funcAst.Parameters != null)
75+
{
76+
foreach (var paramAst in funcAst.Parameters)
77+
{
78+
if (Helper.Instance.IsUninitialized(paramAst.Name, funcAst) && !dscVariables.Contains(paramAst.Name.VariablePath.UserPath))
79+
{
80+
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueError, paramAst.Name.VariablePath.UserPath),
81+
paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
82+
}
83+
}
84+
}
85+
}
86+
}
87+
88+
/// <summary>
89+
/// GetName: Retrieves the name of this rule.
90+
/// </summary>
91+
/// <returns>The name of this rule</returns>
92+
public string GetName()
93+
{
94+
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ProvideDefaultParameterValueName);
95+
}
96+
97+
/// <summary>
98+
/// GetCommonName: Retrieves the common name of this rule.
99+
/// </summary>
100+
/// <returns>The common name of this rule</returns>
101+
public string GetCommonName()
102+
{
103+
return string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueCommonName);
104+
}
105+
106+
/// <summary>
107+
/// GetDescription: Retrieves the description of this rule.
108+
/// </summary>
109+
/// <returns>The description of this rule</returns>
110+
public string GetDescription()
111+
{
112+
return string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueDescription);
113+
}
114+
115+
/// <summary>
116+
/// Method: Retrieves the type of the rule: builtin, managed or module.
117+
/// </summary>
118+
public SourceType GetSourceType()
119+
{
120+
return SourceType.Builtin;
121+
}
122+
123+
/// <summary>
124+
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
125+
/// </summary>
126+
/// <returns></returns>
127+
public RuleSeverity GetSeverity()
128+
{
129+
return RuleSeverity.Warning;
130+
}
131+
132+
/// <summary>
133+
/// Method: Retrieves the module/assembly name the rule is from.
134+
/// </summary>
135+
public string GetSourceName()
136+
{
137+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
138+
}
139+
}
140+
}

‎Rules/ScriptAnalyzerBuiltinRules.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
<Compile Include="AvoidReservedParams.cs" />
6060
<Compile Include="AvoidShouldContinueWithoutForce.cs" />
6161
<Compile Include="AvoidTrapStatement.cs" />
62+
<Compile Include="ProvideDefaultParameterValue.cs" />
6263
<Compile Include="AvoidUninitializedVariable.cs" />
6364
<Compile Include="AvoidUsernameAndPasswordParams.cs" />
6465
<Compile Include="AvoidUsingComputerNameHardcoded.cs" />

‎Rules/Strings.Designer.cs

Lines changed: 37 additions & 1 deletion
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
@@ -714,4 +714,16 @@
714714
<data name="DscExamplesPresentNoExamplesError" xml:space="preserve">
715715
<value>No examples found for resource '{0}'</value>
716716
</data>
717+
<data name="ProvideDefaultParameterValueCommonName" xml:space="preserve">
718+
<value>Default Parameter Values</value>
719+
</data>
720+
<data name="ProvideDefaultParameterValueDescription" xml:space="preserve">
721+
<value>Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters</value>
722+
</data>
723+
<data name="ProvideDefaultParameterValueError" xml:space="preserve">
724+
<value>Parameter '{0}' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters</value>
725+
</data>
726+
<data name="ProvideDefaultParameterValueName" xml:space="preserve">
727+
<value>ProvideDefaultParameterValue</value>
728+
</data>
717729
</root>

‎Tests/Engine/InvokeScriptAnalyzer.tests.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ Describe "Test IncludeRule" {
136136
Context "IncludeRule supports wild card" {
137137
It "includes 1 wildcard rule"{
138138
$includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules
139-
$includeWildcard.Count | Should be 5
139+
$includeWildcard.Count | Should be 3
140140
}
141141

142142
it "includes 2 wildcardrules" {
143143
$includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules
144-
$includeWildcard.Count | Should be 9
144+
$includeWildcard.Count | Should be 7
145145
}
146146
}
147147
}

‎Tests/Engine/RuleSuppression.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Param(
66
function SuppressMe ()
77
{
88
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideVerboseMessage")]
9-
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUninitializedVariable", "unused1")]
9+
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideDefaultParameterValue", "unused1")]
1010
Param([string]$unUsed1, [int] $unUsed2)
1111
{
1212
Write-Host "I do nothing"

‎Tests/Engine/RuleSuppression.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Describe "RuleSuppressionWithoutScope" {
3333

3434
Context "RuleSuppressionID" {
3535
It "Only suppress violations for that ID" {
36-
$suppression = $violations | Where-Object {$_.RuleName -eq "PSAvoidUninitializedVariable" }
36+
$suppression = $violations | Where-Object {$_.RuleName -eq "PSProvideDefaultParameterValue" }
3737
$suppression.Count | Should Be 1
3838
}
3939
}

‎Tests/Rules/ProvideDefaultParameterValue.ps1

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
function BadFunc
2+
{
3+
param(
4+
[Parameter(Mandatory=$true)]
5+
[ValidateNotNullOrEmpty()]
6+
[string]
7+
$Param1,
8+
[Parameter(Mandatory=$false)]
9+
[ValidateNotNullOrEmpty()]
10+
[string]
11+
$Param2
12+
)
13+
$Param1
14+
$Param1 = "test"
15+
}
16+
17+
function BadFunc2($Param1)
18+
{
19+
$Param1
20+
}

‎Tests/Rules/ProvideDefaultParameterValue.tests.ps1

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
Import-Module PSScriptAnalyzer
2+
$violationName = "PSProvideDefaultParameterValue"
3+
$violationMessage = "Parameter 'Param2' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters"
4+
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
5+
$violations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValue.ps1 | Where-Object {$_.RuleName -match $violationName}
6+
$noViolations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValueNoViolations.ps1
7+
8+
Describe "ProvideDefaultParameters" {
9+
Context "When there are violations" {
10+
It "has 2 provide default parameter value violation" {
11+
$violations.Count | Should Be 2
12+
}
13+
14+
It "has the correct description message" {
15+
$violations[0].Message | Should Match $violationMessage
16+
}
17+
}
18+
19+
Context "When there are no violations" {
20+
It "returns no violations" {
21+
$noViolations.Count | Should Be 0
22+
}
23+
}
24+
}

‎Tests/Rules/ProvideDefaultParameterValueNoViolations.ps1

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
function GoodFunc
2+
{
3+
param(
4+
[Parameter(Mandatory=$true)]
5+
[ValidateNotNullOrEmpty()]
6+
[string]
7+
$Param1,
8+
[Parameter(Mandatory=$false)]
9+
[ValidateNotNullOrEmpty()]
10+
[string]
11+
$Param2=$null
12+
)
13+
$Param1
14+
}
15+
16+
function GoodFunc2($Param1 = $null)
17+
{
18+
$Param1
19+
}

0 commit comments

Comments
 (0)