dev: simplify severity processor (#4451)

This commit is contained in:
Ludovic Fernandez 2024-03-04 17:33:36 +01:00 committed by GitHub
parent 8f2459bf0e
commit f81c3f26c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 202 additions and 149 deletions

View File

@ -328,22 +328,11 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs
}) })
} }
var severityRulesProcessor processors.Processor severityOpts := processors.SeverityOptions{
if cfg.CaseSensitive { Default: cfg.Default,
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive( Rules: severityRules,
cfg.Default, CaseSensitive: cfg.CaseSensitive,
severityRules,
files,
log.Child(logutils.DebugKeySeverityRules),
)
} else {
severityRulesProcessor = processors.NewSeverityRules(
cfg.Default,
severityRules,
files,
log.Child(logutils.DebugKeySeverityRules),
)
} }
return severityRulesProcessor return processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, severityOpts)
} }

View File

@ -8,6 +8,8 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
var _ Processor = &Severity{}
type severityRule struct { type severityRule struct {
baseRule baseRule
severity string severity string
@ -18,53 +20,47 @@ type SeverityRule struct {
Severity string 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 defaultSeverity string
rules []severityRule rules []severityRule
files *fsutils.Files
log logutils.Log
} }
func NewSeverityRules(defaultSeverity string, rules []SeverityRule, files *fsutils.Files, log logutils.Log) *SeverityRules { func NewSeverity(log logutils.Log, files *fsutils.Files, opts SeverityOptions) *Severity {
r := &SeverityRules{ p := &Severity{
name: "severity-rules",
files: files, files: files,
log: log, log: log,
defaultSeverity: defaultSeverity, defaultSeverity: opts.Default,
} }
r.rules = createSeverityRules(rules, "(?i)")
return r prefix := "(?i)"
if opts.CaseSensitive {
prefix = ""
p.name = "severity-rules-case-sensitive"
}
p.rules = createSeverityRules(opts.Rules, prefix)
return p
} }
func createSeverityRules(rules []SeverityRule, prefix string) []severityRule { func (p *Severity) Process(issues []result.Issue) ([]result.Issue, error) {
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)
}
return parsedRules
}
func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) {
if len(p.rules) == 0 && p.defaultSeverity == "" { if len(p.rules) == 0 && p.defaultSeverity == "" {
return issues, nil return issues, nil
} }
return transformIssues(issues, func(i *result.Issue) *result.Issue { return transformIssues(issues, func(i *result.Issue) *result.Issue {
for _, rule := range p.rules { for _, rule := range p.rules {
rule := rule rule := rule
@ -79,30 +75,45 @@ func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) {
return i return i
} }
} }
i.Severity = p.defaultSeverity i.Severity = p.defaultSeverity
return i return i
}), nil }), nil
} }
func (SeverityRules) Name() string { return "severity-rules" } func (p *Severity) Name() string { return p.name }
func (SeverityRules) Finish() {}
var _ Processor = SeverityRules{} func (*Severity) Finish() {}
type SeverityRulesCaseSensitive struct { func createSeverityRules(rules []SeverityRule, prefix string) []severityRule {
*SeverityRules parsedRules := make([]severityRule, 0, len(rules))
}
func NewSeverityRulesCaseSensitive(defaultSeverity string, rules []SeverityRule, for _, rule := range rules {
files *fsutils.Files, log logutils.Log) *SeverityRulesCaseSensitive { parsedRule := severityRule{}
r := &SeverityRules{ parsedRule.linters = rule.Linters
files: files, parsedRule.severity = rule.Severity
log: log,
defaultSeverity: defaultSeverity, 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" }

View File

@ -13,67 +13,73 @@ import (
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
func TestSeverityRulesMultiple(t *testing.T) { func TestSeverity_multiple(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "") files := fsutils.NewFiles(lineCache, "")
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{})
p := NewSeverityRules("error", []SeverityRule{
{ opts := SeverityOptions{
Severity: "info", Default: "error",
BaseRule: BaseRule{ Rules: []SeverityRule{
Text: "^ssl$", {
Linters: []string{"gosec"}, Severity: "info",
BaseRule: BaseRule{
Text: "^ssl$",
Linters: []string{"gosec"},
},
},
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"linter"},
Path: "e.go",
},
},
{
Severity: "info",
BaseRule: BaseRule{
Text: "^testonly$",
Path: `_test\.go`,
},
},
{
Severity: "info",
BaseRule: BaseRule{
Text: "^nontestonly$",
PathExcept: `_test\.go`,
},
},
{
BaseRule: BaseRule{
Source: "^//go:generate ",
Linters: []string{"lll"},
},
},
{
Severity: "info",
BaseRule: BaseRule{
Source: "^//go:dosomething",
},
},
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"someotherlinter"},
},
},
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"somelinter"},
},
},
{
Severity: "info",
}, },
}, },
{ }
Severity: "info",
BaseRule: BaseRule{ p := NewSeverity(log, files, opts)
Linters: []string{"linter"},
Path: "e.go",
},
},
{
Severity: "info",
BaseRule: BaseRule{
Text: "^testonly$",
Path: `_test\.go`,
},
},
{
Severity: "info",
BaseRule: BaseRule{
Text: "^nontestonly$",
PathExcept: `_test\.go`,
},
},
{
BaseRule: BaseRule{
Source: "^//go:generate ",
Linters: []string{"lll"},
},
},
{
Severity: "info",
BaseRule: BaseRule{
Source: "^//go:dosomething",
},
},
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"someotherlinter"},
},
},
{
Severity: "info",
BaseRule: BaseRule{
Linters: []string{"somelinter"},
},
},
{
Severity: "info",
},
}, files, log)
cases := []issueTestCase{ cases := []issueTestCase{
{Path: "ssl.go", Text: "ssl", Linter: "gosec"}, {Path: "ssl.go", Text: "ssl", Linter: "gosec"},
@ -87,11 +93,14 @@ func TestSeverityRulesMultiple(t *testing.T) {
{Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter"}, {Path: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter"},
{Path: "empty.go", Text: "empty", Linter: "empty"}, {Path: "empty.go", Text: "empty", Linter: "empty"},
} }
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{
@ -102,6 +111,7 @@ func TestSeverityRulesMultiple(t *testing.T) {
Severity: i.Severity, Severity: i.Severity,
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"}, {Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"},
{Path: "e.go", Text: "some", Linter: "linter", 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: "somenotmatchlinter.go", Text: "somenotmatchlinter", Linter: "somenotmatchlinter", Severity: "error"},
{Path: "empty.go", Text: "empty", Linter: "empty", Severity: "error"}, {Path: "empty.go", Text: "empty", Linter: "empty", Severity: "error"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestSeverityRulesPathPrefix(t *testing.T) { func TestSeverity_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)
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{})
p := NewSeverityRules("error", []SeverityRule{
{ opts := SeverityOptions{
Severity: "info", Default: "error",
BaseRule: BaseRule{ Rules: []SeverityRule{
Text: "some", {
Path: `some/dir/e\.go`, Severity: "info",
BaseRule: BaseRule{
Text: "some",
Path: `some/dir/e\.go`,
},
}, },
}, },
}, files, log) }
p := NewSeverity(log, files, opts)
cases := []issueTestCase{ cases := []issueTestCase{
{Path: "e.go", Text: "some", Linter: "linter"}, {Path: "e.go", Text: "some", Linter: "linter"},
{Path: "other.go", Text: "some", Linter: "linter"}, {Path: "other.go", Text: "some", Linter: "linter"},
} }
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{
@ -151,22 +171,29 @@ func TestSeverityRulesPathPrefix(t *testing.T) {
Severity: i.Severity, Severity: i.Severity,
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "e.go", Text: "some", Linter: "linter", Severity: "info"}, {Path: "e.go", Text: "some", Linter: "linter", Severity: "info"},
{Path: "other.go", Text: "some", Linter: "linter", Severity: "error"}, {Path: "other.go", Text: "some", Linter: "linter", Severity: "error"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestSeverityRulesText(t *testing.T) { func TestSeverity_text(t *testing.T) {
p := NewSeverityRules("", []SeverityRule{ opts := SeverityOptions{
{ Rules: []SeverityRule{
BaseRule: BaseRule{ {
Text: "^severity$", BaseRule: BaseRule{
Linters: []string{"linter"}, Text: "^severity$",
Linters: []string{"linter"},
},
}, },
}, },
}, nil, nil) }
p := NewSeverity(nil, nil, opts)
texts := []string{"seveRity", "1", "", "serverit", "notseverity"} texts := []string{"seveRity", "1", "", "serverit", "notseverity"}
var issues []result.Issue var issues []result.Issue
for _, t := range texts { for _, t := range texts {
@ -183,24 +210,34 @@ func TestSeverityRulesText(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, processedTexts) assert.Equal(t, texts, processedTexts)
} }
func TestSeverityRulesOnlyDefault(t *testing.T) { func TestSeverity_onlyDefault(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "") files := fsutils.NewFiles(lineCache, "")
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{}) 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{ cases := []issueTestCase{
{Path: "ssl.go", Text: "ssl", Linter: "gosec"}, {Path: "ssl.go", Text: "ssl", Linter: "gosec"},
{Path: "empty.go", Text: "empty", Linter: "empty"}, {Path: "empty.go", Text: "empty", Linter: "empty"},
} }
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{
@ -211,38 +248,52 @@ func TestSeverityRulesOnlyDefault(t *testing.T) {
Severity: i.Severity, Severity: i.Severity,
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"}, {Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"},
{Path: "empty.go", Text: "empty", Linter: "empty", Severity: "info"}, {Path: "empty.go", Text: "empty", Linter: "empty", Severity: "info"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }
func TestSeverityRulesEmpty(t *testing.T) { func TestSeverity_empty(t *testing.T) {
processAssertSame(t, NewSeverityRules("", nil, nil, nil), newIssueFromTextTestCase("test")) 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()) lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "") files := fsutils.NewFiles(lineCache, "")
p := NewSeverityRulesCaseSensitive("error", []SeverityRule{
{ opts := SeverityOptions{
Severity: "info", Default: "error",
BaseRule: BaseRule{ Rules: []SeverityRule{
Text: "^ssl$", {
Linters: []string{"gosec", "someotherlinter"}, Severity: "info",
BaseRule: BaseRule{
Text: "^ssl$",
Linters: []string{"gosec", "someotherlinter"},
},
}, },
}, },
}, files, nil) CaseSensitive: true,
}
p := NewSeverity(nil, files, opts)
cases := []issueTestCase{ cases := []issueTestCase{
{Path: "e.go", Text: "ssL", Linter: "gosec"}, {Path: "e.go", Text: "ssL", Linter: "gosec"},
} }
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{
@ -253,8 +304,10 @@ func TestSeverityRulesCaseSensitive(t *testing.T) {
Severity: i.Severity, Severity: i.Severity,
}) })
} }
expectedCases := []issueTestCase{ expectedCases := []issueTestCase{
{Path: "e.go", Text: "ssL", Linter: "gosec", Severity: "error"}, {Path: "e.go", Text: "ssL", Linter: "gosec", Severity: "error"},
} }
assert.Equal(t, expectedCases, resultingCases) assert.Equal(t, expectedCases, resultingCases)
} }