add validation for powershell scripts in CI#64987
add validation for powershell scripts in CI#64987jmarolf wants to merge 2 commits intodotnet:mainfrom
Conversation
3ef4ee0 to
0558cb0
Compare
0558cb0 to
3590b5e
Compare
|
tagging @jaredpar as I really need him to tell me if he thinks this has any value. We have the ability to write our own custom analyzers for this and if Jared knows of any mistakes we want to check for we could write and use those. One of the downsides of unused variable analysis is that they only look in the scope the variable is declared in not the child scopes and we do a lot of magic "declare this variable before calling this function because I know that it expects them to exist". |
| [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'bootstrapConfiguration')] | ||
| [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'buildServerLog')] | ||
| [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'collectDumps')] | ||
| [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'configuration')] |
There was a problem hiding this comment.
I'm confused about this rule. Many of these parameters are clearly used so why are we getting an unused diagnostc here?
There was a problem hiding this comment.
the "scope" they are declared in is just the try-catch which they are not used in. They are used in child scopes (i.e. other functions) implicitly. to remove this we would need to explicitly pass these to all the functions that consume them instead of assuming that they are defined in the parent scope.
There was a problem hiding this comment.
for example:
$configuration is only read from implicitly in BuildSolution() and TestUsingRunTests(). We could instead explicitly pass it as an argument to both those functions if we wanted to get rid of the warning.
No description provided.