From 483842908edf75a25c2b2b6596744d10394ca71d Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Tue, 5 Mar 2024 14:39:49 +0100 Subject: [PATCH] dev: simplify exclude processors (#4456) --- pkg/lint/runner.go | 33 ++--- pkg/result/processors/base_rule.go | 2 + pkg/result/processors/exclude.go | 49 +++---- pkg/result/processors/exclude_rules.go | 94 +++++++------ pkg/result/processors/exclude_rules_test.go | 145 +++++++++++++------- pkg/result/processors/exclude_test.go | 16 ++- pkg/result/processors/severity.go | 2 +- 7 files changed, 186 insertions(+), 155 deletions(-) diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 150e6c7f..dc6b2e73 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -254,20 +254,15 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, s } func getExcludeProcessor(cfg *config.Issues) processors.Processor { - var excludeTotalPattern string + opts := processors.ExcludeOptions{ + CaseSensitive: cfg.ExcludeCaseSensitive, + } if len(cfg.ExcludePatterns) != 0 { - excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(cfg.ExcludePatterns, "|")) + opts.Pattern = fmt.Sprintf("(%s)", strings.Join(cfg.ExcludePatterns, "|")) } - var excludeProcessor processors.Processor - if cfg.ExcludeCaseSensitive { - excludeProcessor = processors.NewExcludeCaseSensitive(excludeTotalPattern) - } else { - excludeProcessor = processors.NewExclude(excludeTotalPattern) - } - - return excludeProcessor + return processors.NewExclude(opts) } func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsutils.Files) processors.Processor { @@ -295,22 +290,12 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsuti } } - var excludeRulesProcessor processors.Processor - if cfg.ExcludeCaseSensitive { - excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive( - excludeRules, - files, - log.Child(logutils.DebugKeyExcludeRules), - ) - } else { - excludeRulesProcessor = processors.NewExcludeRules( - excludeRules, - files, - log.Child(logutils.DebugKeyExcludeRules), - ) + opts := processors.ExcludeRulesOptions{ + Rules: excludeRules, + CaseSensitive: cfg.ExcludeCaseSensitive, } - return excludeRulesProcessor + return processors.NewExcludeRules(log.Child(logutils.DebugKeyExcludeRules), files, opts) } func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor { diff --git a/pkg/result/processors/base_rule.go b/pkg/result/processors/base_rule.go index b5e13880..88140bfa 100644 --- a/pkg/result/processors/base_rule.go +++ b/pkg/result/processors/base_rule.go @@ -8,6 +8,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +const caseInsensitivePrefix = "(?i)" + type BaseRule struct { Text string Source string diff --git a/pkg/result/processors/exclude.go b/pkg/result/processors/exclude.go index 92959a32..ad684299 100644 --- a/pkg/result/processors/exclude.go +++ b/pkg/result/processors/exclude.go @@ -6,24 +6,37 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +var _ Processor = Exclude{} + type Exclude struct { + name string + pattern *regexp.Regexp } -var _ Processor = Exclude{} +type ExcludeOptions struct { + Pattern string + CaseSensitive bool +} -func NewExclude(pattern string) *Exclude { - var patternRe *regexp.Regexp - if pattern != "" { - patternRe = regexp.MustCompile("(?i)" + pattern) +func NewExclude(opts ExcludeOptions) *Exclude { + p := &Exclude{name: "exclude"} + + prefix := caseInsensitivePrefix + if opts.CaseSensitive { + p.name = "exclude-case-sensitive" + prefix = "" } - return &Exclude{ - pattern: patternRe, + + if opts.Pattern != "" { + p.pattern = regexp.MustCompile(prefix + opts.Pattern) } + + return p } func (p Exclude) Name() string { - return "exclude" + return p.name } func (p Exclude) Process(issues []result.Issue) ([]result.Issue, error) { @@ -37,23 +50,3 @@ func (p Exclude) Process(issues []result.Issue) ([]result.Issue, error) { } func (p Exclude) Finish() {} - -type ExcludeCaseSensitive struct { - *Exclude -} - -var _ Processor = ExcludeCaseSensitive{} - -func NewExcludeCaseSensitive(pattern string) *ExcludeCaseSensitive { - var patternRe *regexp.Regexp - if pattern != "" { - patternRe = regexp.MustCompile(pattern) - } - return &ExcludeCaseSensitive{ - &Exclude{pattern: patternRe}, - } -} - -func (p ExcludeCaseSensitive) Name() string { - return "exclude-case-sensitive" -} diff --git a/pkg/result/processors/exclude_rules.go b/pkg/result/processors/exclude_rules.go index 2f7e30b4..c2eba63d 100644 --- a/pkg/result/processors/exclude_rules.go +++ b/pkg/result/processors/exclude_rules.go @@ -8,6 +8,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +var _ Processor = ExcludeRules{} + type excludeRule struct { baseRule } @@ -17,43 +19,35 @@ type ExcludeRule struct { } type ExcludeRules struct { - rules []excludeRule - files *fsutils.Files + name string + log logutils.Log + files *fsutils.Files + + rules []excludeRule } -func NewExcludeRules(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRules { - r := &ExcludeRules{ +type ExcludeRulesOptions struct { + Rules []ExcludeRule + CaseSensitive bool +} + +func NewExcludeRules(log logutils.Log, files *fsutils.Files, opts ExcludeRulesOptions) *ExcludeRules { + p := &ExcludeRules{ + name: "exclude-rules", files: files, log: log, } - r.rules = createRules(rules, "(?i)") - return r -} - -func createRules(rules []ExcludeRule, prefix string) []excludeRule { - parsedRules := make([]excludeRule, 0, len(rules)) - for _, rule := range rules { - parsedRule := excludeRule{} - parsedRule.linters = rule.Linters - 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 := caseInsensitivePrefix + if opts.CaseSensitive { + prefix = "" + p.name = "exclude-rules-case-sensitive" } - return parsedRules + + p.rules = createRules(opts.Rules, prefix) + + return p } func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) { @@ -71,25 +65,35 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) { }), nil } -func (ExcludeRules) Name() string { return "exclude-rules" } -func (ExcludeRules) Finish() {} +func (p ExcludeRules) Name() string { return p.name } -var _ Processor = ExcludeRules{} +func (ExcludeRules) Finish() {} -type ExcludeRulesCaseSensitive struct { - *ExcludeRules -} +func createRules(rules []ExcludeRule, prefix string) []excludeRule { + parsedRules := make([]excludeRule, 0, len(rules)) -func NewExcludeRulesCaseSensitive(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRulesCaseSensitive { - r := &ExcludeRules{ - files: files, - log: log, + for _, rule := range rules { + parsedRule := excludeRule{} + parsedRule.linters = rule.Linters + + if rule.Text != "" { + parsedRule.text = regexp.MustCompile(prefix + rule.Text) + } + + if rule.Source != "" { + parsedRule.source = regexp.MustCompile(prefix + rule.Source) + } + + if rule.Path != "" { + parsedRule.path = regexp.MustCompile(fsutils.NormalizePathInRegex(rule.Path)) + } + + if rule.PathExcept != "" { + parsedRule.pathExcept = regexp.MustCompile(fsutils.NormalizePathInRegex(rule.PathExcept)) + } + + parsedRules = append(parsedRules, parsedRule) } - r.rules = createRules(rules, "") - return &ExcludeRulesCaseSensitive{r} + return parsedRules } - -func (ExcludeRulesCaseSensitive) Name() string { return "exclude-rules-case-sensitive" } - -var _ Processor = ExcludeCaseSensitive{} diff --git a/pkg/result/processors/exclude_rules_test.go b/pkg/result/processors/exclude_rules_test.go index 247cb320..96f6788e 100644 --- a/pkg/result/processors/exclude_rules_test.go +++ b/pkg/result/processors/exclude_rules_test.go @@ -11,11 +11,11 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -func TestExcludeRulesMultiple(t *testing.T) { +func TestExcludeRules_multiple(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) files := fsutils.NewFiles(lineCache, "") - p := NewExcludeRules([]ExcludeRule{ + opts := ExcludeRulesOptions{Rules: []ExcludeRule{ { BaseRule: BaseRule{ Text: "^exclude$", @@ -46,7 +46,9 @@ func TestExcludeRulesMultiple(t *testing.T) { Linters: []string{"lll"}, }, }, - }, files, nil) + }} + + p := NewExcludeRules(nil, files, opts) //nolint:dupl cases := []issueTestCase{ @@ -60,11 +62,14 @@ func TestExcludeRulesMultiple(t *testing.T) { {Path: "e_test.go", Text: "nontestonly", Linter: "linter"}, {Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"}, } + 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{ @@ -74,37 +79,46 @@ func TestExcludeRulesMultiple(t *testing.T) { Line: i.Line(), }) } + expectedCases := []issueTestCase{ {Path: "e.go", Text: "some", Linter: "linter"}, {Path: "e_Test.go", Text: "normal", Linter: "testlinter"}, {Path: "e_test.go", Text: "another", Linter: "linter"}, {Path: "e_test.go", Text: "nontestonly", Linter: "linter"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestExcludeRulesPathPrefix(t *testing.T) { +func TestExcludeRules_pathPrefix(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) pathPrefix := path.Join("some", "dir") files := fsutils.NewFiles(lineCache, pathPrefix) - p := NewExcludeRules([]ExcludeRule{ - { - BaseRule: BaseRule{ - Path: `some/dir/e\.go`, + opts := ExcludeRulesOptions{ + Rules: []ExcludeRule{ + { + BaseRule: BaseRule{ + Path: `some/dir/e\.go`, + }, }, }, - }, files, nil) + } + + p := NewExcludeRules(nil, files, opts) cases := []issueTestCase{ {Path: "e.go"}, {Path: "other.go"}, } + 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{ @@ -114,21 +128,28 @@ func TestExcludeRulesPathPrefix(t *testing.T) { Line: i.Line(), }) } + expectedCases := []issueTestCase{ {Path: "other.go"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestExcludeRulesText(t *testing.T) { - p := NewExcludeRules([]ExcludeRule{ - { - BaseRule: BaseRule{ - Text: "^exclude$", - Linters: []string{"linter"}, +func TestExcludeRules_text(t *testing.T) { + opts := ExcludeRulesOptions{ + Rules: []ExcludeRule{ + { + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, + }, }, }, - }, nil, nil) + } + + p := NewExcludeRules(nil, nil, opts) + texts := []string{"excLude", "1", "", "exclud", "notexclude"} var issues []result.Issue for _, t := range texts { @@ -145,42 +166,49 @@ func TestExcludeRulesText(t *testing.T) { for _, i := range processedIssues { processedTexts = append(processedTexts, i.Text) } + assert.Equal(t, texts[1:], processedTexts) } -func TestExcludeRulesEmpty(t *testing.T) { - processAssertSame(t, NewExcludeRules(nil, nil, nil), newIssueFromTextTestCase("test")) +func TestExcludeRules_empty(t *testing.T) { + processAssertSame(t, NewExcludeRules(nil, nil, ExcludeRulesOptions{}), newIssueFromTextTestCase("test")) } -func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) { +func TestExcludeRules_caseSensitive_multiple(t *testing.T) { lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) files := fsutils.NewFiles(lineCache, "") - p := NewExcludeRulesCaseSensitive([]ExcludeRule{ - { - BaseRule: BaseRule{ - Text: "^exclude$", - Linters: []string{"linter"}, + + opts := ExcludeRulesOptions{ + CaseSensitive: true, + Rules: []ExcludeRule{ + { + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, + }, + }, + { + BaseRule: BaseRule{ + Linters: []string{"testlinter"}, + Path: `_test\.go`, + }, + }, + { + BaseRule: BaseRule{ + Text: "^testonly$", + Path: `_test\.go`, + }, + }, + { + BaseRule: BaseRule{ + Source: "^//go:generate ", + Linters: []string{"lll"}, + }, }, }, - { - BaseRule: BaseRule{ - Linters: []string{"testlinter"}, - Path: `_test\.go`, - }, - }, - { - BaseRule: BaseRule{ - Text: "^testonly$", - Path: `_test\.go`, - }, - }, - { - BaseRule: BaseRule{ - Source: "^//go:generate ", - Linters: []string{"lll"}, - }, - }, - }, files, nil) + } + + p := NewExcludeRules(nil, files, opts) //nolint:dupl cases := []issueTestCase{ @@ -194,11 +222,14 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) { {Path: "e_test.go", Text: "testOnly", Linter: "linter"}, {Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"}, } + 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{ @@ -208,6 +239,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) { Line: i.Line(), }) } + expectedCases := []issueTestCase{ {Path: "e.go", Text: "excLude", Linter: "linter"}, {Path: "e.go", Text: "some", Linter: "linter"}, @@ -216,19 +248,27 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) { {Path: "e_test.go", Text: "testOnly", Linter: "linter"}, {Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"}, } + assert.Equal(t, expectedCases, resultingCases) } -func TestExcludeRulesCaseSensitiveText(t *testing.T) { - p := NewExcludeRulesCaseSensitive([]ExcludeRule{ - { - BaseRule: BaseRule{ - Text: "^exclude$", - Linters: []string{"linter"}, +func TestExcludeRules_caseSensitive_text(t *testing.T) { + opts := ExcludeRulesOptions{ + CaseSensitive: true, + Rules: []ExcludeRule{ + { + BaseRule: BaseRule{ + Text: "^exclude$", + Linters: []string{"linter"}, + }, }, }, - }, nil, nil) + } + + p := NewExcludeRules(nil, nil, opts) + texts := []string{"exclude", "excLude", "1", "", "exclud", "notexclude"} + var issues []result.Issue for _, t := range texts { issues = append(issues, result.Issue{ @@ -244,9 +284,10 @@ func TestExcludeRulesCaseSensitiveText(t *testing.T) { for _, i := range processedIssues { processedTexts = append(processedTexts, i.Text) } + assert.Equal(t, texts[1:], processedTexts) } -func TestExcludeRulesCaseSensitiveEmpty(t *testing.T) { - processAssertSame(t, NewExcludeRulesCaseSensitive(nil, nil, nil), newIssueFromTextTestCase("test")) +func TestExcludeRules_caseSensitive_empty(t *testing.T) { + processAssertSame(t, NewExcludeRules(nil, nil, ExcludeRulesOptions{CaseSensitive: true}), newIssueFromTextTestCase("test")) } diff --git a/pkg/result/processors/exclude_test.go b/pkg/result/processors/exclude_test.go index 3b92ccd7..a62f4ef5 100644 --- a/pkg/result/processors/exclude_test.go +++ b/pkg/result/processors/exclude_test.go @@ -9,8 +9,10 @@ import ( ) func TestExclude(t *testing.T) { - p := NewExclude("^exclude$") + p := NewExclude(ExcludeOptions{Pattern: "^exclude$"}) + texts := []string{"excLude", "1", "", "exclud", "notexclude"} + var issues []result.Issue for _, t := range texts { issues = append(issues, newIssueFromTextTestCase(t)) @@ -23,16 +25,19 @@ func TestExclude(t *testing.T) { for _, i := range processedIssues { processedTexts = append(processedTexts, i.Text) } + assert.Equal(t, texts[1:], processedTexts) } -func TestNoExclude(t *testing.T) { - processAssertSame(t, NewExclude(""), newIssueFromTextTestCase("test")) +func TestExclude_empty(t *testing.T) { + processAssertSame(t, NewExclude(ExcludeOptions{}), newIssueFromTextTestCase("test")) } -func TestExcludeCaseSensitive(t *testing.T) { - p := NewExcludeCaseSensitive("^exclude$") +func TestExclude_caseSensitive(t *testing.T) { + p := NewExclude(ExcludeOptions{Pattern: "^exclude$", CaseSensitive: true}) + texts := []string{"excLude", "1", "", "exclud", "exclude"} + var issues []result.Issue for _, t := range texts { issues = append(issues, newIssueFromTextTestCase(t)) @@ -45,5 +50,6 @@ func TestExcludeCaseSensitive(t *testing.T) { for _, i := range processedIssues { processedTexts = append(processedTexts, i.Text) } + assert.Equal(t, texts[:len(texts)-1], processedTexts) } diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index fdbbe322..2a21e9e5 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -48,7 +48,7 @@ func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) * override: opts.Override, } - prefix := "(?i)" + prefix := caseInsensitivePrefix if opts.CaseSensitive { prefix = "" p.name = "severity-rules-case-sensitive"