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

Enable suppression of PSAvoidAssignmentToAutomaticVariable for specific variable or parameter #1896

Merged
merged 2 commits into from Sep 17, 2023

Conversation

fflaten
Copy link
Contributor

@fflaten fflaten commented Feb 27, 2023

PR Summary

Enables suppression for specific variable/parameter with rule PSAvoidAssignmentToAutomaticVariable.
Fix #1589

PR Checklist

Enables suppression for specific variable/parameter.
Fix PowerShell#1589
Copy link
Collaborator

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for improving it, change looks good to me, can you please add one test case for it though?
If possible a test case for each line that was changed, that can return a diagnostic record, but I can understand if that is too much for you, therefore consider this optional.

@fflaten
Copy link
Contributor Author

fflaten commented Feb 28, 2023

Intentionally skipped it because suppression was only tested in a general test and not per rule.

Can try to add some later today if you want.

)

$testCases_ReadOnlyAutomaticVariables = $testCases_AutomaticVariables | Where-Object { $_.IsReadonly }

It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
param ($VariableName, $ExpectedSeverity)
Copy link
Contributor Author

@fflaten fflaten Feb 28, 2023

Choose a reason for hiding this comment

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

Tip: Param-blocks are no longer needed in Pester v5. Some keep them to make the testcase-contract visible so didn't remove them :)

@@ -7,60 +7,61 @@ BeforeAll {

Describe "AvoidAssignmentToAutomaticVariables" {
Context "ReadOnly Variables" {
BeforeDiscovery {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added per best practice in Pester v5. Only cosmetic and more explicit in this case.

@fflaten
Copy link
Contributor Author

fflaten commented Mar 16, 2023

In case it didn't show up - ready for review again 🙂

Copy link
Member

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

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

This is great - thank you.
@bergmeister

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot Suppress Messages About a Specific Automatic Variable
3 participants