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

Add PSAvoidAssignmentToAutomaticVariable to the default set of PSSA rules #1101

Merged
merged 1 commit into from
Nov 13, 2019

Conversation

bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Nov 12, 2019

See docs below for info. The rule warns only on read-only, automatic variables where assignment would cause an error to be thrown at runtime, therefore there is no scope for false positives.
https://github.com/PowerShell/PSScriptAnalyzer/blob/master/RuleDocumentation/AvoidAssignmentToAutomaticVariable.md

Example of what would happen at runtime in the console if one ignored such a violation:

> $error = 42
WriteError: Cannot overwrite variable Error because it is read-only or constant.

We should back-port this change

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

Seems like a low-risk change and a useful rule. LGTM

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM

@rkeithhill
Copy link
Contributor

This is a good rule to have enabled by default. Thanks.

@bergmeister
Copy link
Contributor Author

bergmeister commented Nov 12, 2019

Kudos to @mklement0 for proposing it originally. I had the intent of doing it when I did the rule but forgot it in the long process of the PSSA release and the extension using a new release...

@mklement0
Copy link

Thanks, @bergmeister; also worth mentioning that enhancements to this rule are coming, so as to also cover automatic variables that are technically writeable but shouldn't be; in case someone wants to help review the proposed change: see PowerShell/PSScriptAnalyzer#712 (comment)

@TylerLeonhardt TylerLeonhardt merged commit 7ba287f into PowerShell:master Nov 13, 2019
@TylerLeonhardt
Copy link
Member

@bergmeister the bar is super high with backporting since the Preview will be stable in Jan. However, can you backport it?

bergmeister pushed a commit to bergmeister/PowerShellEditorServices that referenced this pull request Nov 13, 2019
TylerLeonhardt pushed a commit that referenced this pull request Nov 15, 2019
…fault set of PSSA rules (#1102)

* Fix PipelineIndentation configuration issue (#1050)

* legacy/1.x: Update minimum PSSA version to 1.18.3 (#1052)

* Backport of #1101: Add PSAvoidAssignmentToAutomaticVariable to the default set of PSSA rules
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.

4 participants