From c80fe89aee817ee0b4147a71f477e19520020d19 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 25 Feb 2018 23:01:39 +0000 Subject: [PATCH 1/2] make AvoidDefaultValueForMandatoryParameter also warn when not using cmdletbinding and fix documentation: TODO: tidy up tests --- .../AvoidDefaultValueForMandatoryParameter.md | 26 ++++++++++++------- .../AvoidDefaultValueForMandatoryParameter.cs | 25 +++--------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/RuleDocumentation/AvoidDefaultValueForMandatoryParameter.md b/RuleDocumentation/AvoidDefaultValueForMandatoryParameter.md index 70f56f783..6b1a3eb57 100644 --- a/RuleDocumentation/AvoidDefaultValueForMandatoryParameter.md +++ b/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 + ) } ``` diff --git a/Rules/AvoidDefaultValueForMandatoryParameter.cs b/Rules/AvoidDefaultValueForMandatoryParameter.cs index e3a9d63f4..cb7aa31a4 100644 --- a/Rules/AvoidDefaultValueForMandatoryParameter.cs +++ b/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,7 +13,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// ProvideDefaultParameterValue: Check if any uninitialized variable is used. + /// AvoidDefaultValueForMandatoryParameter: Check if a mandatory parameter does not have a default value. /// #if !CORECLR [Export(typeof(IScriptRule))] @@ -32,7 +21,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class AvoidDefaultValueForMandatoryParameter : IScriptRule { /// - /// AnalyzeScript: Check if any uninitialized variable is used. + /// AnalyzeScript: Check if a mandatory parameter has a default value. /// public IEnumerable AnalyzeScript(Ast ast, string fileName) { @@ -46,12 +35,6 @@ public IEnumerable 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; From 27dd48475c04f909df7cd552d281dfd694c10fa9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 25 Feb 2018 23:25:56 +0000 Subject: [PATCH 2/2] adapt tests --- ...AvoidDefaultValueForMandatoryParameter.ps1 | 42 ------------------- ...efaultValueForMandatoryParameter.tests.ps1 | 22 +++++----- ...ValueForMandatoryParameterNoViolations.ps1 | 19 --------- 3 files changed, 12 insertions(+), 71 deletions(-) delete mode 100644 Tests/Rules/AvoidDefaultValueForMandatoryParameter.ps1 delete mode 100644 Tests/Rules/AvoidDefaultValueForMandatoryParameterNoViolations.ps1 diff --git a/Tests/Rules/AvoidDefaultValueForMandatoryParameter.ps1 b/Tests/Rules/AvoidDefaultValueForMandatoryParameter.ps1 deleted file mode 100644 index d1cccd86b..000000000 --- a/Tests/Rules/AvoidDefaultValueForMandatoryParameter.ps1 +++ /dev/null @@ -1,42 +0,0 @@ -function BadFunc -{ - [CmdletBinding()] - param( - # this one has default value - [Parameter(Mandatory=$true)] - [ValidateNotNullOrEmpty()] - [string] - $Param1="String", - # this parameter has no default value - [Parameter(Mandatory=$false)] - [ValidateNotNullOrEmpty()] - [string] - $Param2 - ) - $Param1 - $Param1 = "test" -} - -function GoodFunc1($Param1) -{ - $Param1 -} - -# same as BadFunc but this one has no cmdletbinding -function GoodFunc2 -{ - param( - # this one has default value - [Parameter(Mandatory=$true)] - [ValidateNotNullOrEmpty()] - [string] - $Param1="String", - # this parameter has no default value - [Parameter(Mandatory=$false)] - [ValidateNotNullOrEmpty()] - [string] - $Param2 - ) - $Param1 - $Param1 = "test" -} \ No newline at end of file diff --git a/Tests/Rules/AvoidDefaultValueForMandatoryParameter.tests.ps1 b/Tests/Rules/AvoidDefaultValueForMandatoryParameter.tests.ps1 index 1f1e205be..a303083e8 100644 --- a/Tests/Rules/AvoidDefaultValueForMandatoryParameter.tests.ps1 +++ b/Tests/Rules/AvoidDefaultValueForMandatoryParameter.tests.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 } } } \ No newline at end of file diff --git a/Tests/Rules/AvoidDefaultValueForMandatoryParameterNoViolations.ps1 b/Tests/Rules/AvoidDefaultValueForMandatoryParameterNoViolations.ps1 deleted file mode 100644 index 0215f9252..000000000 --- a/Tests/Rules/AvoidDefaultValueForMandatoryParameterNoViolations.ps1 +++ /dev/null @@ -1,19 +0,0 @@ -function GoodFunc -{ - param( - [Parameter(Mandatory=$true)] - [ValidateNotNullOrEmpty()] - [string] - $Param1, - [Parameter(Mandatory=$false)] - [ValidateNotNullOrEmpty()] - [string] - $Param2=$null - ) - $Param1 -} - -function GoodFunc2($Param1 = $null) -{ - $Param1 -} \ No newline at end of file