Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPsShouldProcess - ShouldContinue not included #1305
Conversation
|
Thanks.
https://docs.microsoft.com/en-us/dotnet/api/system.management.automation.cmdlet.shouldcontinue If we could also check for the usage of the -Force pattern (not sure how difficult or possible it is), then that would be great. Again, this could be an optional, separate, follow up PR in addition to this awesome PR, would you be interested in doing that as well (no pressure)? |
|
Thanks @bergmeister. I'd be happy to give it a go sometime; though my knowledge of parsers isn't great / this fix was easy to code since I could mostly steal from the existing efforts made for FYI: There is already a rule to ensure that the However, that rule can currently be fooled by adding the switch to the function's parameters, then neglecting to use it. I've created new issue: #1308 to track this. |
|
@rjmholt @JamesWTruher I would be happy to merge this change in, are you ok with it as well? |
|
Sorry, didn't see this PR when it was originally opened. A couple of small suggestions, but not blocking. |
| return functionDefinitionAst.Extent; | ||
| } | ||
|
|
||
| return (invokeMemberExpressionAstFound as InvokeMemberExpressionAst).Member.Extent; |
This comment has been minimized.
This comment has been minimized.
rjmholt
Jan 28, 2020
Member
| return (invokeMemberExpressionAstFound as InvokeMemberExpressionAst).Member.Extent; | |
| return ((InvokeMemberExpressionAst)invokeMemberExpressionAstFound).Member.Extent; | |
| /// </summary> | ||
| private static bool IsShouldContinueCall(Ast ast) | ||
| { | ||
| var invokeMemberExpressionAst = ast as InvokeMemberExpressionAst; |
This comment has been minimized.
This comment has been minimized.
rjmholt
Jan 28, 2020
Member
Might be nicer to do this:
return ast is InvokeMemberExpressionAst invokeMemberExpressionAst
&& invokeMemberExpressionAst.Member is StringConstantExpressionAst memberExprAst
&& string.Equals(memberExprAst.Value, "ShouldContinue", StringComparison.OrdinalIgnoreCase);| private bool callsShouldProcessDirectly(Vertex vertex) | ||
| { | ||
| return funcDigraph.GetNeighbors(vertex).Contains(shouldProcessVertex); | ||
| } | ||
| private bool callsShouldContinueDirectly(Vertex vertex) |
This comment has been minimized.
This comment has been minimized.
rjmholt
Jan 28, 2020
Member
| private bool callsShouldContinueDirectly(Vertex vertex) | |
| private bool callsShouldContinueDirectly(Vertex vertex) | |
|
@rjmholt Should we merge this PR as-is since @JohnLBevan hasn't provided any updates and did not tick the option for other people to allow push to his branch? We could then do a follow-up PR. |
|
Apologies, I missed your comments from Jan. I'm happy to make the suggested changes, though think I deleted the original branch a while back (i.e. the source of the pull request now shows as |
JohnLBevan commentedAug 9, 2019
fixes #1304