PSUseDeclaredVarsMoreThanAssignments is not aware of Pester's scoping

[RemcoKapinga]

Steps to reproduce

Supposing a Pester Test file like below:

$here = $PsScriptRoot

Describe 'My Module' {

    BeforeAll { $module = Import-Module "$here\MyModule.psd1" }
    AfterAll  { $module | Remove-Module -Force }

    It 'can be loaded' {
        $module | Should Not Be $null
     }
}

Expected behavior

No warning on the $module variable, as it is used in the code below.

Actual behavior

Invoke-ScriptAnalyzer reports the following:

PSUseDeclaredVarsMoreThanAssignments  Warning  My.Tests.ps1 5  The variable 'module' is assigned but never used.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.16299.251
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.16299.251
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.16.1
1.16.0
1.15.0
1.14.1
1.14.0
1.13.0
1.12.0
1.11.1
1.11.0
1.10.0
1.9.0
1.8.1
1.8.0
1.7.0
1.6.0
1.5.0
1.4.0
1.3.0
1.2.0
1.1.1
1.1.0
1.0.2
1.0.1
1.0

[bergmeister]

This is due to #938
The rule currently does the analysis on a per scriptblock basis, therefore it does not see the connection between them. It would also mean that PSSA has to know about Pester syntax like Context, BeforeEach or BeforeAll, etc. and therefore this is not straightforward to fix but I can totally understand that this is annoying. A workaround is to use script scope instead, i.e. you assign and use the variable as $script:module.
In case you are interested, this is how it looks like in the debugger (i.e. those are the 5 scriptblocks that it analyses separately:

[RemcoKapinga]

Using $script:module is a good workaround for this complex issue.

[jhoneill]

Checking on Per scriptblock causes a lot of false positives , for example in the begin block below.

 Get-ChildItem -Path "$injectPath\all"  -Directory   |     
        ForEach-Object -Begin   {$DefaultCopyDirectories = @{} }`
                       -Process {$DefaultCopyDirectories[$_.FullName] = "\" }

[bergmeister]

@jhoneill there is already issue #804 for begin, process, end blocks.
Extending the analysis beyond the current per-scriptblock approach is unfortunately not trivial and requires a lot of thinking and experimentation to come up with the right solution. An idea would be to properly implement SSA in a way that it understands PowerShell's scoping and operations such as dot-sourcing

[jhoneill]

@bergmeister thanks, I hadn't looked at #804 ; as you say it isn't trivial to figure out but as-is there are too many false positives … but perhaps not enough to make it a "must fix"

[bergmeister]

@jhoneill Because of all issues related to PSUseDeclaredVarsMoreThanAssignments, I created a special label for it. I think the rule in its current state is very valuable but could definitely be improved. In issue #934 we were already spinning ideas. It is possible to continue tweaking and enhancing the rule but the current implementation itself has limitations, therefore using a different approach like SSA might be better and could lead to a much better behaviour but the SSA implementation and rewrite is a big undertaking. So it is a case of it maybe being better to just leave it for the moment and rather wait a bit longer until we have a better base/framework because having proper SSA will enable also other scenarios such as being able to warn on using += on objects of type array (which requires figuring out the type)

[dmitryserbin]

+1 we're having the same issue

[glennsarti]

Chiming in with a VSCode example

Given the number of false-positive issues, would it be possible to downgrade this rule violation from "Warning" to "Hint"/"Informational" or something less severe?