dev: simplify exclude processors (#4456)

This commit is contained in:
Ludovic Fernandez 2024-03-05 14:39:49 +01:00 committed by GitHub
parent 54b0f3c821
commit 483842908e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 186 additions and 155 deletions

View File

@ -254,20 +254,15 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, s
} }
func getExcludeProcessor(cfg *config.Issues) processors.Processor { func getExcludeProcessor(cfg *config.Issues) processors.Processor {
var excludeTotalPattern string opts := processors.ExcludeOptions{
CaseSensitive: cfg.ExcludeCaseSensitive,
}
if len(cfg.ExcludePatterns) != 0 { 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 return processors.NewExclude(opts)
if cfg.ExcludeCaseSensitive {
excludeProcessor = processors.NewExcludeCaseSensitive(excludeTotalPattern)
} else {
excludeProcessor = processors.NewExclude(excludeTotalPattern)
}
return excludeProcessor
} }
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsutils.Files) processors.Processor { 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 opts := processors.ExcludeRulesOptions{
if cfg.ExcludeCaseSensitive { Rules: excludeRules,
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive( CaseSensitive: cfg.ExcludeCaseSensitive,
excludeRules,
files,
log.Child(logutils.DebugKeyExcludeRules),
)
} else {
excludeRulesProcessor = processors.NewExcludeRules(
excludeRules,
files,
log.Child(logutils.DebugKeyExcludeRules),
)
} }
return excludeRulesProcessor return processors.NewExcludeRules(log.Child(logutils.DebugKeyExcludeRules), files, opts)
} }
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor { func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor {

View File

@ -8,6 +8,8 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
const caseInsensitivePrefix = "(?i)"
type BaseRule struct { type BaseRule struct {
Text string Text string
Source string Source string

View File

@ -6,24 +6,37 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
var _ Processor = Exclude{}
type Exclude struct { type Exclude struct {
name string
pattern *regexp.Regexp pattern *regexp.Regexp
} }
var _ Processor = Exclude{} type ExcludeOptions struct {
Pattern string
CaseSensitive bool
}
func NewExclude(pattern string) *Exclude { func NewExclude(opts ExcludeOptions) *Exclude {
var patternRe *regexp.Regexp p := &Exclude{name: "exclude"}
if pattern != "" {
patternRe = regexp.MustCompile("(?i)" + pattern) 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 { func (p Exclude) Name() string {
return "exclude" return p.name
} }
func (p Exclude) Process(issues []result.Issue) ([]result.Issue, error) { 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() {} 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"
}

View File

@ -8,6 +8,8 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
var _ Processor = ExcludeRules{}
type excludeRule struct { type excludeRule struct {
baseRule baseRule
} }
@ -17,43 +19,35 @@ type ExcludeRule struct {
} }
type ExcludeRules struct { type ExcludeRules struct {
rules []excludeRule name string
files *fsutils.Files
log logutils.Log log logutils.Log
files *fsutils.Files
rules []excludeRule
} }
func NewExcludeRules(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRules { type ExcludeRulesOptions struct {
r := &ExcludeRules{ Rules []ExcludeRule
CaseSensitive bool
}
func NewExcludeRules(log logutils.Log, files *fsutils.Files, opts ExcludeRulesOptions) *ExcludeRules {
p := &ExcludeRules{
name: "exclude-rules",
files: files, files: files,
log: log, log: log,
} }
r.rules = createRules(rules, "(?i)")
return r prefix := caseInsensitivePrefix
} if opts.CaseSensitive {
prefix = ""
p.name = "exclude-rules-case-sensitive"
}
func createRules(rules []ExcludeRule, prefix string) []excludeRule { p.rules = createRules(opts.Rules, prefix)
parsedRules := make([]excludeRule, 0, len(rules))
for _, rule := range rules { return p
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)
}
return parsedRules
} }
func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) { 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 }), nil
} }
func (ExcludeRules) Name() string { return "exclude-rules" } func (p ExcludeRules) Name() string { return p.name }
func (ExcludeRules) Finish() {} func (ExcludeRules) Finish() {}
var _ Processor = ExcludeRules{} func createRules(rules []ExcludeRule, prefix string) []excludeRule {
parsedRules := make([]excludeRule, 0, len(rules))
type ExcludeRulesCaseSensitive struct { for _, rule := range rules {
*ExcludeRules parsedRule := excludeRule{}
} parsedRule.linters = rule.Linters
func NewExcludeRulesCaseSensitive(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRulesCaseSensitive { if rule.Text != "" {
r := &ExcludeRules{ parsedRule.text = regexp.MustCompile(prefix + rule.Text)
files: files,
log: log,
} }
r.rules = createRules(rules, "")
return &ExcludeRulesCaseSensitive{r} 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)
}
return parsedRules
} }
func (ExcludeRulesCaseSensitive) Name() string { return "exclude-rules-case-sensitive" }
var _ Processor = ExcludeCaseSensitive{}

View File

@ -11,11 +11,11 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
func TestExcludeRulesMultiple(t *testing.T) { func TestExcludeRules_multiple(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "") files := fsutils.NewFiles(lineCache, "")
p := NewExcludeRules([]ExcludeRule{ opts := ExcludeRulesOptions{Rules: []ExcludeRule{
{ {
BaseRule: BaseRule{ BaseRule: BaseRule{
Text: "^exclude$", Text: "^exclude$",
@ -46,7 +46,9 @@ func TestExcludeRulesMultiple(t *testing.T) {
Linters: []string{"lll"}, Linters: []string{"lll"},
}, },
}, },
}, files, nil) }}
p := NewExcludeRules(nil, files, opts)
//nolint:dupl //nolint:dupl
cases := []issueTestCase{ cases := []issueTestCase{
@ -60,11 +62,14 @@ func TestExcludeRulesMultiple(t *testing.T) {
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"}, {Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"}, {Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
} }
var issues []result.Issue var issues []result.Issue
for _, c := range cases { for _, c := range cases {
issues = append(issues, newIssueFromIssueTestCase(c)) issues = append(issues, newIssueFromIssueTestCase(c))
} }
processedIssues := process(t, p, issues...) processedIssues := process(t, p, issues...)
var resultingCases []issueTestCase var resultingCases []issueTestCase
for _, i := range processedIssues { for _, i := range processedIssues {
resultingCases = append(resultingCases, issueTestCase{ resultingCases = append(resultingCases, issueTestCase{
@ -74,37 +79,46 @@ func TestExcludeRulesMultiple(t *testing.T) {
Line: i.Line(), Line: i.Line(),
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "e.go", Text: "some", Linter: "linter"}, {Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"}, {Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
{Path: "e_test.go", Text: "another", Linter: "linter"}, {Path: "e_test.go", Text: "another", Linter: "linter"},
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"}, {Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestExcludeRulesPathPrefix(t *testing.T) { func TestExcludeRules_pathPrefix(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
pathPrefix := path.Join("some", "dir") pathPrefix := path.Join("some", "dir")
files := fsutils.NewFiles(lineCache, pathPrefix) files := fsutils.NewFiles(lineCache, pathPrefix)
p := NewExcludeRules([]ExcludeRule{ opts := ExcludeRulesOptions{
Rules: []ExcludeRule{
{ {
BaseRule: BaseRule{ BaseRule: BaseRule{
Path: `some/dir/e\.go`, Path: `some/dir/e\.go`,
}, },
}, },
}, files, nil) },
}
p := NewExcludeRules(nil, files, opts)
cases := []issueTestCase{ cases := []issueTestCase{
{Path: "e.go"}, {Path: "e.go"},
{Path: "other.go"}, {Path: "other.go"},
} }
var issues []result.Issue var issues []result.Issue
for _, c := range cases { for _, c := range cases {
issues = append(issues, newIssueFromIssueTestCase(c)) issues = append(issues, newIssueFromIssueTestCase(c))
} }
processedIssues := process(t, p, issues...) processedIssues := process(t, p, issues...)
var resultingCases []issueTestCase var resultingCases []issueTestCase
for _, i := range processedIssues { for _, i := range processedIssues {
resultingCases = append(resultingCases, issueTestCase{ resultingCases = append(resultingCases, issueTestCase{
@ -114,21 +128,28 @@ func TestExcludeRulesPathPrefix(t *testing.T) {
Line: i.Line(), Line: i.Line(),
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "other.go"}, {Path: "other.go"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestExcludeRulesText(t *testing.T) { func TestExcludeRules_text(t *testing.T) {
p := NewExcludeRules([]ExcludeRule{ opts := ExcludeRulesOptions{
Rules: []ExcludeRule{
{ {
BaseRule: BaseRule{ BaseRule: BaseRule{
Text: "^exclude$", Text: "^exclude$",
Linters: []string{"linter"}, Linters: []string{"linter"},
}, },
}, },
}, nil, nil) },
}
p := NewExcludeRules(nil, nil, opts)
texts := []string{"excLude", "1", "", "exclud", "notexclude"} texts := []string{"excLude", "1", "", "exclud", "notexclude"}
var issues []result.Issue var issues []result.Issue
for _, t := range texts { for _, t := range texts {
@ -145,17 +166,21 @@ func TestExcludeRulesText(t *testing.T) {
for _, i := range processedIssues { for _, i := range processedIssues {
processedTexts = append(processedTexts, i.Text) processedTexts = append(processedTexts, i.Text)
} }
assert.Equal(t, texts[1:], processedTexts) assert.Equal(t, texts[1:], processedTexts)
} }
func TestExcludeRulesEmpty(t *testing.T) { func TestExcludeRules_empty(t *testing.T) {
processAssertSame(t, NewExcludeRules(nil, nil, nil), newIssueFromTextTestCase("test")) 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()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "") files := fsutils.NewFiles(lineCache, "")
p := NewExcludeRulesCaseSensitive([]ExcludeRule{
opts := ExcludeRulesOptions{
CaseSensitive: true,
Rules: []ExcludeRule{
{ {
BaseRule: BaseRule{ BaseRule: BaseRule{
Text: "^exclude$", Text: "^exclude$",
@ -180,7 +205,10 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Linters: []string{"lll"}, Linters: []string{"lll"},
}, },
}, },
}, files, nil) },
}
p := NewExcludeRules(nil, files, opts)
//nolint:dupl //nolint:dupl
cases := []issueTestCase{ cases := []issueTestCase{
@ -194,11 +222,14 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
{Path: "e_test.go", Text: "testOnly", Linter: "linter"}, {Path: "e_test.go", Text: "testOnly", Linter: "linter"},
{Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"}, {Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"},
} }
var issues []result.Issue var issues []result.Issue
for _, c := range cases { for _, c := range cases {
issues = append(issues, newIssueFromIssueTestCase(c)) issues = append(issues, newIssueFromIssueTestCase(c))
} }
processedIssues := process(t, p, issues...) processedIssues := process(t, p, issues...)
var resultingCases []issueTestCase var resultingCases []issueTestCase
for _, i := range processedIssues { for _, i := range processedIssues {
resultingCases = append(resultingCases, issueTestCase{ resultingCases = append(resultingCases, issueTestCase{
@ -208,6 +239,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Line: i.Line(), Line: i.Line(),
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "e.go", Text: "excLude", Linter: "linter"}, {Path: "e.go", Text: "excLude", Linter: "linter"},
{Path: "e.go", Text: "some", 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: "e_test.go", Text: "testOnly", Linter: "linter"},
{Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"}, {Path: filepath.Join("testdata", "exclude_rules_case_sensitive.go"), Line: 3, Linter: "lll"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestExcludeRulesCaseSensitiveText(t *testing.T) { func TestExcludeRules_caseSensitive_text(t *testing.T) {
p := NewExcludeRulesCaseSensitive([]ExcludeRule{ opts := ExcludeRulesOptions{
CaseSensitive: true,
Rules: []ExcludeRule{
{ {
BaseRule: BaseRule{ BaseRule: BaseRule{
Text: "^exclude$", Text: "^exclude$",
Linters: []string{"linter"}, Linters: []string{"linter"},
}, },
}, },
}, nil, nil) },
}
p := NewExcludeRules(nil, nil, opts)
texts := []string{"exclude", "excLude", "1", "", "exclud", "notexclude"} texts := []string{"exclude", "excLude", "1", "", "exclud", "notexclude"}
var issues []result.Issue var issues []result.Issue
for _, t := range texts { for _, t := range texts {
issues = append(issues, result.Issue{ issues = append(issues, result.Issue{
@ -244,9 +284,10 @@ func TestExcludeRulesCaseSensitiveText(t *testing.T) {
for _, i := range processedIssues { for _, i := range processedIssues {
processedTexts = append(processedTexts, i.Text) processedTexts = append(processedTexts, i.Text)
} }
assert.Equal(t, texts[1:], processedTexts) assert.Equal(t, texts[1:], processedTexts)
} }
func TestExcludeRulesCaseSensitiveEmpty(t *testing.T) { func TestExcludeRules_caseSensitive_empty(t *testing.T) {
processAssertSame(t, NewExcludeRulesCaseSensitive(nil, nil, nil), newIssueFromTextTestCase("test")) processAssertSame(t, NewExcludeRules(nil, nil, ExcludeRulesOptions{CaseSensitive: true}), newIssueFromTextTestCase("test"))
} }

View File

@ -9,8 +9,10 @@ import (
) )
func TestExclude(t *testing.T) { func TestExclude(t *testing.T) {
p := NewExclude("^exclude$") p := NewExclude(ExcludeOptions{Pattern: "^exclude$"})
texts := []string{"excLude", "1", "", "exclud", "notexclude"} texts := []string{"excLude", "1", "", "exclud", "notexclude"}
var issues []result.Issue var issues []result.Issue
for _, t := range texts { for _, t := range texts {
issues = append(issues, newIssueFromTextTestCase(t)) issues = append(issues, newIssueFromTextTestCase(t))
@ -23,16 +25,19 @@ func TestExclude(t *testing.T) {
for _, i := range processedIssues { for _, i := range processedIssues {
processedTexts = append(processedTexts, i.Text) processedTexts = append(processedTexts, i.Text)
} }
assert.Equal(t, texts[1:], processedTexts) assert.Equal(t, texts[1:], processedTexts)
} }
func TestNoExclude(t *testing.T) { func TestExclude_empty(t *testing.T) {
processAssertSame(t, NewExclude(""), newIssueFromTextTestCase("test")) processAssertSame(t, NewExclude(ExcludeOptions{}), newIssueFromTextTestCase("test"))
} }
func TestExcludeCaseSensitive(t *testing.T) { func TestExclude_caseSensitive(t *testing.T) {
p := NewExcludeCaseSensitive("^exclude$") p := NewExclude(ExcludeOptions{Pattern: "^exclude$", CaseSensitive: true})
texts := []string{"excLude", "1", "", "exclud", "exclude"} texts := []string{"excLude", "1", "", "exclud", "exclude"}
var issues []result.Issue var issues []result.Issue
for _, t := range texts { for _, t := range texts {
issues = append(issues, newIssueFromTextTestCase(t)) issues = append(issues, newIssueFromTextTestCase(t))
@ -45,5 +50,6 @@ func TestExcludeCaseSensitive(t *testing.T) {
for _, i := range processedIssues { for _, i := range processedIssues {
processedTexts = append(processedTexts, i.Text) processedTexts = append(processedTexts, i.Text)
} }
assert.Equal(t, texts[:len(texts)-1], processedTexts) assert.Equal(t, texts[:len(texts)-1], processedTexts)
} }

View File

@ -48,7 +48,7 @@ func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *
override: opts.Override, override: opts.Override,
} }
prefix := "(?i)" prefix := caseInsensitivePrefix
if opts.CaseSensitive { if opts.CaseSensitive {
prefix = "" prefix = ""
p.name = "severity-rules-case-sensitive" p.name = "severity-rules-case-sensitive"