X Tutup
The Wayback Machine - https://web.archive.org/web/20200918094452/https://github.com/PowerShell/PSScriptAnalyzer/pull/923
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

Do not trigger UseShouldProcessForStateChangingFunctions rule for workflows #923

Conversation

@bergmeister
Copy link
Collaborator

bergmeister commented Mar 5, 2018

PR Summary

Fixes #922

Workflows do not allow SupportsShouldProcess (the parser does not even allow it), therefore do not trigger UseShouldProcessForStateChangingFunctions
As part of this I also update the copyright header and tidied up unused using statements.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets. Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • User facing documentation needed
  • Change is not breaking
  • Make sure you've added a new test if existing tests do not effectively test the code changed
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready
bergmeister added 2 commits Mar 5, 2018
@bergmeister bergmeister self-assigned this Mar 5, 2018
@bergmeister bergmeister changed the title Workflows do not allow SupportsShouldProcess --> do not trigger UseShouldProcessForStateChangingFunctions rule Do not trigger UseShouldProcessForStateChangingFunctions rule for workflows Mar 5, 2018
@bergmeister bergmeister requested a review from JamesWTruher Mar 5, 2018
@@ -42,5 +42,11 @@ Function New-{0} () {{ }}
It "returns no violations" {
$noViolations.Count | Should -Be 0
}

It "Workflows should not trigger a warning because they do not allow SupportsShouldProcess" {

This comment has been minimized.

@JamesWTruher

JamesWTruher Mar 5, 2018 Member

I think we need to protect this from execution on core, where workflow is not supported. Add '-skip:$IsCore' I think.

This comment has been minimized.

@JamesWTruher

JamesWTruher Mar 5, 2018 Member

PS> Invoke-ScriptAnalyzer -ScriptDefinition "workflow foo { 'zap' }"                                                                        
Invoke-ScriptAnalyzer : Parse error in script definition:  Workflow is not supported in PowerShell Core at line 1 column 1.
At line:1 char:1
+ Invoke-ScriptAnalyzer -ScriptDefinition "workflow foo { 'zap' }"
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : ParserError: (WorkflowNotSupportedInPowerShellCore:String) [Invoke-ScriptAnalyzer], ParseException
+ FullyQualifiedErrorId : Parse error in script definition:  Workflow is not supported in PowerShell Core at line 1 column 1.,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand

This comment has been minimized.

@bergmeister

bergmeister Mar 5, 2018 Author Collaborator

Good catch, thanks, I should've run the test suite in PSCore as well.

Copy link
Member

JamesWTruher left a comment

looks good

@JamesWTruher JamesWTruher merged commit f002c1c into PowerShell:development Mar 6, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.
X Tutup