PSScriptAnalyzer icon indicating copy to clipboard operation
PSScriptAnalyzer copied to clipboard

Cmdlets that run in current scope (like ForEach-Object) not accounted for by PSUseDeclaredVarsMoreThanAssignments

Open iRon7 opened this issue 5 years ago • 9 comments

Although issue #1031 and #1129 probably have the same cause, I have added a new bug report as it is (afaik) not directly related to the Begin, Process and End function blocks as suggested in there which brings the issue in a different perspective.

Apparently for some cmdlets, PowerShell is invoked in the current scope but that is apparently not respected by the PSScriptAnalyzer.

Steps to reproduce

$Test = $False
1..3 | ForEach-Object {$Test = $True}
$Test

note that the result of the above is $True, meaning that the value of $Test is actually assigned and changed to $True within the ForEach-Object cmdlet.

Expected behavior

No warning.

Actual behavior

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseDeclaredVarsMoreThanAssignment Warning      Test.ps1   2     The variable 'Test' is assigned but never used.

Environment data

Name                           Value
----                           -----
PSVersion                      5.1.17134.590
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17134.590
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

PSScriptAnalyzer.Version:    1.17.1

Will this eventually be resolved? How can I nicely avoid the incorrect warning in an example like above?

iRon7 avatar Mar 04 '19 08:03 iRon7

Hello, @robfaie and I been looking into this issue. The problem is that assignments made in script blocks for functions like Where-Object and Select-Object are accessible after the fact but similar blocks for functions like Invoke-Command are not. I put together a little PoC that skips processing for Where-Object script blocks. I wanted to touch base before putting more time into this to discuss the approach.

image

VonRegi avatar Mar 20 '19 06:03 VonRegi

I do not have enough C# knowledge to comment on the specific code, but I am a little confused that the focus shifted to the Where-Object cmdlet instead of the ForEach-Object cmdlet example. Although, the issue also exist in the Where-Object cmdlet, the Where-Object cmdlet is mainly used for comparison and therefore unlikely contains a variable assignment (or do I miss something?).
For the ForEach-Object cmdlet, I can give you used case, that might occur (to my opinion) more often:
I have written a PowerShell Join-Object cmdlet (https://www.powershellgallery.com/packages/Join).
This cmdlet has a -Property parameter which similar to the Select-Object -Property parameter where it accepts one or more property name, calculated properties and/or a asterisks for all know properties. Basically, something like:

$All = $False
$Property | ForEach-Object {
	If ($_ -eq "*") {$All = $True}
	ElseIf ($_ -eq a calcultated property...
}
If ($All) {
	Add any known property to output object that is not filled by a calculated property
}
...

Besides this example, I also imaging that a state variable like a $Dirty flag could be quiet often set in a ForEach-Object cmdlet.

iRon7 avatar Mar 21 '19 18:03 iRon7

Currently the rule is limited to analysis within a scriptblock. Extending this functionality would require more thinking how to handle the scoping behaviour that PowerShell exhibits.

bergmeister avatar Mar 21 '19 22:03 bergmeister

I'm seeing the same issue. Here is a contrived repro:

Get-Process | ForEach-Object -Begin { $ProcessCount = 0 } -Process { $ProcessCount += 1 } -End { $ProcessCount }
Screen Shot 2019-09-30 at 8 20 26 PM

pcgeek86 avatar Oct 01 '19 03:10 pcgeek86

Regardless if this is easy to detect or not, having warnings all over the place because the variable is "never used" is less than useless. I would almost suggest removing this rule until it is better at detection. Either that or assume everything is in the same scope for now, which would only cause things not in scope to get a false "pass". I think this would be a much better situation than what we currently have.

FireInWinter avatar Mar 05 '20 16:03 FireInWinter

This is a known issue due to the rule being limited to the scope of a scriptblock. If you look for issues with the Area - PSUseDeclaredVarsMoreThanAssignments label, you will find similar reports. Specifically, #1031 already covers Begin/Process/End blocks, therefore I suggest to close as duplicate. @fireInWinter I disagree with that. The rule definitely has value, in some cases you have to explicitly look at the situation and suppress it on a case by case basis. If it is too much for you, you can globally suppress it using the -ExcludeRule switch or by using the equivalent in a PSSA settings file, which VS-Code would be able to use.

bergmeister avatar Mar 05 '20 17:03 bergmeister

Another cmdlet I've stumbled upon that's impacted by this is Measure-Command. The -Expression scriptblock it takes in is also executed in current scope.

amis92 avatar Apr 17 '20 22:04 amis92

Why does this fire on $global:variables if it only accounts for a single scriptblock? Makes the check entirely useless.

nascentt avatar Jan 21 '22 11:01 nascentt