[Meta issue] PSUseDeclaredVarsMoreThanAssignments presents false positives

[rjmholt]

I'm assembling this issue as more of a map/canonical issue for all the issues we have today around the PSUseDeclaredVarsMoreThanAssignments rule, just so it's easier to deduplicate new issues and consolidate some of the discussion and explanation around this rule.

You might notice we have a dedicated label for this rule, and that's because its false positives come up a lot. There's an explanation of those below, but first some links to specific issues:

• Unaware of cmdlets that implicitly dot-source: Cmdlets that run in current scope (like ForEach-Object) not accounted for by PSUseDeclaredVarsMoreThanAssignments #1163
• Unaware of dot-sourcing: PSUseDeclaredVarsMoreThanAssignments is not aware of the scriptblock context (invocation or dot-sourcing) #938
• Unaware of specially-scoped variables: PSUseDeclaredVarsMoreThanAssignments unaware of global/env variables #663
• Unaware of Pester: PSUseDeclaredVarsMoreThanAssignments is not aware of Pester's scoping #946
• Unaware of remote contexts: Script analyzer warns about unused variables when variable is used in Invoke-Command #825
• Unaware of module-exported/-defined variables: Module exported variables should not be flagged with PSUseDeclaredVarsMoreThanAssignment #819

I've written an explanation of some of this before in this comment: #711 (comment). However, since this is intended to be a reference issue, I'll expand on that here.

Why doesn't PSUseDeclaredVarsMoreThanAssignments work?

PowerShell has something called dynamic scope, meaning scopes are inherited based on the runtime call stack, rather than the lexical scope of the script. So when a variable is referenced, its value is determined not by reading up the page of the editor (where was $x defined last in my script) but by the caller (when was $x defined last at runtime). This is technically impossible to analyse generally because it's possible to pick up variable values on the fly from any caller.

For example:

$x = 7

function Test-X
{
    $x
}

function Test-InnerX
{
    $x = 5
    Test-X
}

$x             # 7
Test-X         # 7
Test-InnerX    # 5 (Even though we ultimately call Test-X, it gets its value by reading up the stack and hits Test-InnerX's definition first)
$x             # 7 (Just to prove that Test-InnerX didn't set the outer $x)

How many times is outer $x referred to here? Depends on where Test-X is called, because the $x it references it resolved at call time, not at definition time like it would be in Python for example.

Also "call time" here could mean after script execution is started:

$x = 7
function Test-X { $x }

[scriptblock]::Create("Test-X").Invoke()

Or even:

Set-Content -Path ./script.ps1 -Value "Test-X"

function Test-X { $x }

$x = 111

./script.ps1     # 111

In fact this is further complicated because:

• PowerShell has different variable scopes, like global:, script:, local:.

• PowerShell doesn't just pick up variable values dynamically, it can also set them dynamically. Consider:

  Set-Variable x 'Hello'
  $x

  Or indeed:

  Set-Item 'variable:/x'
  $x

  And perhaps that could be analysed, but what about this:

  $variableName = Get-Content -Raw ./varname.txt
  Set-Variable $variableName 'Hello'
  $x

  In this scenario, there's no way to know if $x was set without examining the local filesystem. And we could make it worse, because we could get the variable name from a web API or from Active Directory or absolutely anything and the only way to know the value would be to run the script, which probably does things, so we can't do that (and if we could, how do we know we'd get the same results -- the web API might return something different for example). Meaning we simply can't know.

But of course there are some scenarios where a human can read the script and know, meaning that a sufficiently enlightened analyser could also know. What's the solution there? Probably to special-case those scenarios, but that takes time and work and diligence (things that seem simple to read as a human are often quite painstaking to implement as a PSScriptAnalyzer rule).

If it's not possible for it to always get things right, what's the point?

So based on the above, we've said this is an impossible problem to solve in general. Maybe we can special case things, but what's the point if there are always going to be things we can't determine?

Well there are a few reasons:

1. PowerShell is a very dynamic language. There are a number of other rules that are also undermined by how hard to analyse PowerShell actually is. So if we apply logic about being able to formally decide in every scenario whether a violation has occurred there are a number of rules in use today that we'd need to strip out and probably a number of people are going to be unhappy. We've already committed to doing to good job here, so we just have to do our best.

2. As a heuristic, this rule is actually pretty helpful. Most of the time it helps to flag actual bugs. And that's generally in keeping with the philosophy of PSScriptAnalyzer.

3. More than that, most of the open issues about this rule are about specific cases that we could work around, rather than the particularly pathological examples given above. So it could be made more helpful.

4. Finally, like with all linters, the point is also to help you improve your code style. If you hit a case that's bad enough that PSUseDeclaredVarsMoreThanAssignments gets it wrong, it's also an indicator that your code is hard to analyse (possibly also to a human), and that it might be worth reconsidering the pathological construct you've used. An example:

   $x = 10
   Invoke-Expression 'x is $x'

   Here the rule can't see into Invoke-Expression's string argument to know that $x is being referenced,
   but the scripter really shouldn't be using Invoke-Expression, so the warning is helpful if a bit indirect.

Ok, so what now?

Well we've consolidated the issues into a few scenarios, and in some cases there have even been some attempts to fix them:

• Dot-sourcing awareness: https://github.com/rjmholt/PSScriptAnalyzer/tree/declaredvariable-childscope (stuck trying to get parameter binding right for cmdlets)
• Pester awareness: https://github.com/rjmholt/PSScriptAnalyzer/tree/pester-variable-analysis (got rather complicated trying to special-case Pester's structure)

We have a reasonable idea of what the cases are, but the work-to-result ratio has meant we haven't been able to prioritise the work. On the other hand, PSScriptAnalyzer always accepts contributions and the maintainers are here to help!

[wsmelton]

A nice option would be to allow us to control when this rule (or any other) is suppressed or not. Right now:

• functions or cmdlets via Diagnostics.CodeAnalysis.SuppressMessageAttribute.
• project/workspace as a whole using psd1 file

If we had some way of doing this at a script level would benefit and allow author to control ignoring rules they do not want (or know can't cover the structure in the script). I'm in the boat of Pester so it is the one rule that comes back right now on every file.

At most if I had the ability to tell VS Code to ignore this rule for *.tests.ps1 files would help clear the problems panel 😬 .

[rjmholt]

@wsmelton I've added your scenario to #1483

[Vacant0mens]

A couple of the examples given in the initial post seem like scenarios that would merit a warning to the writer that they've done something else wrong (overshadowed from outer scope, double-assigning before reference, etc). Or is that the kind of analysis that you're referring to? If not, those seem a bit out of scope.

[JustinGrote]

I had to fully disable the rule because I couldn't get a suppress attribute to work anywhere in this pester test (The UpdateGithubReleaseCommonParams gets used as a splat in future It statements and it's not getting recognized). Guidance would be appreciated but I don't think it'll work.

https://github.com/JustinGrote/Press/blob/main/Source/Public/Update-GithubRelease.Tests.ps1

[Jaykul]

I had to fully disable the rule because I couldn't get a suppress attribute to work anywhere in this pester test

Yeah, ScriptAnalyzer doesn't understand Pester syntax, so it can't tell that those script blocks all execute in the same context...

[JustinGrote]

@Jaykul A workaround is to set the variables in the BeforeAll at Script scope it seems.