PowerShell / PowerShell/PSScriptAnalyzer

`Invoke-ScriptAnalyzer` `-Severity` filters rules rather than diagnostics

未关闭
#2,156 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

主要语言
C#
星标
2.2k
派生
414
平均合并
13 小时 1 分钟
30 天内合并 PR
2

描述

The -Severity parameter of Invoke-ScriptAnalyzer does not do what it says it does.

Parameter help for the -Severity parameter says:

After running Script Analyzer with all rules, this parameter selects rule violations with the specified severity.
...
The parameter filters the rules violations only after running all rules.
...

In reality the parameter filters rules and not diagnostics; both have the concept of Severity (RuleSeverity, and DiagnosticSeverity)


How the -Severity parameter is ultimately used:

  1. The user passes -Severity to Invoke-ScriptAnalyzer. Value validated to be one of "Warning", "Error", "Information", "ParseError".

  2. The ScriptAnalyzer singleton instance is initialised with this -Severity string[] value, which is stored as a private member of the singleton object (severity).

  3. When analyzing a script, the engine determines which rules to run. It uses IsRuleAllowed() to check each rule.

  4. IsRuleAllowed() gets a list of allowedSeverities. It does so by calling GetAllowedSeveritiesInInt().

    Each severity (an array of strings) is parsed into the underlying uint value of the DiagnosticSeverity enum type.

    (I believe this should actually be RuleSeverity enum. It works as both enums have identical members and underlying uint values).
    https://github.com/PowerShell/PSScriptAnalyzer/blob/aba29c31151925bd7180e7dfd578e1bfacdaf2a2/Engine/ScriptAnalyzer.cs#L1929-L1934

  5. IsRuleAllowed() then calls IsSeverityAllowed(allowedSeverities, rule) as part of it's decision making on whether to execute a rule. Passing in the list of allowedSeverities and the current rule being considered.

  6. IsSeverityAllowed(..) then checks if allowedSeverities contains the rules severity (calling rule.GetSeverity() and casting it to a uint). If it does the rule is allowed to run.
    https://github.com/PowerShell/PSScriptAnalyzer/blob/aba29c31151925bd7180e7dfd578e1bfacdaf2a2/Engine/ScriptAnalyzer.cs#L1914-L1921


So there are 2 issues that need to be addressed:

  1. There is a disparity between the documentation and implementation. Either:

    • The documentation needs updating to match reality OR
    • The implementation needs to be updated so -Severity filters the output of DiagnosticRecords.
  2. GetAllowedSeveritiesInInt() resolves severities using DiagnosticSeverity and that is then used to compare to rule.GetSeverity() which is a RuleSeverity. It works now as both enums are identical in their definition and are being cast to their underlying uint value. If either was updated this could break. It also does not seem intentional.

@bergmeister - Should we update the documentation as a first pass and revisit behaviour later if it's desired? I will also get the enum corrected to RuleSeverity.

This explains the issue described in #2049 - where a custom rule is emitting error-level DiagnosticRecord but is not showing with -Severity Error, but is with -Severity Warning. -Severity is filtering the rules that are run (by their severity) and all custom rules have Warning severity.

https://github.com/PowerShell/PSScriptAnalyzer/blob/aba29c31151925bd7180e7dfd578e1bfacdaf2a2/Engine/Generic/ExternalRule.cs#L55-L59

n.b. I have typed the word severity so many times now that it no longer looks like a real word...

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 Engine/Commands/InvokeScriptAnalyzerCommand.cs 和 Engine/ScriptAnalyzer.cs 开始,重点关注严重性初始化以及 GetAllowedSeveritiesInInt()、IsSeverityAllowed() 和 IsRuleAllowed() 入口点。确认过滤规则而不是 DiagnosticRecords 时的预期行为,然后确保文档和严重性处理一致地反映这一决定。

由索引模型根据 Issue 内容生成。

评估

技术栈
csharp, powershell
领域
tooling
Issue 类型
缺陷
难度
4/5
预计耗时
3-5 天
活跃度
冷清
描述清晰度
基本清楚
新手友好度
45/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。