Skip to content

PSUseDeclaredVarsMoreThanAssignment not correctly handling global vars assigned to within functions #698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HowardWolosky opened this issue Jan 31, 2017 · 0 comments

Comments

@HowardWolosky
Copy link

This rule does not correctly take into account global variables if they are assigned to within a function.

If I set a global variable and then use it later, I still see a warning that the global variable was assigned to, but never used.

Given the following example:

function InitVars()
{
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidGlobalVars", "", Justification="We need a global foo")]
    param()

    $global:globalFoo = $false
    $script:scriptFoo = $false
}

function UseVars()
{
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidGlobalVars", "", Justification="We need a global foo")]
    param()

    if ($global:globalFoo)
    {
        Write-Information "`$global:globalFoo is true"
    }

    if ($scriptFoo)
    {
        Write-Information "`$script:scriptFoo is true"
    }
}

I get the following warning:

  • The variable 'globalFoo' is assigned but never used. (line 6)

There are scenarios where using global variables is still necessary, and so in those few instances I am suppressing PSAvoidGlobalVars. To do that, I have to do the assignment within a method.

As you can see above, it hits with global scoped variables, but not with script scoped variables.

This issue doesn't happen if I leave the global assignment in the root of the file (as opposed to within InitVars, but then I can't use SuppressMessageAttribute.

@HowardWolosky HowardWolosky changed the title PSUseDeclaredVarsMoreThanAssignment PSUseDeclaredVarsMoreThanAssignment not correctly handling global vars assigned to within functions Jan 31, 2017
HowardWolosky pushed a commit to microsoft/StoreBroker that referenced this issue Feb 8, 2017
There are some new warnings coming up when checking StoreBroker
with newer versions of [PSScriptAnalyzer](https://github.com/PowerShell/PSScriptAnalyzer).

* `PSUseDeclaredVarsMoreThanAssignments` - We had some instances where
  a variable was being assigned to but never used.  For the API instances,
  we now capture to $null in the instances where we don't want the helper
  method's results being returned to the user.

* `PSUseShouldProcessForStateChangingFunctions` - Two PackageTool methods
  use the verb "Remove" but have no need for providing ShouldProcess support.
  Those instances are now suppressed.

All remaining warnings are false positives of `PSUseDeclaredVarsMoreThanAssignments`.
I have opened two different issues against PSScriptAnalyzer to track these false
positives:
   * [PSUseDeclaredVarsMoreThanAssignment not correctly handling global vars assigned to within functions](PowerShell/PSScriptAnalyzer#698)
   * [PSUseDeclaredVarsMoreThanAssignment not correctly handling array adds](PowerShell/PSScriptAnalyzer#699)

Updated CONTRIBUTING.md to remind users to keep the analyzer module up-to-date.

Additionally, the links in PDP.md were invalid.  Those have now been fixed.
HowardWolosky added a commit to microsoft/StoreBroker that referenced this issue Feb 10, 2017
There are some new warnings coming up when checking StoreBroker
with newer versions of [PSScriptAnalyzer](https://github.com/PowerShell/PSScriptAnalyzer).

* `PSUseDeclaredVarsMoreThanAssignments` - We had some instances where
  a variable was being assigned to but never used.  For the API instances,
  we now capture to $null in the instances where we don't want the helper
  method's results being returned to the user.

* `PSUseShouldProcessForStateChangingFunctions` - Two PackageTool methods
  use the verb "Remove" but have no need for providing ShouldProcess support.
  Those instances are now suppressed.

All remaining warnings are false positives of `PSUseDeclaredVarsMoreThanAssignments`.
I have opened two different issues against PSScriptAnalyzer to track these false
positives:
   * [PSUseDeclaredVarsMoreThanAssignment not correctly handling global vars assigned to within functions](PowerShell/PSScriptAnalyzer#698)
   * [PSUseDeclaredVarsMoreThanAssignment not correctly handling array adds](PowerShell/PSScriptAnalyzer#699)

Updated CONTRIBUTING.md to remind users to keep the analyzer module up-to-date.

Additionally, the links in PDP.md were invalid.  Those have now been fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants