Skip to content
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

PSUseDeclaredVarsMoreThanAssignments - mention scoping #17

Closed
HeyDude378 opened this issue Nov 4, 2021 · 3 comments · Fixed by #74
Closed

PSUseDeclaredVarsMoreThanAssignments - mention scoping #17

HeyDude378 opened this issue Nov 4, 2021 · 3 comments · Fixed by #74
Assignees
Labels
area-scriptanalyzer Area - ScriptAnalyzer module

Comments

@HeyDude378
Copy link

It doesn't seem that variables have to be used more than their assignments; they can also be used the same number of times. You should clarify the language by saying something like "greater than or equal to their assignments".

You should also mention that the variable must be used within the same scope in which it was declared or else it won't be considered to be "used".

I have an example...

**$foo | for-eachobject {if($_ -eq $false){$bar = $true}}

if($bar){write-host "You are still going to get a PSUseDeclaredVarsMoreThanAssignments error even though you are using the bar variable here."}**


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@sdwheeler
Copy link
Collaborator

sdwheeler commented Nov 22, 2021

@bergmeister Can you comment on this issue?

I think the docs need a better description but I am wondering if the rule should be renamed to better describe what it does.

@sdwheeler sdwheeler changed the title mention scoping UseDeclaredVarsMoreThanAssignments - mention scoping Nov 22, 2021
@sdwheeler sdwheeler changed the title UseDeclaredVarsMoreThanAssignments - mention scoping PSUseDeclaredVarsMoreThanAssignments - mention scoping Nov 22, 2021
@bergmeister
Copy link
Contributor

@sdwheeler The rule's analysis is limited when it comes to variables definition and usage between different scopes. The author is correct that their example produces a false positive but there are also examples where the rule has some intelligence/knowledge between scopes such as in '$array=@(); $list | ForEach-Object { $array += 'foo' }' .
This has been a long-standing and well known limitation of the rule. The rule was improved a little bit but due to technical reasons and PowerShell's dynamic scoping, there is a limit to the intelligence of the script analysis when it comes to variable usage. People therefore need to be conscious that any analyzer will bring up false positives and that they can be suppressed.

Because the rule name is used in PSSA settings files, a rename would be quite disruptive and even if we created an alias name I am not sure if this would just make things confusing? The name of the rule should be read as 'You should use your declared variables and not just assign them a value' but I agree as well that the rule name doesn't read very fluently.

@HeyDude378
Copy link
Author

Hi, thank you for your reply. I think my first suggested change would be in the description line, "Generally variables that are not used more than their assignments are considered wasteful and not needed." I would rephrase it to "Generally variables that are not used are considered wasteful and not needed".

I think my second suggested change is to add a "How to Fix" section like you have for some of the other Problems documentation (like UseLiteralInitializerForHashtable) where you mention that "a variable's usage is sometimes ignored if it is not used in the same scope in which it was declared. Try using it in the same scope." Something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-scriptanalyzer Area - ScriptAnalyzer module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants