Use PSScriptAnalyzer on this repo

[jmarolf]

We should run PSScriptAnalyzer on this repo as a way to get some basic ci for the powershell changes that we are making.

CC: @jaredpar @tmat

Running this command

Invoke-ScriptAnalyzer -Recurse -ExcludeRule PSAvoidUsingWriteHost,PSUseApprovedVerbs,PSAvoidTrailingWhitespace,PSUseSingularNouns,PSProvideCommentHelp,PSAvoidGlobalVars

over the arcade repo produced these results:

Severity	RuleName	ScriptName	Line	Message
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	13	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	16	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	29	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	62	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	66	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	71	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseDeclaredVarsMoreThanAssignments	init-tools-native.ps1	104	The variable 'toolInstallationFailure' is assigned but never used.
Warning	PSAvoidUsingInvokeExpression	init-tools-native.ps1	99	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	13	The variable 'ci' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	14	The variable 'binaryLog' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	15	The variable 'warnAsError' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	508	The variable 'EngRoot' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	399	The variable 'nugetCache' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	416	The variable 'buildTool' is assigned but never used.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	106	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	111	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	113	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	122	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	195	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	197	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	209	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	225	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	376	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	471	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	482	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	492	$null should be on the left side of equality comparisons.
Warning	PSUseShouldProcessForStateChangingFunctions	tools.ps1	439	"Function 'Stop-Processes' has verb that could change system state. Therefore
Information	PSAvoidUsingPositionalParameters	tools.ps1	422	Cmdlet 'MSBuild' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
Warning	PSUseDeclaredVarsMoreThanAssignments	CommonLibrary.psm1	199	The variable 'InstallStatus' is assigned but never used.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	52	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	63	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	65	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	111	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	131	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	148	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	191	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	213	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	218	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	321	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	328	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	339	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	346	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	348	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Warning	PSAvoidUsingInvokeExpression	CommonLibrary.psm1	212	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseShouldProcessForStateChangingFunctions	CommonLibrary.psm1	173	"Function 'New-ScriptShim' has verb that could change system state. Therefore
Warning	PSAvoidUsingCmdletAliases	install-tool.ps1	101	'%' is an alias of 'ForEach-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	13	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	16	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	darc-init.ps1	29	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	62	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	66	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSAvoidUsingInvokeExpression	generate-graph-files.ps1	71	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseDeclaredVarsMoreThanAssignments	init-tools-native.ps1	104	The variable 'toolInstallationFailure' is assigned but never used.
Warning	PSAvoidUsingInvokeExpression	init-tools-native.ps1	99	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	13	The variable 'ci' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	14	The variable 'binaryLog' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	sdk-task.ps1	15	The variable 'warnAsError' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	508	The variable 'EngRoot' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	399	The variable 'nugetCache' is assigned but never used.
Warning	PSUseDeclaredVarsMoreThanAssignments	tools.ps1	416	The variable 'buildTool' is assigned but never used.
Warning	PSUseShouldProcessForStateChangingFunctions	tools.ps1	439	"Function 'Stop-Processes' has verb that could change system state. Therefore
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	106	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	111	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	113	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	122	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	195	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	197	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	209	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	225	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	376	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	471	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	482	$null should be on the left side of equality comparisons.
Warning	PSPossibleIncorrectComparisonWithNull	tools.ps1	492	$null should be on the left side of equality comparisons.
Information	PSAvoidUsingPositionalParameters	tools.ps1	422	Cmdlet 'MSBuild' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
Warning	PSUseDeclaredVarsMoreThanAssignments	CommonLibrary.psm1	199	The variable 'InstallStatus' is assigned but never used.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	52	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	63	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	65	The cmdlet 'DownloadAndExtract' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	111	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	131	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	148	The cmdlet 'Get-File' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	191	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	213	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	218	The cmdlet 'New-ScriptShim' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	321	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	328	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	339	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	346	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Information	PSUseOutputTypeCorrectly	CommonLibrary.psm1	348	The cmdlet 'Expand-Zip' returns an object of type 'System.Boolean' but this type is not declared in the OutputType attribute.
Warning	PSAvoidUsingInvokeExpression	CommonLibrary.psm1	212	Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Warning	PSUseShouldProcessForStateChangingFunctions	CommonLibrary.psm1	173	"Function 'New-ScriptShim' has verb that could change system state. Therefore
Warning	PSAvoidUsingCmdletAliases	install-tool.ps1	101	'%' is an alias of 'ForEach-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSPossibleIncorrectComparisonWithNull	GitHubMergeBranches.ps1	122	$null should be on the left side of equality comparisons.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	203	'%' is an alias of 'ForEach-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	204	'%' is an alias of 'ForEach-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	205	'?' is an alias of 'Where-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	206	'select' is an alias of 'Select-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	208	'measure' is an alias of 'Measure-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	208	'select' is an alias of 'Select-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	213	'%' is an alias of 'ForEach-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	283	'?' is an alias of 'Where-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	GitHubMergeBranches.ps1	284	'select' is an alias of 'Select-Object'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingEmptyCatchBlock	GitHubMergeBranches.ps1	233	Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	22	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	25	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	40	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	43	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	59	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.
Warning	PSAvoidUsingCmdletAliases	configure-pool.ps1	62	'echo' is an alias of 'Write-Output'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content.

[markwilkie]

@jmarolf - this seems like general goodness. Could you explain a bit more who would look at the results? Is this the kind of thing where Arcade folks should run the tool as a way to keep things clean?

[jmarolf]

@markwilkie the idea would be that we install this analyzer in the dotnet/arcade repo and work to clean up all warnings. At some point I think we should fail the build if a PR introduces new issues.

[markwilkie]

Makes sense. Adding to our post //build backlog.

[jcagme]

@markwilkie what's the reason behind moving this to a release related epic when the work is to fix PS scripts that reported warnings?

[markwilkie]

I think the focus here is on running the (cleaned up) scripts. Instead of adding these to the build, it seems better to add them along side the other "checkers" we're running already in the release pipeline.

[JohnTortugo]

At some point I think we should fail the build if a PR introduces new issues.

@markwilkie I think this would make more sense as a PR check thing. Looks like @jmarolf has the same point of view.

/cc @chcosta

[markwilkie]

Perhaps - but keeping PR's fast is important.

[jcagme]

Yeah, I would rather fail fast (PR) rather than realize there is a script warning blocking my packages to be published

[JohnTortugo]

@markwilkie the idea would be that we install this analyzer in the dotnet/arcade repo and work to clean up all warnings. At some point I think we should fail the build if a PR introduces new issues.

IMO the best place to run this would be in PR builds.. nevertheless, I like the idea proposed by @jmarolf

I'm thinking to move forward with this by adding a new validation job to the post-build Arcade templates that don't fail the build but just show a warning in case of validation errors. Once people become more aware of the issues we can work to move these checks to PR time. Makes sense?

[markwilkie]

@jcagme - adding this to evaluate for the validation ring.

[jcagme]

As I understand, the suggestion is to use the script analyzer on PS scripts on arcade. If my reading is correct, we cannot do this in the validation ring since by then we are only working with shippable assets (PS are not included) produced by builds on the longest path, arcade doesn't fall in this category. In order to activate this in arcade, we'd need to add a build step, most likely on PR builds so we know when scripts are not correct and/or run this every so often, but again, not as part of the release.

[jcagme]

Now with the addition of source code validation this is something we could check but Arcade is not part of the candidate build. I feel this is something we could add to PRs or CI better.

[riarenas]

Triage: Should Arcade be included in the validation?

[missymessa]

PSScriptAnalyzer is now being injected via 1ES PT. Closing.