PSReviewUnusedParameter with script parameter and function using it

[NN---]

Before submitting a bug report:

• Make sure you are able to repro it on the latest released version
• Perform a quick search for existing issues to check if this bug has already been reported

Steps to reproduce

param(
	$a
)


function f
{
  echo $a
}

f

Expected behavior

No warnings

Actual behavior

Invoke-ScriptAnalyzer .\a.ps1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSReviewUnusedParameter             Warning      a.ps1      2     The parameter 'a' has been declared but not used.

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      7.1.3
PSEdition                      Core
GitCommitId                    7.1.3
OS                             Microsoft Windows 10.0.19042
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.1

[SydneyhSmith]

Thanks @NN--- this looks related to #1641 and a duplicate of #1643

[rjmholt]

This is actually a valid warning. If you dot-source that script and call f like this:

. ./script.ps1 -a 'x'

f

You're going to see that second call of f output $null.

[NN---]

@rjmholt Cannot reproduce it:

> . .\a.ps1 -a 1
1
> f
1

[rjmholt]

Ah, yeah, I think the parameter is also dot-sourced.

Try this:

. \a.ps1 -a 1
$a = 2
f

[NN---]

Not sure I understand the problem here.
There is still no unused parameter.

> . .\a.ps1 -a 1
1
> $a
1
> $a = 2
> f
2

[rjmholt]

Not sure I understand the problem here.

The $a value being referenced in f is not necessarily the $a being passed in (because the $a reference in f, at runtime, looks up the runtime variable scope stack to determine what value to use), so in general this isn't a usage of the $a parameter and there are no other usages of $a in the immediate scope, so PSSA considers the parameter unused.

It happens that in your scenario, due to dynamic scope, it is used thanks to scope inheritance. But PSScriptAnalyzer is a static analyser; it's impossible in general for it (or any static analyser) to be able to infer a variable usage that depends on dynamic scope inheritance. In addition to that, most of the time when a parameter is used in the way that you've used it, it's because the author is expecting lexical scope rather than dynamic scope and this represents a bug in their script.

So because analysing variables from dynamic scope isn't something that PSScriptAnalyzer can feasibly provide guarantees about, and because depending on dynamic scope like this is generally not something that PSScriptAnalyzer is trying to encourage, getting a warning about the $a variable in this scenario is expected. The warning message could probably be improved, but that's part of a larger work item around improving the way the unused variable logic works in #1641.

[NN---]

Thanks for the explanation.
So what is the correct way to reference script parameter inside a function ?
Always pass it explicitly ?

[rjmholt]

So what is the correct way to reference script parameter inside a function ?
Always pass it explicitly ?

Yeah, that's the ideal method. Sometimes you do want to take advantage of dynamic scope, but that's usually in quite specific scenarios like:

• Wanting to pass a scriptblock through another command that will invoke it immediately (with that command acting as a sort of "decorator" for the scriptblock)
• Implementing a scope-state dependent logic, common in something like a domain-specific language

But generally, and especially for functions, explicitly parameterising and passing those parameters is the recommended way to do things, because:

• You can declare the parameter explicitly, so it's known what's a parameter and what's a variable
• You can depend on non-parameter variables to be locals
• You can validate parameters using things like [ValidateSet()] and make them mandatory or optional with a default
• When your invocation hits an error, you'll be able to read all the things that were passed in at the callsite in the script, rather than needing to debug it and work out what the variables on the scope stack were at call time

[NN---]

Can this be considered as a reasonable workaround or should it be also reported as unused parameter ?

param(
	$a
)

$a = $a

function f
{
  echo $a
}