Skip to content
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

ReviewUnusedParameter does not capture parameters implicitly used to determine ParameterSetName #1527

Closed
ThePoShWolf opened this issue Jun 16, 2020 · 1 comment

Comments

@ThePoShWolf
Copy link

Steps to reproduce

Function Get-Thing {
    [cmdletbinding(
        DefaultParameterSetName = '1'
    )]
    param (
        [Parameter(
            ParameterSetName = '2'
        )]
        [switch]$ParamSet2,
        [Parameter(
            ParameterSetName = '3'
        )]
        [switch]$ParamSet3
    )
    Switch ($PSCmdlet.ParameterSetName) {
        '1' {
            # ParameterSet = 1
        }
        '2' {
            # ParameterSet = 2
        }
        '3' {
            # ParameterSet = 3
        }
    }
    # Get the thing
}

Check it with the default rules in PSScriptAnalyzer:

Invoke-ScriptAnalyzer -Path .\Get-Thing.ps1

RuleName                Severity ScriptName    Line Message
--------                -------- ----------    ---- -------
PSReviewUnusedParameter Warning  Get-Thing.ps1 9    The parameter 'ParamSet2' has been declared but not used.
PSReviewUnusedParameter Warning  Get-Thing.ps1 13   The parameter 'ParamSet3' has been declared but not used.

Is it objectively better to rewrite the function? Something like:

Function Get-Thing {
    [cmdletbinding(
        DefaultParameterSetName = '1'
    )]
    param (
        [Parameter(
            ParameterSetName = '2'
        )]
        [switch]$ParamSet2,
        [Parameter(
            ParameterSetName = '3'
        )]
        [switch]$ParamSet3
    )
    if ($ParamSet2.IsPresent) {
        # ParameterSet = 2
    } elseif ($ParamSet3.IsPresent) {
        # ParameterSet = 3
    } else {
        # ParameterSet = 1
    }
    # Get the thing
}

This is what I've been doing for the functions that have been caught with this error.

Expected behavior

I would expect the rule not to be triggered on this script.

Actual behavior

RuleName                Severity ScriptName    Line Message
--------                -------- ----------    ---- -------
PSReviewUnusedParameter Warning  Get-Thing.ps1 9    The parameter 'ParamSet2' has been declared but not used.
PSReviewUnusedParameter Warning  Get-Thing.ps1 13   The parameter 'ParamSet3' has been declared but not used.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.2
PSEdition                      Core
GitCommitId                    7.0.2
OS                             Microsoft Windows 10.0.18362
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.19.0
@ghost ghost added the Needs: Triage 🔍 label Jun 16, 2020
@bergmeister
Copy link
Collaborator

bergmeister commented Jun 16, 2020

This sounds like a deja-vu of the original issue description in #1505
A parameter set can consist of one or more parameters. It's fine to use switch ($PSCmdlet.ParameterSetName) if it helps you to structure your code easier but the rule is still right that you do not use the actual parameter. If you use the parameters implicitly via $PSBoundParameters, then the rule won't fire and PR #1520 recently also added $MyInvocation.BoundParameters and $PSCmdlet.MyInvocation.BoundParameters to the exclusion list but that PR hasn't been released yet.
Also: The rule is not supposed to be an absolute but rather a heuristic. If you can justify your special use-case then that is what rule suppression is for.

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

No branches or pull requests

3 participants