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

Warn against assignment to read-only automatic variables #864

Conversation

bergmeister
Copy link
Collaborator

@bergmeister bergmeister commented Feb 3, 2018

This is the first of a series of PRs to address issues #858 and #712
Because the analysis of automatic variables is quite complex and in certain cases assignment to automatic variables can be by design, the list of 'naughty' variable assignments is going to be opinionated.
Therefore I decided to start with the read-only variables first where PowerShell will throw an error if assignment or usage as function parameter is attempted.
Note that I still have to look into the other tests that started failing.

@bergmeister bergmeister changed the title Warn against assignment to read-only automatic variables [WIP] Warn against assignment to read-only automatic variables Feb 3, 2018
@bergmeister bergmeister changed the title [WIP] Warn against assignment to read-only automatic variables Warn against assignment to read-only automatic variables Feb 3, 2018

## Description

`PowerShell` exposes some of its build in variables that are known as automatic variables. Many of them are read-only and PowerShell would throw an error when trying to assign an value on those. The remaining automatic variables should only be assigned to in certain special cases to achieve a certain effect as a special technique.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

built-in rather than "build in"
"the remaining" would probably be better as other

It "Variable '<VariableName>' produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)

$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$$($VariableName) = 'foo'" | Where-Object { $_.RuleName -eq $ruleName }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the $(...) isn't needed

"`$${VariableName} = 'foo'"

will work just fine

It "Using Variable '<VariableName>' as parameter name in param block produces warning of severity error" -TestCases $testCases_ReadOnlyVariables {
param ($VariableName)

[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" | Where-Object {$_.RuleName -eq $ruleName }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks to be the same as line 36

@LaurentDardenne
Copy link

One remark about the texte of the erreur message :
"Variable true should use a different name since it is a readonly automatic variable."
We can consider that we must change the name of the variable $true.
may be :
"The variable 'true' should not be assigned since it is a PowerShell automatic variable, use a different name."
what, for a beginner, specifies the intention of the rule.

…nalyzer into WarnAgainstAssignmentToAutomaticVariables

# Conflicts:
#	Rules/Strings.resx
@bergmeister
Copy link
Collaborator Author

Thanks for the feedback, I addressed your comments.

@JamesWTruher JamesWTruher merged commit d432e00 into PowerShell:development Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants