Rule PSAvoidShouldContinueWithoutForce doesn't enforce correct use of the Force parameter

[JohnLBevan]

Currently PSScriptAnalyzer shows a PSAvoidShouldContinueWithoutForce violation for any scripts using $PSCmdlet.ShouldContinue that do not include a $Force parameter.
However, once the parameter the violation is resolved; even if that parameter is never used elsewhere in the code.

Steps to reproduce

function Invoke-MVP {
    [CmdletBinding(SupportsShouldProcess = $true)] 
    [OutputType('System.String')]
    Param (
        [Parameter()]
        [switch]$Force # Including this parameter resolves the PSAvoidShouldContinueWithoutForce violation; even though we don't reference Force in the rest of the code
    )
    if ($PSCmdlet.ShouldContinue('Should I continue','Should I continue')) {
        'I continued'
    }
}

# ...
Invoke-ScriptAnalyzer -Path '.\Invoke-MVP.ps1'

Expected behavior

A violation of rule PSAvoidShouldContinueWithoutForce should occur.

Actual behavior

The code is deemed free of violations.

Environment data

> $PSVersionTable

Name                           Value                                                                                                                                                                                                             
----                           -----                                                                                                                                                                                                             
PSVersion                      5.1.14409.1018                                                                                                                                                                                                    
PSEdition                      Desktop                                                                                                                                                                                                           
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}                                                                                                                                                                                           
BuildVersion                   10.0.14409.1018                                                                                                                                                                                                   
CLRVersion                     4.0.30319.42000                                                                                                                                                                                                   
WSManStackVersion              3.0                                                                                                                                                                                                               
PSRemotingProtocolVersion      2.3                                                                                                                                                                                                               
SerializationVersion           1.1.0.1           
> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.18.1

[JohnLBevan]

Question: Does anyone have a preference on which line is flagged as in violation?

• [CmdletBinding(SupportsShouldProcess = $true)]
• [switch]$Force
• $PSCmdlet.ShouldContinue('Should I continue','Should I continue'))

i.e. as it's the combination of these three, plus the lack of the use of the force parameter further on which cause the issue.

My instinct is to attach the violation to the [CmdletBinding(SupportsShouldProcess = $true)] line, as this is the line the violation for not having the Force parameter is attached to, so remains consistent; but I can see how it could be argued other ways.

[JohnLBevan]

Note: All of the following scenarios should be deemed valid:

function Invoke-MVP {
    [CmdletBinding(SupportsShouldProcess = $true)] 
    [OutputType('System.String')]
    Param (
        [Parameter()]
        [switch]$Force
    )
    if ($Force -or $PSCmdlet.ShouldContinue('Should I continue','Should I continue')) {
        'I continued'
    }
}

function Invoke-MVP {
    [CmdletBinding(SupportsShouldProcess = $true)] 
    [OutputType('System.String')]
    Param (
        [Parameter(Mandatory = $true)]
        [string[]]$ComputerName
        ,
        [Parameter()]
        [switch]$Force 
    )
    [bool]$yesToAll = $Force
    [bool]$noToAll = $false
    foreach ($computer in $ComputerName) {
        if ($PSCmdlet.ShouldContinue('Should I continue','Should I continue', [ref]$yesToAll, [ref]$noToAll)) {
            "I continued: $computer"
        }
    }
}

In theory the below is also valid... but I suspect this is considered bad practice so should be reported as a violation / up to the coder to hide this via a SupressMessageAttribute attribute if they can justify going this route.

function Invoke-MVP {
    [CmdletBinding(SupportsShouldProcess = $true)] 
    [OutputType('System.String')]
    Param (
        [Parameter()]
        [switch]$Force
    )
    if ($Force) {
            'I continued... She made me do it!'
    } else {
        if ($PSCmdlet.ShouldContinue('Should I continue','Should I continue')) {
            'I continued'
        }
    }
}

Note: I wouldn't have it treat $noToAll = $Force as acceptable, since that's counter intuitive. If people wanted an option for defaulting that they'd be better of using a, $AvoidClobber / $NoForce option; which is outside the scope of this rule.

[bergmeister]

I'd flag on [CmdletBinding(SupportsShouldProcess)]. You're right that the correct usage can be hard to determine, I originally thought that we could simply check for the presence of the -Force switch but remembered only now that we already have the AvoidShouldContinueWithoutForce rule for that. Unless you've got some use cases where we know for sure that the usage is not correct, then we might need to close the issue if it is too hard to accurately make a decision of the usage...

[thomasrayner]

If you declare a parameter and then never use it, should you get a UseDeclaredVarsMoreThanAssignments violation (or something similar)? 🤔

I know this is an issue about specifically using a $Force param if you have SupportsShouldProcess, but it made me wonder about the above.

[mattmcnabb]

If you declare a parameter and then never use it, should you get a UseDeclaredVarsMoreThanAssignments violation (or something similar)? 🤔

That rule doesn't currently look at param blocks, but I think this is a good idea for a new rule. I have a POC of it that works so far but doesn't cover cases like $PSBoundParameters. Does anyone else agree that this might be a good rule to implement? If so I'll take a stab at it.