Add PSProvideDefaultParameterValue Rule

Rules/AvoidUninitializedVariable.cs

@@ -52,26 +52,24 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
             IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true);
             IEnumerable<Ast> funcMemberAsts = ast.FindAll(item => item is FunctionMemberAst, true);
-
-            // Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
-            bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
-
-            List<string> targetResourcesFunctions = new List<string>( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
-
+
             foreach (FunctionDefinitionAst funcAst in funcAsts)
             {
                 // Finds all VariableExpressionAst.
                 IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
 
                 HashSet<string> paramVariables = new HashSet<string>();
 
-                if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
+                // don't raise the rules for variables in the param block.
+                if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
                 {
-                    // don't raise the rules for variables in the param block.
-                    if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
-                    {
-                        paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
-                    }
+                    paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
+                }
+
+                //don't raise the rules for parameters outside the param block
+                if(funcAst.Parameters != null)
+                {
+                    paramVariables.UnionWith(funcAst.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
                 }
 
                 // Iterates all VariableExpressionAst and check the command name.

Rules/ProvideDefaultParameterValue.cs

@@ -0,0 +1,140 @@
+//
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+
+using System;
+using System.Linq;
+using System.Collections.Generic;
+using System.Management.Automation.Language;
+using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
+using System.ComponentModel.Composition;
+using System.Globalization;
+
+namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// ProvideDefaultParameterValue: Check if any uninitialized variable is used.
+    /// </summary>
+    [Export(typeof(IScriptRule))]
+    public class ProvideDefaultParameterValue : IScriptRule
+    {
+        /// <summary>
+        /// AnalyzeScript: Check if any uninitialized variable is used.
+        /// </summary>
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
+
+            // Finds all functionAst
+            IEnumerable<Ast> functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);
+
+            // Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
+            bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
+
+            List<string> targetResourcesFunctions = new List<string>(new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
+
+
+            foreach (FunctionDefinitionAst funcAst in functionAsts)
+            {
+                // Finds all ParamAsts.
+                IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
+
+                // Iterrates all ParamAsts and check if their names are on the list.
+
+                HashSet<string> dscVariables = new HashSet<string>();
+                if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
+                {
+                    // don't raise the rules for variables in the param block.
+                    if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
+                    {
+                        dscVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
+                    }
+                }
+                // only raise the rules for variables in the param block.
+                if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
+                {
+                    foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
+                    {
+                        if (Helper.Instance.IsUninitialized(paramAst.Name, funcAst) && !dscVariables.Contains(paramAst.Name.VariablePath.UserPath))
+                        {
+                            yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueError, paramAst.Name.VariablePath.UserPath),
+                            paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
+                        }
+                    }
+                }
+
+                if (funcAst.Parameters != null)
+                {
+                    foreach (var paramAst in funcAst.Parameters)
+                    {
+                        if (Helper.Instance.IsUninitialized(paramAst.Name, funcAst) && !dscVariables.Contains(paramAst.Name.VariablePath.UserPath))
+                        {
+                            yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueError, paramAst.Name.VariablePath.UserPath),
+                            paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
+                        }
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ProvideDefaultParameterValueName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.ProvideDefaultParameterValueDescription);
+        }
+
+        /// <summary>
+        /// Method: Retrieves the type of the rule: builtin, managed or module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Method: Retrieves the module/assembly name the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Rules/ScriptAnalyzerBuiltinRules.csproj

@@ -59,6 +59,7 @@
     <Compile Include="AvoidReservedParams.cs" />
     <Compile Include="AvoidShouldContinueWithoutForce.cs" />
     <Compile Include="AvoidTrapStatement.cs" />
+    <Compile Include="ProvideDefaultParameterValue.cs" />
     <Compile Include="AvoidUninitializedVariable.cs" />
     <Compile Include="AvoidUsernameAndPasswordParams.cs" />
     <Compile Include="AvoidUsingComputerNameHardcoded.cs" />

Rules/Strings.resx

@@ -714,4 +714,16 @@
   <data name="DscExamplesPresentNoExamplesError" xml:space="preserve">
     <value>No examples found for resource '{0}'</value>
   </data>
+  <data name="ProvideDefaultParameterValueCommonName" xml:space="preserve">
+    <value>Default Parameter Values</value>
+  </data>
+  <data name="ProvideDefaultParameterValueDescription" xml:space="preserve">
+    <value>Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters</value>
+  </data>
+  <data name="ProvideDefaultParameterValueError" xml:space="preserve">
+    <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>
+  </data>
+  <data name="ProvideDefaultParameterValueName" xml:space="preserve">
+    <value>ProvideDefaultParameterValue</value>
+  </data>
 </root>

Tests/Engine/InvokeScriptAnalyzer.tests.ps1

@@ -136,12 +136,12 @@ Describe "Test IncludeRule" {
     Context "IncludeRule supports wild card" {
         It "includes 1 wildcard rule"{
             $includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules
-            $includeWildcard.Count | Should be 5
+            $includeWildcard.Count | Should be 3
         }
 
         it "includes 2 wildcardrules" {
             $includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules 
-            $includeWildcard.Count | Should be 9
+            $includeWildcard.Count | Should be 7
         }
     }
 }

Tests/Engine/RuleSuppression.ps1

@@ -6,7 +6,7 @@ Param(
 function SuppressMe ()
 {
     [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideVerboseMessage")]
-    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUninitializedVariable", "unused1")]
+    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideDefaultParameterValue", "unused1")]
     Param([string]$unUsed1, [int] $unUsed2)
     {
         Write-Host "I do nothing"

Tests/Engine/RuleSuppression.tests.ps1

@@ -33,7 +33,7 @@ Describe "RuleSuppressionWithoutScope" {
 
     Context "RuleSuppressionID" {
         It "Only suppress violations for that ID" {
-            $suppression = $violations | Where-Object {$_.RuleName -eq "PSAvoidUninitializedVariable" }
+            $suppression = $violations | Where-Object {$_.RuleName -eq "PSProvideDefaultParameterValue" }
             $suppression.Count | Should Be 1
         }
     }

Tests/Rules/ProvideDefaultParameterValue.ps1

@@ -0,0 +1,20 @@
+function BadFunc
+{
+    param(
+        [Parameter(Mandatory=$true)]
+        [ValidateNotNullOrEmpty()]    
+        [string]
+        $Param1,
+        [Parameter(Mandatory=$false)]
+        [ValidateNotNullOrEmpty()]    
+        [string]
+        $Param2
+    )
+    $Param1
+    $Param1 = "test"
+}
+
+function BadFunc2($Param1)
+{
+    $Param1
+}

Tests/Rules/ProvideDefaultParameterValue.tests.ps1

@@ -0,0 +1,24 @@
+Import-Module PSScriptAnalyzer
+$violationName = "PSProvideDefaultParameterValue"
+$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"
+$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
+$violations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValue.ps1 | Where-Object {$_.RuleName -match $violationName}
+$noViolations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValueNoViolations.ps1
+
+Describe "ProvideDefaultParameters" {
+    Context "When there are violations" {
+        It "has 2 provide default parameter value violation" {
+            $violations.Count | Should Be 2
+        }
+
+        It "has the correct description message" {
+            $violations[0].Message | Should Match $violationMessage
+        }
+    }
+
+    Context "When there are no violations" {
+        It "returns no violations" {
+            $noViolations.Count | Should Be 0
+        }
+    }
+}

Tests/Rules/ProvideDefaultParameterValueNoViolations.ps1

@@ -0,0 +1,19 @@
+function GoodFunc
+{
+    param(
+        [Parameter(Mandatory=$true)]
+        [ValidateNotNullOrEmpty()]    
+        [string]
+        $Param1,
+        [Parameter(Mandatory=$false)]
+        [ValidateNotNullOrEmpty()]    
+        [string]
+        $Param2=$null
+    )
+    $Param1
+}
+
+function GoodFunc2($Param1 = $null)
+{
+    $Param1
+}