Use AST to determine SupportsShouldProcess when an error is thrown by rjmholt · Pull Request #1397 · PowerShell/PSScriptAnalyzer
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
PR Summary
Fixes #1217.
When PowerShell throws an error due to not being able to resolve an attribute type, we sift through the AST to determine whether SupportsShouldProcess has been set on a function.
Also cleans up some of the logic so it's not all nested ass.
PR Checklist
- PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
- Summarized changes
- Change is not breaking
- Make sure all
.cs,.ps1and.psm1files have the correct copyright header - Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
- 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.
- If the PR is work in progress, please add the prefix
Code change looks ok but the build says it has a problem when targeting psv3, looks like we need a pragma using the PSV3 variable
UseShouldProcessCorrectly.cs(382,99): error CS1061: 'ExpressionAst' does not contain a definition for 'SafeGetValue' and no accessible extension method 'SafeGetValue' accepting a first argument of type 'ExpressionAst' could be found (are you missing a using directive or an assembly reference?)
Co-Authored-By: Christoph Bergmeister [MVP] <c.bergmeister@gmail.com>
Copy link
Contributor Author
Ok I've repurposed the SafeGetValue substitute method I wrote to handle that. It's now available as a Helper method.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I verified that the diff in Helper.cs/Settings.cs is mainly just the move to make the method publicly accessible.
Only an optional style suggestion.
Co-Authored-By: Christoph Bergmeister [MVP] <c.bergmeister@gmail.com>
Co-Authored-By: Christoph Bergmeister [MVP] <c.bergmeister@gmail.com>
2 participants