PSScriptAnalyzer: ReviewUnusedParameter does not capture parameter usage within a scriptblock

Steps to reproduce

Run Invoke-ScriptAnalyzer against the following with the new 1.19.0 release.

function foo {
    Param(
        $MyParameter
    )

    Get-Item| ForEach-Object { Get-ChildItem $MyParameter }
}

Expected behavior

No rule violations.

Actual behavior

The new ReviewUnusedParameter rule doesn’t notice the usage. I suspect this is similar to the limitation of the AvoidUsingCmdletAliases rule though. Not sure if we should relax the ReviewUnusedParameter rule in this case to search nested scriptblocks inside a function scope. cc @mattmcnabb @rjmholt @JamesWTruher

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSReviewUnusedParameter             Warning      test.ps1   4     The parameter 'MyParameter' has been declared but not used.

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.1.0-preview.2
PSEdition                      Core
GitCommitId                    7.1.0-preview.2
OS                             Microsoft Windows 10.0.18363
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

About this issue

  • Original URL
  • State: open
  • Created 4 years ago
  • Reactions: 32
  • Comments: 29 (3 by maintainers)

Commits related to this issue

Most upvoted comments

@robinmalik the suppression example you wrote above works as-is, but with the attribute attached to the param block, not the parameter. Thanks for the tip though, because we had this rule disabled for our scripts, but this will be a great improvement!

Amazing, thanks for the clarification there. I can confirm it works now 😃 For anyone else getting stuck like I did:

[CmdletBinding(SupportsShouldProcess)]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'SkipModules', Justification = 'false positive')]
param(
   [Array]$SkipModules
   ...
)

I have tried suppressing the warning as you suggested:

[Parameter(Mandatory=$false, HelpMessage="The SystemAttributeValue.ObjectId property (Required)")]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '$ObjectId', Justification = 'false positive')]

But I am still getting the PSScriptAnalyzer warning. What am I doing wrong?

To suppress PSReviewUnusedParameter for a specific parameter you should set SuppressMessageAttribute’s checkId to the name of the parameter without the $ prefix.

In this case, the simple solution is to also look in the child scriptblock. The better solution is to search the child scriptblock when the command is ForEach-Object or Where-Object

To add to the list, we’re also seeing this fail with Invoke-Command -ScriptBlock {} (though this using $using:varName), and with usage via @PSBoundParameters, which is frustrating. I’m unsure what to suggest, though.

This caused some of my CI pipelines to suddenly fail today. Since I hadn’t pinned the version of PSScriptAnalyzer that was used in the pipelines, and since I hadn’t updated to 1.19.0 locally, I thought something was wrong with the pipeline or that I’d inadvertently introduced an error to our scripts.

As a user I’d expect to receive a warning only if there really is an issue. I like the idea of the rule, but I’d rather forgo it entirely than have to add workarounds to our scripts or toggle Strict mode in certain cases. If a Strict config were to be added, I’d expect it to be off by default so that one would only (possibly) see false positive if one deliberately turned on the rule.

In case I’m not understanding what the Strict config would do, the behavior I’d expect as a user is: 1) by default, don’t check for unused parameters at all, 2) require the user to explicitly enable the rule and 3) indicate that the rule may yield false positives (I would have appreciated a mention of that directly in the warning message).

(This is my first time commenting on a PSScriptAnalyzer issue or PR. -Even though this new rule has been negative for me, I want to thank you for your work on this tool–it’s been a great help not only for ensuring a clear and consistent style for our scripts but also for teaching how to use PowerShell.)

Heya, as this rule has been bugging me for some time now, I’ve done a quick PR for a proposed solution, traversing into the scriptblocks passed to whitelisted commands (like Where-Object). Of course configurable to extend the list as the user prefers: https://github.com/PowerShell/PSScriptAnalyzer/pull/1921

Any thoughts on that approach?

@robinmalik I do not see the behaviour that you see, the following does not report a warning

function Test {
    Param (
        [String]$a
    )
    if ($a) {
        return
    }
}

@robinmalik the suppression example you wrote above works as-is, but with the attribute attached to the param block, not the parameter. Thanks for the tip though, because we had this rule disabled for our scripts, but this will be a great improvement!

I’m also seeing this for .foreach loops. I’m not seeing it happening for standard foreach() loops, however. I assume it has to do with a .foreach being a script block and foreach() not being a script block.

@bergmeister I have tried suppressing the warning as you suggested:

        [Parameter(Mandatory=$false, HelpMessage="The SystemAttributeValue.ObjectId property (Required)")]
        [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '$ObjectId', Justification = 'false positive')]
        [int] $ObjectId

But I am still getting the PSScriptAnalyzer warning. What am I doing wrong? Do I need to suppress it at the cmdlet level for the specific parameter?, ie

function Assert-ValidSystemAttributeValue {
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '$ObjectId', Justification = 'false positive')]
    [CmdletBinding()]
    param(

I dunno, I’ve tried several things and none of them are working, and I’m not seeing any real documentation on it.

There is also this proposal in PowerShell to help PSSA: PowerShell/PowerShell#12287

Yeah, this proposal would help in the cases we don’t know about, but most of the time people use ForEach-Object and we already know. The hard part for us is not knowing the common commands that do this, but being able to perform the analysis once we know.

In this case, the simple solution is to also look in the child scriptblock. The better solution is to search the child scriptblock when the command is ForEach-Object or Where-Object