From f81c3f26c1d7ef3e5750b41a0961c154727c2fe6 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Mon, 4 Mar 2024 17:33:36 +0100 Subject: [PATCH] dev: simplify severity processor (#4451) --- pkg/lint/runner.go | 21 +- .../{severity_rules.go => severity.go} | 109 +++++----- ...everity_rules_test.go => severity_test.go} | 205 +++++++++++------- 3 files changed, 194 insertions(+), 141 deletions(-) rename pkg/result/processors/{severity_rules.go => severity.go} (61%) rename pkg/result/processors/{severity_rules_test.go => severity_test.go} (73%) diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index d5571c236def..d1a096db4514 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -328,22 +328,11 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs }) } - var severityRulesProcessor processors.Processor - if cfg.CaseSensitive { - severityRulesProcessor = processors.NewSeverityRulesCaseSensitive( - cfg.Default, - severityRules, - files, - log.Child(logutils.DebugKeySeverityRules), - ) - } else { - severityRulesProcessor = processors.NewSeverityRules( - cfg.Default, - severityRules, - files, - log.Child(logutils.DebugKeySeverityRules), - ) + severityOpts := processors.SeverityOptions{ + Default: cfg.Default, + Rules: severityRules, + CaseSensitive: cfg.CaseSensitive, } - return severityRulesProcessor + return processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, severityOpts) } diff --git a/pkg/result/processors/severity_rules.go b/pkg/result/processors/severity.go similarity index 61% rename from pkg/result/processors/severity_rules.go rename to pkg/result/processors/severity.go index 0a4a643b7120..b24fd58422d8 100644 --- a/pkg/result/processors/severity_rules.go +++ b/pkg/result/processors/severity.go @@ -8,6 +8,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +var _ Processor = &Severity{} + type severityRule struct { baseRule severity string @@ -18,53 +20,47 @@ type SeverityRule struct { Severity string } -type SeverityRules struct { +type SeverityOptions struct { + Default string + Rules []SeverityRule + CaseSensitive bool +} + +type Severity struct { + name string + + log logutils.Log + + files *fsutils.Files + defaultSeverity string rules []severityRule - files *fsutils.Files - log logutils.Log } -func NewSeverityRules(defaultSeverity string, rules []SeverityRule, files *fsutils.Files, log logutils.Log) *SeverityRules { - r := &SeverityRules{ +func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *Severity { + p := &Severity{ + name: "severity-rules", files: files, log: log, - defaultSeverity: defaultSeverity, + defaultSeverity: opts.Default, } - r.rules = createSeverityRules(rules, "(?i)") - - return r -} -func createSeverityRules(rules []SeverityRule, prefix string) []severityRule { - parsedRules := make([]severityRule, 0, len(rules)) - for _, rule := range rules { - parsedRule := severityRule{} - parsedRule.linters = rule.Linters - parsedRule.severity = rule.Severity - if rule.Text != "" { - parsedRule.text = regexp.MustCompile(prefix + rule.Text) - } - if rule.Source != "" { - parsedRule.source = regexp.MustCompile(prefix + rule.Source) - } - if rule.Path != "" { - path := fsutils.NormalizePathInRegex(rule.Path) - parsedRule.path = regexp.MustCompile(path) - } - if rule.PathExcept != "" { - pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept) - parsedRule.pathExcept = regexp.MustCompile(pathExcept) - } - parsedRules = append(parsedRules, parsedRule) + prefix := "(?i)" + if opts.CaseSensitive { + prefix = "" + p.name = "severity-rules-case-sensitive" } - return parsedRules + + p.rules = createSeverityRules(opts.Rules, prefix) + + return p } -func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) { +func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) { if len(p.rules) == 0 && p.defaultSeverity == "" { return issues, nil } + return transformIssues(issues, func(i *result.Issue) *result.Issue { for _, rule := range p.rules { rule := rule @@ -79,30 +75,45 @@ func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) { return i } } + i.Severity = p.defaultSeverity + return i }), nil } -func (SeverityRules) Name() string { return "severity-rules" } -func (SeverityRules) Finish() {} +func (p *Severity) Name() string { return p.name } -var _ Processor = SeverityRules{} +func (*Severity) Finish() {} -type SeverityRulesCaseSensitive struct { - *SeverityRules -} +func createSeverityRules(rules []SeverityRule, prefix string) []severityRule { + parsedRules := make([]severityRule, 0, len(rules)) -func NewSeverityRulesCaseSensitive(defaultSeverity string, rules []SeverityRule, - files *fsutils.Files, log logutils.Log) *SeverityRulesCaseSensitive { - r := &SeverityRules{ - files: files, - log: log, - defaultSeverity: defaultSeverity, + for _, rule := range rules { + parsedRule := severityRule{} + parsedRule.linters = rule.Linters + parsedRule.severity = rule.Severity + + if rule.Text != "" { + parsedRule.text = regexp.MustCompile(prefix + rule.Text) + } + + if rule.Source != "" { + parsedRule.source = regexp.MustCompile(prefix + rule.Source) + } + + if rule.Path != "" { + path := fsutils.NormalizePathInRegex(rule.Path) + parsedRule.path = regexp.MustCompile(path) + } + + if rule.PathExcept != "" { + pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept) + parsedRule.pathExcept = regexp.MustCompile(pathExcept) + } + + parsedRules = append(parsedRules, parsedRule) } - r.rules = createSeverityRules(rules, "") - return &SeverityRulesCaseSensitive{r} + return parsedRules } - -func (SeverityRulesCaseSensitive) Name() string { return "severity-rules-case-sensitive" } diff --git a/pkg/result/processors/severity_rules_test.go b/pkg/result/processors/severity_test.go similarity index 73% rename from pkg/result/processors/severity_rules_test.go rename to pkg/result/processors/severity_test.go index c142524ff49b..574387eda344 100644 --- a/pkg/result/processors/severity_rules_test.go +++ b/pkg/result/processors/severity_test.go @@ -13,67 +13,73 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -func TestSeverityRulesMultiple(t *testing.T) { +func TestSeverity_multiple(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) files := fsutils.NewFiles(lineCache, "") log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) - p := NewSeverityRules("error", []SeverityRule{ - { - Severity: "info", - BaseRule: BaseRule{ - Text: "^ssl$", - Linters: []string{"gosec"}, + + opts := SeverityOptions{ + Default: "error", + Rules: []SeverityRule{ + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^ssl$", + Linters: []string{"gosec"}, + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Linters: []string{"linter"}, - Path: "e.go", + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"linter"}, + Path: "e.go", + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Text: "^testonly$", - Path: `_test\.go`, + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^testonly$", + Path: `_test\.go`, + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Text: "^nontestonly$", - PathExcept: `_test\.go`, + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^nontestonly$", + PathExcept: `_test\.go`, + }, }, - }, - { - BaseRule: BaseRule{ - Source: "^//go:generate ", - Linters: []string{"lll"}, + { + BaseRule: BaseRule{ + Source: "^//go:generate ", + Linters: []string{"lll"}, + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Source: "^//go:dosomething", + { + Severity: "info", + BaseRule: BaseRule{ + Source: "^//go:dosomething", + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Linters: []string{"someotherlinter"}, + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"someotherlinter"}, + }, }, - }, - { - Severity: "info", - BaseRule: BaseRule{ - Linters: []string{"somelinter"}, + { + Severity: "info", + BaseRule: BaseRule{ + Linters: []string{"somelinter"}, + }, + }, + { + Severity: "info", }, }, - { - Severity: "info", - }, - }, files, log) + } + + p := NewSeverity(log, files, opts) cases := []issueTestCase{ {Path: "ssl.go", Text: "ssl", Linter: "gosec"}, @@ -87,11 +93,14 @@ func TestSeverityRulesMultiple(t *testing.T) { {Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter"}, {Path: "empty.go", Text: "empty", Linter: "empty"}, } + var issues []result.Issue for _, c := range cases { issues = append(issues, newIssueFromIssueTestCase(c)) } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase for _, i := range processedIssues { resultingCases = append(resultingCases, issueTestCase{ @@ -102,6 +111,7 @@ func TestSeverityRulesMultiple(t *testing.T) { Severity: i.Severity, }) } + expectedCases := []issueTestCase{ {Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"}, {Path: "e.go", Text: "some", Linter: "linter", Severity: "info"}, @@ -114,33 +124,43 @@ func TestSeverityRulesMultiple(t *testing.T) { {Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter", Severity: "error"}, {Path: "empty.go", Text: "empty", Linter: "empty", Severity: "error"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestSeverityRulesPathPrefix(t *testing.T) { +func TestSeverity_pathPrefix(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) pathPrefix := path.Join("some", "dir") files := fsutils.NewFiles(lineCache, pathPrefix) log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) - p := NewSeverityRules("error", []SeverityRule{ - { - Severity: "info", - BaseRule: BaseRule{ - Text: "some", - Path: `some/dir/e\.go`, + + opts := SeverityOptions{ + Default: "error", + Rules: []SeverityRule{ + { + Severity: "info", + BaseRule: BaseRule{ + Text: "some", + Path: `some/dir/e\.go`, + }, }, }, - }, files, log) + } + + p := NewSeverity(log, files, opts) cases := []issueTestCase{ {Path: "e.go", Text: "some", Linter: "linter"}, {Path: "other.go", Text: "some", Linter: "linter"}, } + var issues []result.Issue for _, c := range cases { issues = append(issues, newIssueFromIssueTestCase(c)) } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase for _, i := range processedIssues { resultingCases = append(resultingCases, issueTestCase{ @@ -151,22 +171,29 @@ func TestSeverityRulesPathPrefix(t *testing.T) { Severity: i.Severity, }) } + expectedCases := []issueTestCase{ {Path: "e.go", Text: "some", Linter: "linter", Severity: "info"}, {Path: "other.go", Text: "some", Linter: "linter", Severity: "error"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestSeverityRulesText(t *testing.T) { - p := NewSeverityRules("", []SeverityRule{ - { - BaseRule: BaseRule{ - Text: "^severity$", - Linters: []string{"linter"}, +func TestSeverity_text(t *testing.T) { + opts := SeverityOptions{ + Rules: []SeverityRule{ + { + BaseRule: BaseRule{ + Text: "^severity$", + Linters: []string{"linter"}, + }, }, }, - }, nil, nil) + } + + p := NewSeverity(nil, nil, opts) + texts := []string{"seveRity", "1", "", "serverit", "notseverity"} var issues []result.Issue for _, t := range texts { @@ -183,24 +210,34 @@ func TestSeverityRulesText(t *testing.T) { for _, i := range processedIssues { processedTexts = append(processedTexts, i.Text) } + assert.Equal(t, texts, processedTexts) } -func TestSeverityRulesOnlyDefault(t *testing.T) { +func TestSeverity_onlyDefault(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) files := fsutils.NewFiles(lineCache, "") log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) - p := NewSeverityRules("info", []SeverityRule{}, files, log) + + opts := SeverityOptions{ + Default: "info", + Rules: []SeverityRule{}, + } + + p := NewSeverity(log, files, opts) cases := []issueTestCase{ {Path: "ssl.go", Text: "ssl", Linter: "gosec"}, {Path: "empty.go", Text: "empty", Linter: "empty"}, } + var issues []result.Issue for _, c := range cases { issues = append(issues, newIssueFromIssueTestCase(c)) } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase for _, i := range processedIssues { resultingCases = append(resultingCases, issueTestCase{ @@ -211,38 +248,52 @@ func TestSeverityRulesOnlyDefault(t *testing.T) { Severity: i.Severity, }) } + expectedCases := []issueTestCase{ {Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"}, {Path: "empty.go", Text: "empty", Linter: "empty", Severity: "info"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestSeverityRulesEmpty(t *testing.T) { - processAssertSame(t, NewSeverityRules("", nil, nil, nil), newIssueFromTextTestCase("test")) +func TestSeverity_empty(t *testing.T) { + p := NewSeverity(nil, nil, SeverityOptions{}) + + processAssertSame(t, p, newIssueFromTextTestCase("test")) } -func TestSeverityRulesCaseSensitive(t *testing.T) { +func TestSeverity_caseSensitive(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) files := fsutils.NewFiles(lineCache, "") - p := NewSeverityRulesCaseSensitive("error", []SeverityRule{ - { - Severity: "info", - BaseRule: BaseRule{ - Text: "^ssl$", - Linters: []string{"gosec", "someotherlinter"}, + + opts := SeverityOptions{ + Default: "error", + Rules: []SeverityRule{ + { + Severity: "info", + BaseRule: BaseRule{ + Text: "^ssl$", + Linters: []string{"gosec", "someotherlinter"}, + }, }, }, - }, files, nil) + CaseSensitive: true, + } + + p := NewSeverity(nil, files, opts) cases := []issueTestCase{ {Path: "e.go", Text: "ssL", Linter: "gosec"}, } + var issues []result.Issue for _, c := range cases { issues = append(issues, newIssueFromIssueTestCase(c)) } + processedIssues := process(t, p, issues...) + var resultingCases []issueTestCase for _, i := range processedIssues { resultingCases = append(resultingCases, issueTestCase{ @@ -253,8 +304,10 @@ func TestSeverityRulesCaseSensitive(t *testing.T) { Severity: i.Severity, }) } + expectedCases := []issueTestCase{ {Path: "e.go", Text: "ssL", Linter: "gosec", Severity: "error"}, } + assert.Equal(t, expectedCases, resultingCases) }