Fix AvoidDefaultValueForMandatoryParameter documentation, rule and tests (#907)

* make AvoidDefaultValueForMandatoryParameter also warn when not using cmdletbinding and fix documentation: TODO: tidy up tests

* adapt tests

Author: bergmeister

RuleDocumentation/AvoidDefaultValueForMandatoryParameter.md

@@ -4,29 +4,35 @@
 
 ## Description
 
-Just like non-global scoped variables, parameters must have a default value if they are not mandatory, i.e `Mandatory=$false`.
-Having optional parameters without default values leads to uninitialized variables leading to potential bugs.
-
-## How
-
-Specify a default value for all parameters that are not mandatory.
+Mandatory parameters should not have a default values because there is no scenario where the default can be used because `PowerShell` will prompt anyway if the parameter value is not specified when calling the function.
 
 ## Example
 
 ### Wrong
 
 ``` PowerShell
-function Test($Param1)
+function Test
 {
-	$Param1
+
+    [CmdletBinding()]
+    Param
+    (
+        [Parameter(Mandatory=$true)]
+        $Parameter1 = 'default Value'
+    )
 }
 ```
 
 ### Correct
 
 ``` PowerShell
-function Test($Param1 = $null)
+function Test
 {
-	$Param1
+    [CmdletBinding()]
+    Param
+    (
+        [Parameter(Mandatory=$true)]
+        $Parameter1
+    )
 }
 ```

Rules/AvoidDefaultValueForMandatoryParameter.cs

@@ -1,19 +1,8 @@
-//
-// 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.
-//
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
 
 using System;
-using System.Linq;
 using System.Collections.Generic;
-using System.Management.Automation;
 using System.Management.Automation.Language;
 #if !CORECLR
 using System.ComponentModel.Composition;
@@ -24,15 +13,15 @@
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
     /// <summary>
-    /// ProvideDefaultParameterValue: Check if any uninitialized variable is used.
+    /// AvoidDefaultValueForMandatoryParameter: Check if a mandatory parameter does not have a default value.
     /// </summary>
 #if !CORECLR
 [Export(typeof(IScriptRule))]
 #endif
     public class AvoidDefaultValueForMandatoryParameter : IScriptRule
     {
         /// <summary>
-        /// AnalyzeScript: Check if any uninitialized variable is used.
+        /// AnalyzeScript: Check if a mandatory parameter has a default value.
         /// </summary>
         public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
         {
@@ -46,12 +35,6 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 if (funcAst.Body != null && funcAst.Body.ParamBlock != null
                     && funcAst.Body.ParamBlock.Attributes != null && funcAst.Body.ParamBlock.Parameters != null)
                 {
-                    // only raise this rule for function with cmdletbinding
-                    if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute)))
-                    {
-                        continue;
-                    }
-
                     foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
                     {
                         bool mandatory = false;

Tests/Rules/AvoidDefaultValueForMandatoryParameter.ps1

@@ -1,24 +1,26 @@
 Import-Module PSScriptAnalyzer
-$violationName = "PSAvoidDefaultValueForMandatoryParameter"
-$violationMessage = "Mandatory Parameter 'Param1' is initialized in the Param block. To fix a violation of this rule, please leave it unintialized."
-$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
-$violations = Invoke-ScriptAnalyzer "$directory\AvoidDefaultValueForMandatoryParameter.ps1" | Where-Object {$_.RuleName -match $violationName}
-$noViolations = Invoke-ScriptAnalyzer "$directory\AvoidDefaultValueForMandatoryParameterNoViolations.ps1"
+$ruleName = 'PSAvoidDefaultValueForMandatoryParameter'
 
 Describe "AvoidDefaultValueForMandatoryParameter" {
     Context "When there are violations" {
-        It "has 1 provide default value for mandatory parameter violation" {
+        It "has 1 provide default value for mandatory parameter violation with CmdletBinding" {
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ [CmdletBinding()]Param([Parameter(Mandatory)]$Param1=''defaultValue'') }' |
+                Where-Object { $_.RuleName -eq $ruleName }
             $violations.Count | Should -Be 1
         }
 
-        It "has the correct description message" {
-            $violations[0].Message | Should -Match $violationMessage
+        It "has 1 provide default value for mandatory=$true parameter violation without CmdletBinding" {
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=$true)]$Param1=''defaultValue'') }' |
+                Where-Object { $_.RuleName -eq $ruleName }
+            $violations.Count | Should -Be 1
         }
     }
 
     Context "When there are no violations" {
-        It "returns no violations" {
-            $noViolations.Count | Should -Be 0
+        It "has 1 provide default value for mandatory parameter violation" {
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=$false)]$Param1=''val1'', [Parameter(Mandatory)]$Param2=''val2'', $Param3=''val3'') }' |
+                Where-Object { $_.RuleName -eq $ruleName }
+            $violations.Count | Should -Be 0
         }
     }
 }