Skip to content

Custom Rule Consistency #1237

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

Closed
Jaykul opened this issue May 8, 2019 · 1 comment · Fixed by #1245
Closed

Custom Rule Consistency #1237

Jaykul opened this issue May 8, 2019 · 1 comment · Fixed by #1245

Comments

@Jaykul
Copy link
Contributor

Jaykul commented May 8, 2019

It's frustrating that there's no automatic relationship between the rules in the output of Get-ScriptAnalzyerRule -CustomRulePath and Invoke-ScriptAnalyzer ...

Specifically, the -ExcludeRule of Invoke-ScriptAnalyzer has to match the custom rule function name (without the module on the front).

But the name you have to put in SuppressMessageAttribute has to match the RuleName which is set on the output of the function, so if you use RuleName = $PSCmdlet.MyInvocation.InvocationName then you must use the module-qualified rule name, and if you use RuleName = $PSCmdlet.MyInvocation.MyCommand.Name then you must not ...

But the coup-de-grace, if you Invoke-ScriptAnalyzer with -IncludeDefaultRules then you can't use names in SuppressMessageAttribute which are not module-qualified except for the default rules.

There is just no way to make it work

There's no combination where I can consistently use the same name for -ExcludeRule and SuppressMessageAttribute

This is a problem. It makes sense for these to be module-qualified, and calculated by PSScriptAnalyzer, not the rule author. I think ScriptAnalyzer should require the rule function names to use the Measure verb, and then use the noun as the rule name -- and ignore/overwrite the output value.

That way you can guarantee that the rule name will always be what you expect it to be. Then, the -ExcludeRule parameter and Get-ScriptAnalyzerRule need to be updated to return the same value too.

Severity is wrong too

Currently all Custom rules are shown as "Warning" severity in the output of Get-ScriptAnalyzerRule but (can) have different severity when the rule is actually triggered (actually, they can assign a severity based on logic in the rule).

We should add an attribute to allow authors to specify the (default?) severity of the rule that would be reported by Get-ScriptAnalyzerRule (and we should use that severity by default, allowing rule authors to skip setting the RuleName, Severity, and RuleSuppressionId and have them correctly populated in the output).

@bergmeister
Copy link
Collaborator

Thanks for raising it. I was quite busy in the last days but hope to have a look at this next week. For me the behaviour of Invoke-ScriptAnalyzer has priority, I would first like to write down a few scenarios and compare 1.17.1 to 1.18.0 where I made a change with good will that improved it but in some cases seems to have also been breaking (mainly because of the requirement for module qualified name to be able to differentiate duplicate rule names from different custom rules). I first want to address the breaking change as I hope other fixes after 1.18.0 can hopefully go into a 1.18.1 release. After that it is probably time to properly fix it and then make Get-ScriptAnalyzer behave exactly as Invoke-ScriptAnalyzer. Out of interest: Is there a scripting use case where you rely on the behaviour of Get-ScriptAnalyzer or is this 'only' a problem of unintuitive UX?

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