Skip to content
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

Should rule severity be configurable ? #1515

Open
jegannathanmaniganadan opened this issue May 29, 2020 · 6 comments
Open

Should rule severity be configurable ? #1515

jegannathanmaniganadan opened this issue May 29, 2020 · 6 comments

Comments

@jegannathanmaniganadan
Copy link
Contributor

jegannathanmaniganadan commented May 29, 2020

I'd like to start the discussion about having an option of adjusting rule severity. I am not sure how PSSA community decides rule severity while onboarding any new rules. But I personally think it would be nice if I am given an option to upgrade some of the rules to Error or vice-versa.

For example, I would really like to upgrade PSAvoidGlobalVars, PSAvoidUsingWriteHost, PSAvoidUsingPlainTextForPassword rules severity to Error within my organization. This would not be the case for everyone. Similarly, I would like to change some of them to Information.

Let me know if this was something discussed and ignored for any reason.

@ghost ghost added the Needs: Triage 🔍 label May 29, 2020
@bergmeister
Copy link
Collaborator

bergmeister commented May 29, 2020

How do you use Severity and what is the impact/reason of wanting to having different values to you (i.e. what difference does it make to you)?
I think it's just meant to be an easy way of grouping/filtering rules and severity is usually determined by the impact that a violation might have. The only difference to the user is that VS-Code marks the squiggly as red instead of yellow when it is of type Error as far as I know.
With the returned results from Invoke-ScriptAnalyzer you can just filter out the diagnostic records of rule that you are interested in and then do a custom action like e.g. failing CI or only adding a warning message.

@SydneyhSmith
Copy link
Collaborator

Thanks @jegannathanmaniganadan for opening this issue, this is something we want to consider as we re-vamp the configuration options for PSSA 2.0 (our next major version). As @bergmeister mentioned it would still be great to know how you use the severity warnings in your work flow?

@jegannathanmaniganadan
Copy link
Contributor Author

@SydneyhSmith I am glad to hear that. My reasons are almost covered by @bergmeister .

The only difference to the user is that VS-Code marks the squiggly as red instead of yellow when it is of type Error as far as I know.

This is not much I am concerned about. Given that the PSSA (with custom rules too) & VSCode performance is not great, I usually turn off PSSA in VScode.

you can just filter out the diagnostic records of rule that you are interested in and then do a custom action like e.g. failing CI or only adding a warning message.

CI is where I am trying to make difference. Grouping based on Severity will be useful and it will also add more meaning to the rule Severity. It is easy to say catch all Violations with Error than array of values using Include/Exclude rule feature

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 8, 2020

@jegannathanmaniganadan

CI is where I am trying to make difference. Grouping based on Severity will be useful and it will also add more meaning to the rule Severity. It is easy to say catch all Violations with Error than array of values using Include/Exclude rule feature

PSSA already has the -Severity parameter so that it only runs rules of one or more certain severity levels. Changing the severity of rules to create your custom list of rules you are interested in does not sound right to me as you'd rather want to use the -IncludeRule and/or -ExcludeRule directly, where I do not see advantages/disadvantages. If you want to avoid maintaining a big list of rules to include, then I suggest you to use Get-ScriptAnalyzerRule, which also has a -Severity parameter, this way you could easily create your custom rule set for -IncludeRule:

$includeRule = ((Get-ScriptAnalyzerRule -Severity Error) + (Get-ScriptAnalyzerRule -Name PSAvoidUsingCmdletAliases)).RuleName

Also: you can pipe the results of Invoke-ScriptAnalyzer to Where-Object to include exclude certain violations and PowerShell was explicitly designed for this so that every cmdlet doesn't have to re-invent the wheel around filtering of results.

@VWACRansom
Copy link

We have a tool that runs PSScriptAnalyzer as part of our Pull Request process. It will not let anything be checked in that has any Errors or Warnings.
Recently, we started seeing a lot of PSUseSingularNouns. In some cases this is intentional. For example: Send-MessageToTeams (As in Microsoft Teams)

Ideally, we would like to disable the rule for just these cases, but #849 isn't available yet.
In the meantime, we would like to change the rule from Warning to Info so that our devs know about it, but aren't blocked when using it appropriately.

@o-l-a-v
Copy link

o-l-a-v commented Feb 21, 2024

I'd like to be able to change severity from Error to Warning for PSAvoidUsingConvertToSecureStringWithPlainText, because Microsoft tooling sometimes forces me to ConvertTo-SecureString -AsPlainText:

It'd be better to have a warning for this than disabling the rule all together.


I'd rather do this:

@{
  Rules = @{
    'PSAvoidUsingConvertToSecureStringWithPlainText' = @{
      'Severity' = 'Warning'
    }
  }
}

Than this:

@{
  ExcludeRules = @(
    'PSAvoidUsingConvertToSecureStringWithPlainText'
  }
}

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

5 participants