GeezerVison

Ok, I've spent ages trying to debug this but it doesn't make any sense. The new parameter sets aren't present, but the command is loaded from the asset location, and this only happens in WinPS 5.1...

I finally worked this out. We have a bizarre test that overrides Invoke-ScriptAnalyzer and then calls other tests... Naturally now we have to fix all of that. I've never seen anyone actually call into the theoretically exposed API...

This can make the experience more confusing for users and makes maintenance more complicated as well. How about an Enum parameter instead that has a default?

So I would prefer an enum parameter, but that would be a breaking change; invocations that previously use -SuppressedOnly will no longer work.

In order to keep -SuppressedOnly while also including another parameter that affects suppression output, I think we're forced to split things into parameter sets in some way. The two mutually exclusive switches seemed like a simple approach. An alternative might be putting a more future-proof enum in the other parameter set.

Hmm, didn't realize -SuppressedOnly was an already existing parameter.
But I think we could still have the -IncludeSuppressions switch side by side with it but instead have some early validation logic to disallows the usage of both switches and throw an error (yes, this would be at runtime unfortunately). To me, multiple param sets for different switches not only make the code more complex but are also unintuitive as a user when I try to do tab completion and might not even be aware of other params that are excluded just because I chose to use one param and therefore don't know in most cases that this unintentional action is limiting what tab completion shows. Also, in case one person actually reads the docs and uses both switches, PowerShell's generic error is not helpful to users and although users could look at the cmdlet syntax block and try to understand what the param sets are, the reality is that most users don't understand it or just end up being frustrated by the time it takes them to figure out how to call the command.

Having said that, this topic is opinionated and I am sure the other side will argue for param sets with its early validation before actually running the command. Therefore I suggest we let a 3rd person like James Truher (@JamesWTruher) decide on this matter. I am happy with the rest of the PR.

Rob Holt (@rjmholt) Christoph Bergmeister (@bergmeister) i'm generally in favor of adding parameter sets where they make sense and don't over complicate things. In this case we're going from 2 to only 4 so i don't believe it's a higher cognitive load. The benefit with parameter sets is that we can determine behavior via inspection rather than runtime, so tools like vscode provide a better experience.

Christoph Bergmeister (@bergmeister) James Truher (@JamesWTruher) need an approval to merge 🙂