PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.85k stars 373 forks source link

`PSAvoidDefaultValueForMandatoryParameter` false positive when using parameter sets #1893

Closed iRon7 closed 1 year ago

iRon7 commented 1 year ago

Steps to reproduce

.\Test.ps1 contains:

function Test {
    Param (
        [Parameter(ParameterSetName = '1', Mandatory = $True)]
        [String]$a,

        [Parameter(ParameterSetName = '2', Mandatory = $True)]
        [Parameter(ParameterSetName = '1', Mandatory = $False)]
        [String]$b = "Default value"
    )
    $a,$b
}
. .\Test.ps1
Test -?

NAME
    Test

SYNTAX
    Test -a <string> [-b <string>] [<CommonParameters>]
    Test -b <string> [<CommonParameters>]

ALIASES
    None

REMARKS
    None
Invoke-ScriptAnalyzer .\Test.ps1

Expected behavior

No analyzer warnings (The rule should only apply when the parameter is mandatory in all parameter sets)

Actual behavior

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSAvoidDefaultValueForMandatoryPara Warning      Test.ps1   8     Mandatory Parameter 'b' is initialized in the Param block. T
meter                                                             o fix a violation of this rule, please leave it uninitialize
                                                                  d.

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

Environment data

$PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.2.9
PSEdition                      Core
GitCommitId                    7.2.9
OS                             Microsoft Windows 10.0.19044
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

(Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.21.0

The funny part is that no issues are found when the Test function is supplied as a script:

Param (
    [Parameter(ParameterSetName = '1', Mandatory = $True)]
    [String]$a,

    [Parameter(ParameterSetName = '2', Mandatory = $True)]
    [Parameter(ParameterSetName = '1', Mandatory = $False)]
    [String]$b = "Default value"
)
$a,$b
Invoke-ScriptAnalyzer .\Test.ps1
bergmeister commented 1 year ago

The rule is quite simple and goes through all attributes and if it finds a mandatory attribute, it gives the warning https://github.com/PowerShell/PSScriptAnalyzer/blob/8826d1cab3a39af3e03b413b858bc3a9a22753c0/Rules/AvoidDefaultValueForMandatoryParameter.cs#L50-L61 However, I don't think PSSA should do analysis of parameter sets not just because it can get complicated to reach the correct logic conclusion but also b) the rule principle still applies that defaults for mandatory parameters do not make sense. It is therefore up to you as an author to decide whether a) you remove the default or b) you decide to suppress because it's worthwhile to you keeping a default because of the other parameter set. Therefore I'd say this by design that the author should review the warning and decide what to do with it.

iRon7 commented 1 year ago

Noted