Enhance PSShouldProcess rule to check for ShouldProcess in nested functions #625
Conversation
The new implementation adds ability to check if a nested function calls shouldprocess/shouldcontinue when an upstream function declares SupportsShouldProcess attribute.
If a cmdlet declares SupportsShouldProcess attribute, it must call ShouldProcess but calling ShouldContinue is optional.
| @@ -3555,4 +3555,239 @@ public object VisitUsingExpression(UsingExpressionAst usingExpressionAst) | |||
| return null; | |||
| } | |||
| } | |||
|
|
|||
| /// Class to represent a directed graph | |||
| public class Digraph<T> | |||
There was a problem hiding this comment.
Consider making this an internal class for now. We can open it up as public as the scenarios evolve.
There was a problem hiding this comment.
We use Pester for our testing purposes. If I make it internal, how do I go about testing? I do not think I will be able find the type in Pester tests if it is internal.
| } | ||
|
|
||
| /// <summary> | ||
| /// Construct digraph with an EqualityComparer used for comparing type T |
There was a problem hiding this comment.
Is it possible for you to give more details in the summary
| /// Check if the given vertex is part of the graph. | ||
| /// | ||
| /// If the vertex is null, it will throw an ArgumentNullException. | ||
| /// If the vertex is non-null but not present in the graph, it will throw an ArgumentOutOfRangeException |
There was a problem hiding this comment.
Why not just return null in this case instead of throwing an exception
There was a problem hiding this comment.
Line 3608 is not applicable for this method (ContainsVertex). I will remove it. The method ContainsVertex expects a non-null object for comparison and returns a bool type.
| /// <returns>True if the vertices are conneted, otherwise false</returns> | ||
| private bool IsConnected(int fromIdx, int toIdx, ref bool[] visited) | ||
| { | ||
| visited[fromIdx] = true; |
There was a problem hiding this comment.
I understand you are doing the marking here..but i dont see where the cleanup happens.
There was a problem hiding this comment.
visited is cleaned up when public bool IsConnected(T vertex1, T vertex2) returns.
| Context "Where ShouldProcess is called in nested function" { | ||
| It "finds no violation" { | ||
| $scriptDef = @' | ||
| function Outer |
There was a problem hiding this comment.
have a testcase related to recursion
Resolves #521
Resolves #608
Resolves #194
This change is