Skip to content

Commit

Permalink
feat: syntax to not override severity from linters (#4472)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez authored Mar 11, 2024
1 parent bb30bbe commit ec52d3c
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 27 deletions.
9 changes: 5 additions & 4 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2881,20 +2881,21 @@ severity:
# - GitHub: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
# - TeamCity: https://www.jetbrains.com/help/teamcity/service-messages.html#Inspection+Instance
#
# `@linter` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...)
#
# Default: ""
default-severity: error

# If set to true `severity-rules` regular expressions become case-sensitive.
# Default: false
case-sensitive: true

# Don't override severity defined by linters.
# Default: false
keep-linter-severity: true

# When a list of severity rules are provided, severity information will be added to lint issues.
# Severity rules have the same filtering capability as exclude rules
# except you are allowed to specify one matcher per severity rule.
#
# `@linter` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...)
#
# Only affects out formats that support setting severity information.
#
# Default: []
Expand Down
7 changes: 3 additions & 4 deletions pkg/config/severity.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import (
const severityRuleMinConditionsCount = 1

type Severity struct {
Default string `mapstructure:"default-severity"`
CaseSensitive bool `mapstructure:"case-sensitive"`
Rules []SeverityRule `mapstructure:"rules"`
KeepLinterSeverity bool `mapstructure:"keep-linter-severity"` // TODO(ldez): in v2 should be changed to `Override`.
Default string `mapstructure:"default-severity"`
CaseSensitive bool `mapstructure:"case-sensitive"`
Rules []SeverityRule `mapstructure:"rules"`
}

func (s *Severity) Validate() error {
Expand Down
1 change: 0 additions & 1 deletion pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs
Default: cfg.Default,
Rules: severityRules,
CaseSensitive: cfg.CaseSensitive,
Override: !cfg.KeepLinterSeverity,
}

return processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, severityOpts)
Expand Down
1 change: 1 addition & 0 deletions pkg/result/processors/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func newIssueFromIssueTestCase(c issueTestCase) result.Issue {
return result.Issue{
Text: c.Text,
FromLinter: c.Linter,
Severity: c.Severity,
Pos: token.Position{
Filename: c.Path,
Line: c.Line,
Expand Down
36 changes: 18 additions & 18 deletions pkg/result/processors/severity.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

const severityFromLinter = "@linter"

var _ Processor = &Severity{}

type severityRule struct {
Expand All @@ -24,7 +26,6 @@ type SeverityOptions struct {
Default string
Rules []SeverityRule
CaseSensitive bool
Override bool
}

type Severity struct {
Expand All @@ -36,7 +37,6 @@ type Severity struct {

defaultSeverity string
rules []severityRule
override bool
}

func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *Severity {
Expand All @@ -45,7 +45,6 @@ func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *
files: files,
log: log,
defaultSeverity: opts.Default,
override: opts.Override,
}

prefix := caseInsensitivePrefix
Expand All @@ -64,29 +63,30 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) {
return issues, nil
}

return transformIssues(issues, func(issue *result.Issue) *result.Issue {
if issue.Severity != "" && !p.override {
return issue
}

for _, rule := range p.rules {
rule := rule
return transformIssues(issues, p.transform), nil
}

ruleSeverity := p.defaultSeverity
if rule.severity != "" {
ruleSeverity = rule.severity
func (p *Severity) transform(issue *result.Issue) *result.Issue {
for _, rule := range p.rules {
if rule.match(issue, p.files, p.log) {
if rule.severity == severityFromLinter || rule.severity == "" && p.defaultSeverity == severityFromLinter {
return issue
}

if rule.match(issue, p.files, p.log) {
issue.Severity = ruleSeverity
return issue
issue.Severity = rule.severity
if issue.Severity == "" {
issue.Severity = p.defaultSeverity
}

return issue
}
}

if p.defaultSeverity != severityFromLinter {
issue.Severity = p.defaultSeverity
}

return issue
}), nil
return issue
}

func (p *Severity) Name() string { return p.name }
Expand Down
184 changes: 184 additions & 0 deletions pkg/result/processors/severity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,187 @@ func TestSeverity_caseSensitive(t *testing.T) {

assert.Equal(t, expectedCases, resultingCases)
}

func TestSeverity_transform(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "")

testCases := []struct {
desc string
opts SeverityOptions
issue *result.Issue
expected *result.Issue
}{
{
desc: "apply severity from rule",
opts: SeverityOptions{
Default: "error",
Rules: []SeverityRule{
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
Severity: "info",
},
},
{
desc: "apply severity from default",
opts: SeverityOptions{
Default: "error",
Rules: []SeverityRule{
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "error",
},
},
{
desc: "severity from rule override severity from linter",
opts: SeverityOptions{
Default: "error",
Rules: []SeverityRule{
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
Severity: "huge",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
Severity: "info",
},
},
{
desc: "severity from default override severity from linter",
opts: SeverityOptions{
Default: "error",
Rules: []SeverityRule{
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "huge",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "error",
},
},
{
desc: "keep severity from linter as rule",
opts: SeverityOptions{
Default: "error",
Rules: []SeverityRule{
{
Severity: severityFromLinter,
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
Severity: "huge",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter1",
Severity: "huge",
},
},
{
desc: "keep severity from linter as default",
opts: SeverityOptions{
Default: severityFromLinter,
Rules: []SeverityRule{
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter1"},
},
},
},
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "huge",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "huge",
},
},
{
desc: "keep severity from linter as default (without rule)",
opts: SeverityOptions{
Default: severityFromLinter,
},
issue: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "huge",
},
expected: &result.Issue{
Text: "This is a report",
FromLinter: "linter2",
Severity: "huge",
},
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

p := NewSeverity(nil, files, test.opts)

newIssue := p.transform(test.issue)

assert.Equal(t, test.expected, newIssue)
})
}
}

0 comments on commit ec52d3c

Please sign in to comment.