Skip to content

Invoke-ScriptAnalyzer should implement SupportsShouldProcess #1517

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

Open
rjmholt opened this issue Jun 4, 2020 · 9 comments
Open

Invoke-ScriptAnalyzer should implement SupportsShouldProcess #1517

rjmholt opened this issue Jun 4, 2020 · 9 comments

Comments

@rjmholt
Copy link
Contributor

rjmholt commented Jun 4, 2020

From #1476 (comment)

@SydneyhSmith
Copy link
Collaborator

@bergmeister we were discussing this and were thinking that since Invoke-ScriptAnalyzer does not change any state (or any script) that this might not be applicable--is there something we are missing? Thanks!

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 9, 2020

@SydneyhSmith PSSA has internal and external state (CommandInfo cache in memory and DSC module cache in user profile path on disk).
That aside, to help answer your question: the -Fix switch changes the analysed files on disk, which would be a classic use case of -Whatif. The -Recurse switch makes it analyse scripts in subfolders as well and is therefore another use cases where it currently provides information around analysed/affected files and (I think but not sure) also which rules would be run. This could be useful to know in the case of a large repo of files to be analysed without having to wait too long, which is not really about state but still useful.
If I had to redesign it for v2, I would log information of a dry run. Whether parsing is part of a dry run is questionable but I could see that being valueable as well

@bergmeister
Copy link
Collaborator

@SydneyhSmith After a conversation with @thomasrayner there also seems to be the case that people want to know which rules are run without having to parse the verbose output. If -WhatIf returned a rich object around the rules to be run and files to be analysed, this could be useful

@rjmholt
Copy link
Contributor Author

rjmholt commented Jun 11, 2020

Seems like Get-ScriptAnalyzerRule could also be applicable there.

Personally I'm not totally convinced that Invoke-ScriptAnalyzer should implement ShouldProcess, especially since the internal state feels to me more like an unwanted implementation detail than anything else. A linter that needs a WhatIf sounds to me like it does too much. Other commands that format files etc make sense though.

But I think this is valuable discussion, and I don't see too much harm in adding the feature. Could be useful for debugging at least.

@bergmeister
Copy link
Collaborator

Yes, Get-ScriptAnalyzerRule should return in its RuleInfo more details such as e.g. if a rule is run by default by Invoke-ScriptAnalyzer or Invoke-Formatter. It should also have a parameters like -Settings and the Include/ExcludeRule params to 'simulate' a run

@thomasrayner
Copy link
Contributor

Elaborating on what @bergmeister said, when I run PSSA in a CI environment like Azure Pipelines, I want to be able to publish the results to the Test Results section (ideally formatted as nUnit or something else compatible, but that's not the point of this issue). For the test results to be most meaningful, I'd want consistency over which rules showed up there each time so I can track when each file violated each rule and when.

Maybe what I really want is for Get-ScriptAnalyzerRule to take a -Settings file and produce calculated output based on what rules would run given a particular psd1 settings file. I already have to wrap PSSA with Pester to get nUnit formatted results, so if I didn't have to parse my own settings and compare with the default rules, it wouldn't be too far a stretch.

@bergmeister
Copy link
Collaborator

Issue #1296 and #672 track that. I feel we should close the latter in favour of the former though.
There is already a script that converts a DiagnosticRecord to NUnit XML (not ideal as one would also want positive results but it's something).
https://github.com/MathieuBuisson/PowerShell-DevOps/blob/master/Export-NUnitXml/Export-NUnitXml.psm1

@thomasrayner
Copy link
Contributor

Is there a larger discussion anywhere (or has there been one in the past) about how PSSA fits into CI/CD systems?

@bergmeister
Copy link
Collaborator

Not aware, even Steve complained to me about this one but there is the PSCodeHealth module and its Azure DevOps extension here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants