From aa5169362952600870b52e39640cc187059165e1 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 5 Mar 2018 19:18:53 +0000 Subject: [PATCH 1/3] Workflows do not allow SupportsShouldProcess -> do not trigger UseShouldProcessForStateChangingFunctions --- ...seShouldProcessForStateChangingFunctions.cs | 18 +++++------------- ...dProcessForStateChangingFunctions.tests.ps1 | 6 ++++++ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Rules/UseShouldProcessForStateChangingFunctions.cs b/Rules/UseShouldProcessForStateChangingFunctions.cs index 15e578f87..4448e6693 100644 --- a/Rules/UseShouldProcessForStateChangingFunctions.cs +++ b/Rules/UseShouldProcessForStateChangingFunctions.cs @@ -1,26 +1,17 @@ -// Copyright (c) Microsoft Corporation. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. using System; using System.Collections.Generic; #if !CORECLR using System.ComponentModel.Composition; #endif -using System.Management.Automation; using System.Management.Automation.Language; using System.Globalization; -using System.Linq; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules -{ +{ /// /// UseShouldProcessForStateChangingFunctions: Analyzes the ast to check if ShouldProcess is included in Advanced functions if the Verb of the function could change system state. /// @@ -61,7 +52,8 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) private bool IsStateChangingFunctionWithNoShouldProcessAttribute(Ast ast) { var funcDefAst = ast as FunctionDefinitionAst; - if (funcDefAst == null) + // SupportsShouldProcess is not supported in workflows + if (funcDefAst == null || funcDefAst.IsWorkflow) { return false; } diff --git a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 index 15a5a3af6..b6e99c25b 100644 --- a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 +++ b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 @@ -42,5 +42,11 @@ Function New-{0} () {{ }} It "returns no violations" { $noViolations.Count | Should -Be 0 } + + It "Workflows do no trigger a warning because they do not allow SupportsShouldProcess" { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'workflow Set-Something {[CmdletBinding()]Param($Param1)}' | Where-Object { + $_.RuleName -eq 'PSUseShouldProcessForStateChangingFunctions' } + $violations.Count | Should -Be 0 + } } } From 9d9d1b9ef6dfdff3b42086660dc7e10f0c8a7f2c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 5 Mar 2018 19:22:17 +0000 Subject: [PATCH 2/3] correct typo --- Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 index b6e99c25b..85bc55dfc 100644 --- a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 +++ b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 @@ -43,7 +43,7 @@ Function New-{0} () {{ }} $noViolations.Count | Should -Be 0 } - It "Workflows do no trigger a warning because they do not allow SupportsShouldProcess" { + It "Workflows should not trigger a warning because they do not allow SupportsShouldProcess" { $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'workflow Set-Something {[CmdletBinding()]Param($Param1)}' | Where-Object { $_.RuleName -eq 'PSUseShouldProcessForStateChangingFunctions' } $violations.Count | Should -Be 0 From 46e2f59ec6f4306569976e66c2b15fa8dab4d6ea Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 5 Mar 2018 20:57:51 +0000 Subject: [PATCH 3/3] Workflows are not supported in PowerShell 6.0 -> do not run test in this case --- Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 index 85bc55dfc..70cb68193 100644 --- a/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 +++ b/Tests/Rules/UseShouldProcessForStateChangingFunctions.tests.ps1 @@ -43,7 +43,7 @@ Function New-{0} () {{ }} $noViolations.Count | Should -Be 0 } - It "Workflows should not trigger a warning because they do not allow SupportsShouldProcess" { + It "Workflows should not trigger a warning because they do not allow SupportsShouldProcess" -Skip:$IsCoreCLR { $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'workflow Set-Something {[CmdletBinding()]Param($Param1)}' | Where-Object { $_.RuleName -eq 'PSUseShouldProcessForStateChangingFunctions' } $violations.Count | Should -Be 0