PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.86k stars 376 forks source link

incorrect "variable assinged but never used" when used in $env: expression #1028

Closed powercode closed 4 years ago

powercode commented 6 years ago

Before submitting a bug report:

Steps to reproduce

$n = "envVarName"
${env:$n} = "Value"

Expected behavior

No warning for unused variable 'n'

Actual behavior

The variable 'n' is assigned but never used. (PSUseDeclaredVarsMoreThanAssignments)

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      6.0.2
PSEdition                      Core
GitCommitId                    v6.0.2
OS                             Microsoft Windows 10.0.16299
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.17.1
1.16.1
rjmholt commented 6 years ago

I encountered this the other day too. I imagine that the lexically scoped variables will need separate code paths for this:

powercode commented 6 years ago

For the env:var case, the analyzer cannot do anything. When I ran into it, I was just setting up environment for a legacy project. The whole purpose of the assignment was the side effect.

JamesWTruher commented 6 years ago

There is a misunderstanding about variable declaration. When you create a variable as follows:

$n = "foo"
${env:$n}

you haven't created a variable env:foo but the environment variable $n (The variable name is not reinterpreted in the braces). When you create a variable and include the braces, everything inside the braces is used as the variable name. You can create variables with any set of characters with {

${ } = "foo" has created a variable whose name is (3 spaces) with the value of "foo"

if you want to create a dynamic variable in this way, you need to find another way

$n = "foo"
Set-Content "env:$n" 'value for foo'
powercode commented 6 years ago

@JamesWTruher Oh, good to know! Thx for the clarification!

JamesWTruher commented 6 years ago

@powercode - is it ok to close this issue?

rjmholt commented 6 years ago

My particular use-case involves a less ambiguous scenario. Basically a case where two functions act as an interface to a shared state in a higher scope:

$script:stack = [System.Collections.Stack[string]]::new()

function Push-Stack
{
    $script:stack.Push($env:PSModulePath)
    $env:PSModulePath = $env:PSModulePath + [System.IO.Path]::PathSeparator + $args[0]
}

function Pop-Stack
{
    $env:PSModulePath = $script:stack.Pop()
}

PSSA gives a warning in $env:PSModulePath = $script:stack.Pop() saying:

[PSScriptAnalyzer] The variable 'PSModulePath' is assigned but never used. (PSUseDeclaredVarsMoreThanAssignments)

The problem I think is that PSSA doesn't understand variable scopes as well as it should.

I imagine that PSSA should essentially treat env: and global: like volatile; anything can change them at any time, so static analysis is impossible.

module: is possible if PSSA can handle a module at a time (i.e. can hold that much state). Given the architecture of rules in PSSA, that might be a lot of work to get to.

But script: should be simple enough; do a full search in the whole script AST for usages...

rjmholt commented 6 years ago

FYI I just tried this with the latest version of PSSA and my repro is now fixed

SydneyhSmith commented 4 years ago

FYI I just tried this with the latest version of PSSA and my repro is now fixed

@rjmholt does this mean that the issue can be closed?

rjmholt commented 4 years ago

does this mean that the issue can be closed?

We should verify again with the current build, but I believe there's still work to be done here for differently scoped variables

bergmeister commented 4 years ago

Yes, I think we should close it. The scoping issue is a well known one, I even created a special issue tag for issues related to scoping problem/limitation. This specific issue was resolved by PR #958 by excluding drive qualified variables from analysis