Skip to content

PSAvoidDefaultValueForMandatoryParameter: Fix param block and parameter set handling #2121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liamjpeters
Copy link
Contributor

@liamjpeters liamjpeters commented Aug 18, 2025

PR Summary

While looking into issue #1623, I noticed that the PSAvoidDefaultValueForMandatoryParameter rule has several issues. This PR attempts to resolve them and increase test coverage.

Observed issues are:

  1. Rule doesn't flag for parameters which are part of a param block not nested within a function. So the below .ps1 file contents are not flagged:

    [CmdletBinding()]
    Param
    (
        [Parameter(Mandatory)]
        $Parameter1 = 'default Value'
    )
  2. If a parameter is used in multiple parametersets, only being mandatory in one of them, the rule still flags the parameter if it has a default value. (As raised in issue AvoidDefaultValueForMandatoryParameter mistakenly reported. #1623)

    function Test
    {
        [CmdletBinding()]
        Param
        (
            [Parameter(Mandatory, ParameterSetName = "Mandatory")]
            [Parameter(ParameterSetName = "Optional")]
            $Parameter1 = 'default Value'
        )
    }
  3. When checking whether a parameter is mandatory, the rule does not validate that mandatory is a named argument of a Parameter Attribute. So the below is flagged:

    function Test {
        [CmdletBinding()]
        Param
        (
            [MyAttribute(Mandatory)]
            $Parameter1 = 'default Value'
        )
    }
  4. The below test case is faulty. The tested script definition should cause a violation and isn't due to an issue:

    Context "When there are no violations" {
    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
    }

    I believe any parameter in the block having Mandatory=$false causes the rule to break for subsequent parameters in the block.

Fixes #1623

The performance of the old rule and new rule are comparable, whilst flagging more cases. This is running the old test file (correcting for the erroneous test) 1,000 times. Times for each run reported by Pester (Duration.TotalMilliseconds field).

Average (ms) Min (ms) Max (ms)
Old Rule 108.68 92.74 174.55
New Rule 105.28 85.46 195.63

Note: Caching of the string comparer suggested by copilot as an improvement. Not sure on the effectiveness of this change. Function comment-based help also written mostly by copilot - but reading it, it seems sensible.

PR Checklist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AvoidDefaultValueForMandatoryParameter mistakenly reported.
1 participant