Skip to content

Do not trigger UseShouldProcessForStateChangingFunctions rule for workflows #923

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions Rules/UseShouldProcessForStateChangingFunctions.cs
Original file line number Diff line number Diff line change
@@ -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
{
{
/// <summary>
/// UseShouldProcessForStateChangingFunctions: Analyzes the ast to check if ShouldProcess is included in Advanced functions if the Verb of the function could change system state.
/// </summary>
Expand Down Expand Up @@ -61,7 +52,8 @@ public IEnumerable<DiagnosticRecord> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" -Skip:$IsCoreCLR {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'workflow Set-Something {[CmdletBinding()]Param($Param1)}' | Where-Object {
$_.RuleName -eq 'PSUseShouldProcessForStateChangingFunctions' }
$violations.Count | Should -Be 0
}
}
}