Skip to content

Commit

Permalink
dev: removes BaseRule, ExcludeRule, SeverityRule duplications (#4676)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ldez
ldez authored Apr 27, 2024
1 parent b2df2f4 commit 55b2f5d
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 189 deletions.
75 changes: 3 additions & 72 deletions pkg/lint/runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti
// Must be before exclude because users see already marked output and configure excluding by it.
processors.NewIdentifierMarker(),

getExcludeProcessor(&cfg.Issues),
getExcludeRulesProcessor(&cfg.Issues, log, files),
processors.NewExclude(&cfg.Issues),
processors.NewExcludeRules(log.Child(logutils.DebugKeyExcludeRules), files, &cfg.Issues),
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
Expand All @@ -91,7 +91,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg),
processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
processors.NewPathShortener(),
getSeverityRulesProcessor(&cfg.Severity, log, files),
processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, &cfg.Severity),

// The fixer still needs to see paths for the issues that are relative to the current directory.
processors.NewFixer(cfg, log, fileCache),
Expand Down Expand Up @@ -242,72 +242,3 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, s

return issues
}

func getExcludeProcessor(cfg *config.Issues) processors.Processor {
opts := processors.ExcludeOptions{
CaseSensitive: cfg.ExcludeCaseSensitive,
}

if len(cfg.ExcludePatterns) != 0 {
opts.Pattern = fmt.Sprintf("(%s)", strings.Join(cfg.ExcludePatterns, "|"))
}

return processors.NewExclude(opts)
}

func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsutils.Files) processors.Processor {
var excludeRules []processors.ExcludeRule
for _, r := range cfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
BaseRule: processors.BaseRule{
Text: r.Text,
Source: r.Source,
Path: r.Path,
PathExcept: r.PathExcept,
Linters: r.Linters,
},
})
}

if cfg.UseDefaultExcludes {
for _, r := range config.GetExcludePatterns(cfg.IncludeDefaultExcludes) {
excludeRules = append(excludeRules, processors.ExcludeRule{
BaseRule: processors.BaseRule{
Text: r.Pattern,
Linters: []string{r.Linter},
},
})
}
}

opts := processors.ExcludeRulesOptions{
Rules: excludeRules,
CaseSensitive: cfg.ExcludeCaseSensitive,
}

return processors.NewExcludeRules(log.Child(logutils.DebugKeyExcludeRules), files, opts)
}

func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor {
var severityRules []processors.SeverityRule
for _, r := range cfg.Rules {
severityRules = append(severityRules, processors.SeverityRule{
Severity: r.Severity,
BaseRule: processors.BaseRule{
Text: r.Text,
Source: r.Source,
Path: r.Path,
PathExcept: r.PathExcept,
Linters: r.Linters,
},
})
}

severityOpts := processors.SeverityOptions{
Default: cfg.Default,
Rules: severityRules,
CaseSensitive: cfg.CaseSensitive,
}

return processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, severityOpts)
}
8 changes: 0 additions & 8 deletions pkg/result/processors/base_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ import (

const caseInsensitivePrefix = "(?i)"

type BaseRule struct {
Text string
Source string
Path string
PathExcept string
Linters []string
}

type baseRule struct {
text *regexp.Regexp
source *regexp.Regexp
Expand Down
21 changes: 12 additions & 9 deletions pkg/result/processors/exclude.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package processors

import (
"fmt"
"regexp"
"strings"

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/result"
)

Expand All @@ -14,22 +17,22 @@ type Exclude struct {
pattern *regexp.Regexp
}

type ExcludeOptions struct {
Pattern string
CaseSensitive bool
}

func NewExclude(opts ExcludeOptions) *Exclude {
func NewExclude(cfg *config.Issues) *Exclude {
p := &Exclude{name: "exclude"}

var pattern string
if len(cfg.ExcludePatterns) != 0 {
pattern = fmt.Sprintf("(%s)", strings.Join(cfg.ExcludePatterns, "|"))
}

prefix := caseInsensitivePrefix
if opts.CaseSensitive {
if cfg.ExcludeCaseSensitive {
p.name = "exclude-case-sensitive"
prefix = ""
}

if opts.Pattern != "" {
p.pattern = regexp.MustCompile(prefix + opts.Pattern)
if pattern != "" {
p.pattern = regexp.MustCompile(prefix + pattern)
}

return p
Expand Down
31 changes: 18 additions & 13 deletions pkg/result/processors/exclude_rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package processors
import (
"regexp"

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
Expand All @@ -14,10 +15,6 @@ type excludeRule struct {
baseRule
}

type ExcludeRule struct {
BaseRule
}

type ExcludeRules struct {
name string

Expand All @@ -27,25 +24,33 @@ type ExcludeRules struct {
rules []excludeRule
}

type ExcludeRulesOptions struct {
Rules []ExcludeRule
CaseSensitive bool
}

func NewExcludeRules(log logutils.Log, files *fsutils.Files, opts ExcludeRulesOptions) *ExcludeRules {
func NewExcludeRules(log logutils.Log, files *fsutils.Files, cfg *config.Issues) *ExcludeRules {
p := &ExcludeRules{
name: "exclude-rules",
files: files,
log: log,
}

prefix := caseInsensitivePrefix
if opts.CaseSensitive {
if cfg.ExcludeCaseSensitive {
prefix = ""
p.name = "exclude-rules-case-sensitive"
}

p.rules = createRules(opts.Rules, prefix)
excludeRules := cfg.ExcludeRules

if cfg.UseDefaultExcludes {
for _, r := range config.GetExcludePatterns(cfg.IncludeDefaultExcludes) {
excludeRules = append(excludeRules, config.ExcludeRule{
BaseRule: config.BaseRule{
Text: r.Pattern,
Linters: []string{r.Linter},
},
})
}
}

p.rules = createRules(excludeRules, prefix)

return p
}
Expand All @@ -71,7 +76,7 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) {

func (ExcludeRules) Finish() {}

func createRules(rules []ExcludeRule, prefix string) []excludeRule {
func createRules(rules []config.ExcludeRule, prefix string) []excludeRule {
parsedRules := make([]excludeRule, 0, len(rules))

for _, rule := range rules {
Expand Down
51 changes: 26 additions & 25 deletions pkg/result/processors/exclude_rules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/assert"

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/result"
)
Expand All @@ -15,33 +16,33 @@ func TestExcludeRules_multiple(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "")

opts := ExcludeRulesOptions{Rules: []ExcludeRule{
opts := &config.Issues{ExcludeRules: []config.ExcludeRule{
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^exclude$",
Linters: []string{"linter"},
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Linters: []string{"testlinter"},
Path: `_test\.go`,
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^testonly$",
Path: `_test\.go`,
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^nontestonly$",
PathExcept: `_test\.go`,
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Source: "^//go:generate ",
Linters: []string{"lll"},
},
Expand Down Expand Up @@ -94,10 +95,10 @@ func TestExcludeRules_pathPrefix(t *testing.T) {
pathPrefix := path.Join("some", "dir")
files := fsutils.NewFiles(lineCache, pathPrefix)

opts := ExcludeRulesOptions{
Rules: []ExcludeRule{
opts := &config.Issues{
ExcludeRules: []config.ExcludeRule{
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Path: `some/dir/e\.go`,
},
},
Expand Down Expand Up @@ -136,10 +137,10 @@ func TestExcludeRules_pathPrefix(t *testing.T) {
}

func TestExcludeRules_text(t *testing.T) {
opts := ExcludeRulesOptions{
Rules: []ExcludeRule{
opts := &config.Issues{
ExcludeRules: []config.ExcludeRule{
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^exclude$",
Linters: []string{"linter"},
},
Expand Down Expand Up @@ -170,36 +171,36 @@ func TestExcludeRules_text(t *testing.T) {
}

func TestExcludeRules_empty(t *testing.T) {
processAssertSame(t, NewExcludeRules(nil, nil, ExcludeRulesOptions{}), newIssueFromTextTestCase("test"))
processAssertSame(t, NewExcludeRules(nil, nil, &config.Issues{}), newIssueFromTextTestCase("test"))
}

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

opts := ExcludeRulesOptions{
CaseSensitive: true,
Rules: []ExcludeRule{
opts := &config.Issues{
ExcludeCaseSensitive: true,
ExcludeRules: []config.ExcludeRule{
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^exclude$",
Linters: []string{"linter"},
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Linters: []string{"testlinter"},
Path: `_test\.go`,
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^testonly$",
Path: `_test\.go`,
},
},
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Source: "^//go:generate ",
Linters: []string{"lll"},
},
Expand Down Expand Up @@ -251,11 +252,11 @@ func TestExcludeRules_caseSensitive_multiple(t *testing.T) {
}

func TestExcludeRules_caseSensitive_text(t *testing.T) {
opts := ExcludeRulesOptions{
CaseSensitive: true,
Rules: []ExcludeRule{
opts := &config.Issues{
ExcludeCaseSensitive: true,
ExcludeRules: []config.ExcludeRule{
{
BaseRule: BaseRule{
BaseRule: config.BaseRule{
Text: "^exclude$",
Linters: []string{"linter"},
},
Expand Down Expand Up @@ -287,5 +288,5 @@ func TestExcludeRules_caseSensitive_text(t *testing.T) {
}

func TestExcludeRules_caseSensitive_empty(t *testing.T) {
processAssertSame(t, NewExcludeRules(nil, nil, ExcludeRulesOptions{CaseSensitive: true}), newIssueFromTextTestCase("test"))
processAssertSame(t, NewExcludeRules(nil, nil, &config.Issues{ExcludeCaseSensitive: true}), newIssueFromTextTestCase("test"))
}
7 changes: 4 additions & 3 deletions pkg/result/processors/exclude_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (

"github.com/stretchr/testify/assert"

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/result"
)

func TestExclude(t *testing.T) {
p := NewExclude(ExcludeOptions{Pattern: "^exclude$"})
p := NewExclude(&config.Issues{ExcludePatterns: []string{"^exclude$"}})

texts := []string{"excLude", "1", "", "exclud", "notexclude"}

Expand All @@ -30,11 +31,11 @@ func TestExclude(t *testing.T) {
}

func TestExclude_empty(t *testing.T) {
processAssertSame(t, NewExclude(ExcludeOptions{}), newIssueFromTextTestCase("test"))
processAssertSame(t, NewExclude(&config.Issues{}), newIssueFromTextTestCase("test"))
}

func TestExclude_caseSensitive(t *testing.T) {
p := NewExclude(ExcludeOptions{Pattern: "^exclude$", CaseSensitive: true})
p := NewExclude(&config.Issues{ExcludePatterns: []string{"^exclude$"}, ExcludeCaseSensitive: true})

texts := []string{"excLude", "1", "", "exclud", "exclude"}

Expand Down
Loading

0 comments on commit 55b2f5d

Please sign in to comment.