From 440133ca307a978ee3c398fa8ee28ea12638522b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:31:20 +0100 Subject: [PATCH 01/11] Revert "feat: option to not override severity from linters (#4452)" This reverts commit 3d9135248ba0ac28fae50bdfc06f4593e03fb940. --- .golangci.reference.yml | 4 ---- pkg/config/severity.go | 7 +++---- pkg/lint/runner.go | 1 - pkg/result/processors/severity.go | 7 ------- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 6a6239774b19..ae5a971ab04a 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2888,10 +2888,6 @@ severity: # 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. diff --git a/pkg/config/severity.go b/pkg/config/severity.go index 49874c9a4c3a..a6d2c9ec3fb3 100644 --- a/pkg/config/severity.go +++ b/pkg/config/severity.go @@ -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 { diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 78be9382dac4..13daa3b3b6cd 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -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) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 943b14a429db..8c62b64b9d5b 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -24,7 +24,6 @@ type SeverityOptions struct { Default string Rules []SeverityRule CaseSensitive bool - Override bool } type Severity struct { @@ -36,7 +35,6 @@ type Severity struct { defaultSeverity string rules []severityRule - override bool } func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *Severity { @@ -45,7 +43,6 @@ func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) * files: files, log: log, defaultSeverity: opts.Default, - override: opts.Override, } prefix := caseInsensitivePrefix @@ -65,10 +62,6 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { } return transformIssues(issues, func(issue *result.Issue) *result.Issue { - if issue.Severity != "" && !p.override { - return issue - } - for _, rule := range p.rules { rule := rule From 5edb11cf9d05092b72341fe7e44e966e161a5344 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:31:59 +0100 Subject: [PATCH 02/11] chore: move severity computing --- pkg/result/processors/severity.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 8c62b64b9d5b..1a94e0927dbf 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -65,12 +65,12 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { for _, rule := range p.rules { rule := rule - ruleSeverity := p.defaultSeverity - if rule.severity != "" { - ruleSeverity = rule.severity - } - if rule.match(issue, p.files, p.log) { + ruleSeverity := p.defaultSeverity + if rule.severity != "" { + ruleSeverity = rule.severity + } + issue.Severity = ruleSeverity return issue } From 1d47a184ff471979d972a4cec32a30766e9a0ae7 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:32:35 +0100 Subject: [PATCH 03/11] chore: use field instead of variable --- pkg/result/processors/severity.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 1a94e0927dbf..04f085c04611 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -66,12 +66,11 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { rule := rule if rule.match(issue, p.files, p.log) { - ruleSeverity := p.defaultSeverity + issue.Severity = p.defaultSeverity if rule.severity != "" { - ruleSeverity = rule.severity + issue.Severity = rule.severity } - issue.Severity = ruleSeverity return issue } } From 3a8eb913ee56ff79c5bb52286b670eed37f9dd8b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:33:17 +0100 Subject: [PATCH 04/11] chore: invert if --- pkg/result/processors/severity.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 04f085c04611..9be7f6bea647 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -66,9 +66,9 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { rule := rule if rule.match(issue, p.files, p.log) { - issue.Severity = p.defaultSeverity - if rule.severity != "" { - issue.Severity = rule.severity + issue.Severity = rule.severity + if issue.Severity == "" { + issue.Severity = p.defaultSeverity } return issue From 1cc987a958bd5dc86a78f476863805c582ff5eb3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:35:10 +0100 Subject: [PATCH 05/11] feat: use a specific severity value to keep linter severity Inspire by DNS notation --- pkg/result/processors/severity.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 9be7f6bea647..17704f1b2c2f 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -66,6 +66,10 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { rule := rule if rule.match(issue, p.files, p.log) { + if rule.severity == "@" || rule.severity == "" && p.defaultSeverity == "@" { + return issue + } + issue.Severity = rule.severity if issue.Severity == "" { issue.Severity = p.defaultSeverity @@ -75,7 +79,9 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { } } - issue.Severity = p.defaultSeverity + if p.defaultSeverity != "@" { + issue.Severity = p.defaultSeverity + } return issue }), nil From 97a9a6d66d7ffdd8fea50b5b4cbf9d235c391df9 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:47:38 +0100 Subject: [PATCH 06/11] chore: fix missing field inside newIssueFromIssueTestCase --- pkg/result/processors/processor_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/result/processors/processor_test.go b/pkg/result/processors/processor_test.go index d263cf294b85..216a8fcd5997 100644 --- a/pkg/result/processors/processor_test.go +++ b/pkg/result/processors/processor_test.go @@ -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, From df8cf939827f01fddee5409f261048d60235a4f3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:49:43 +0100 Subject: [PATCH 07/11] docs: add explanation inside the reference file --- .golangci.reference.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index ae5a971ab04a..2e48669b07af 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2881,6 +2881,8 @@ 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 # + # `@` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...) + # # Default: "" default-severity: error @@ -2891,6 +2893,9 @@ severity: # 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. + # + # `@` 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: [] From 7846b86f95f0febc4294414945a39321524ac799 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 22:50:17 +0100 Subject: [PATCH 08/11] chore: convert anomynous function to method --- pkg/result/processors/severity.go | 36 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 17704f1b2c2f..5bda4450002e 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -61,30 +61,32 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { return issues, nil } - return transformIssues(issues, func(issue *result.Issue) *result.Issue { - for _, rule := range p.rules { - rule := rule - - if rule.match(issue, p.files, p.log) { - if rule.severity == "@" || rule.severity == "" && p.defaultSeverity == "@" { - return issue - } + return transformIssues(issues, p.transform), nil +} - issue.Severity = rule.severity - if issue.Severity == "" { - issue.Severity = p.defaultSeverity - } +func (p *Severity) transform(issue *result.Issue) *result.Issue { + for _, rule := range p.rules { + rule := rule + if rule.match(issue, p.files, p.log) { + if rule.severity == "@" || rule.severity == "" && p.defaultSeverity == "@" { return issue } - } - if p.defaultSeverity != "@" { - issue.Severity = p.defaultSeverity + issue.Severity = rule.severity + if issue.Severity == "" { + issue.Severity = p.defaultSeverity + } + + return issue } + } + + if p.defaultSeverity != "@" { + issue.Severity = p.defaultSeverity + } - return issue - }), nil + return issue } func (p *Severity) Name() string { return p.name } From eff3ab17053ec9fcfefb7fcd276709ee47868438 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 23:04:01 +0100 Subject: [PATCH 09/11] tests: add tests on transform --- pkg/result/processors/severity_test.go | 184 +++++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/pkg/result/processors/severity_test.go b/pkg/result/processors/severity_test.go index 9690b5505edd..2f28e9762418 100644 --- a/pkg/result/processors/severity_test.go +++ b/pkg/result/processors/severity_test.go @@ -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: "@", + 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: "@", + 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: "@", + }, + 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) + }) + } +} From d7666392bec395e5ca59091b89f2437c610cd099 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 9 Mar 2024 15:48:48 +0100 Subject: [PATCH 10/11] chore: use @linter instead of @ --- .golangci.reference.yml | 4 ++-- pkg/result/processors/severity.go | 6 ++++-- pkg/result/processors/severity_test.go | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 2e48669b07af..27293fb8948e 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2881,7 +2881,7 @@ 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 # - # `@` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...) + # `@linter` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...) # # Default: "" default-severity: error @@ -2894,7 +2894,7 @@ severity: # Severity rules have the same filtering capability as exclude rules # except you are allowed to specify one matcher per severity rule. # - # `@` can be used as severity value to keep the severity from linters (e.g. revive, gosec, ...) + # `@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. # diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index 5bda4450002e..c41c47e512fe 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -8,6 +8,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +const severityFromLinter = "@linter" + var _ Processor = &Severity{} type severityRule struct { @@ -69,7 +71,7 @@ func (p *Severity) transform(issue *result.Issue) *result.Issue { rule := rule if rule.match(issue, p.files, p.log) { - if rule.severity == "@" || rule.severity == "" && p.defaultSeverity == "@" { + if rule.severity == severityFromLinter || rule.severity == "" && p.defaultSeverity == severityFromLinter { return issue } @@ -82,7 +84,7 @@ func (p *Severity) transform(issue *result.Issue) *result.Issue { } } - if p.defaultSeverity != "@" { + if p.defaultSeverity != severityFromLinter { issue.Severity = p.defaultSeverity } diff --git a/pkg/result/processors/severity_test.go b/pkg/result/processors/severity_test.go index 2f28e9762418..cbf18e6c79ae 100644 --- a/pkg/result/processors/severity_test.go +++ b/pkg/result/processors/severity_test.go @@ -421,7 +421,7 @@ func TestSeverity_transform(t *testing.T) { Default: "error", Rules: []SeverityRule{ { - Severity: "@", + Severity: severityFromLinter, BaseRule: BaseRule{ Linters: []string{"linter1"}, }, @@ -442,7 +442,7 @@ func TestSeverity_transform(t *testing.T) { { desc: "keep severity from linter as default", opts: SeverityOptions{ - Default: "@", + Default: severityFromLinter, Rules: []SeverityRule{ { Severity: "info", @@ -466,7 +466,7 @@ func TestSeverity_transform(t *testing.T) { { desc: "keep severity from linter as default (without rule)", opts: SeverityOptions{ - Default: "@", + Default: severityFromLinter, }, issue: &result.Issue{ Text: "This is a report", From 6c033fa7038a2c26e2fc333336a4a7247eb855f4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 9 Mar 2024 15:53:53 +0100 Subject: [PATCH 11/11] review: remove useless variable copy --- pkg/result/processors/severity.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index c41c47e512fe..f087f951b7f2 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -68,8 +68,6 @@ func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { func (p *Severity) transform(issue *result.Issue) *result.Issue { for _, rule := range p.rules { - rule := rule - if rule.match(issue, p.files, p.log) { if rule.severity == severityFromLinter || rule.severity == "" && p.defaultSeverity == severityFromLinter { return issue