Skip to content

Commit

Permalink
WIP: consider path prefix when matching exclude rules
Browse files Browse the repository at this point in the history
When invoking or configuring golangci-lint with a path prefix,
the output contains files with that path. But the paths that
the exclude rules get compared against are the ones without
the additional prefix.

This makes it impossible to use a configuration file with non-trivial path
rules (i.e. anything that tries to match directory names) from different
directories because the rules will only match in one of them (typically the
root).
  • Loading branch information
pohly committed Feb 6, 2023
1 parent a9acb8d commit 9664d89
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 21 deletions.
6 changes: 4 additions & 2 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
processors.NewIdentifierMarker(),

getExcludeProcessor(&cfg.Issues),
getExcludeRulesProcessor(&cfg.Issues, log, lineCache),
getExcludeRulesProcessor(&cfg.Issues, log, cfg.Output.PathPrefix, lineCache),
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
Expand Down Expand Up @@ -259,7 +259,7 @@ func getExcludeProcessor(cfg *config.Issues) processors.Processor {
return excludeProcessor
}

func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, pathPrefix string, lineCache *fsutils.LineCache) processors.Processor {
var excludeRules []processors.ExcludeRule
for _, r := range cfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
Expand Down Expand Up @@ -287,12 +287,14 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
if cfg.ExcludeCaseSensitive {
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
excludeRules,
pathPrefix,
lineCache,
log.Child(logutils.DebugKeyExcludeRules),
)
} else {
excludeRulesProcessor = processors.NewExcludeRules(
excludeRules,
pathPrefix,
lineCache,
log.Child(logutils.DebugKeyExcludeRules),
)
Expand Down
5 changes: 3 additions & 2 deletions pkg/result/processors/base_rule.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package processors

import (
"path"
"regexp"

"github.com/golangci/golangci-lint/pkg/fsutils"
Expand All @@ -26,14 +27,14 @@ func (r *baseRule) isEmpty() bool {
return r.text == nil && r.source == nil && r.path == nil && len(r.linters) == 0
}

func (r *baseRule) match(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool {
func (r *baseRule) match(issue *result.Issue, pathPrefix string, lineCache *fsutils.LineCache, log logutils.Log) bool {
if r.isEmpty() {
return false
}
if r.text != nil && !r.text.MatchString(issue.Text) {
return false
}
if r.path != nil && !r.path.MatchString(issue.FilePath()) {
if r.path != nil && !r.path.MatchString(path.Join(pathPrefix, issue.FilePath())) {
return false
}
if len(r.linters) != 0 && !r.matchLinter(issue) {
Expand Down
23 changes: 13 additions & 10 deletions pkg/result/processors/exclude_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@ type ExcludeRule struct {
}

type ExcludeRules struct {
rules []excludeRule
lineCache *fsutils.LineCache
log logutils.Log
rules []excludeRule
lineCache *fsutils.LineCache
log logutils.Log
pathPrefix string
}

func NewExcludeRules(rules []ExcludeRule, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRules {
func NewExcludeRules(rules []ExcludeRule, pathPrefix string, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRules {
r := &ExcludeRules{
lineCache: lineCache,
log: log,
lineCache: lineCache,
log: log,
pathPrefix: pathPrefix,
}
r.rules = createRules(rules, "(?i)")

Expand Down Expand Up @@ -59,7 +61,7 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssues(issues, func(i *result.Issue) bool {
for _, rule := range p.rules {
rule := rule
if rule.match(i, p.lineCache, p.log) {
if rule.match(i, p.pathPrefix, p.lineCache, p.log) {
return false
}
}
Expand All @@ -76,10 +78,11 @@ type ExcludeRulesCaseSensitive struct {
*ExcludeRules
}

func NewExcludeRulesCaseSensitive(rules []ExcludeRule, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRulesCaseSensitive {
func NewExcludeRulesCaseSensitive(rules []ExcludeRule, pathPrefix string, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRulesCaseSensitive {
r := &ExcludeRules{
lineCache: lineCache,
log: log,
lineCache: lineCache,
log: log,
pathPrefix: pathPrefix,
}
r.rules = createRules(rules, "")

Expand Down
12 changes: 6 additions & 6 deletions pkg/result/processors/exclude_rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
Linters: []string{"lll"},
},
},
}, lineCache, nil)
}, "" /* path prefix */, lineCache, nil)

cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestExcludeRulesText(t *testing.T) {
Linters: []string{"linter"},
},
},
}, nil, nil)
}, "" /* path prefix */, nil, nil)
texts := []string{"excLude", "1", "", "exclud", "notexclude"}
var issues []result.Issue
for _, t := range texts {
Expand All @@ -99,7 +99,7 @@ func TestExcludeRulesText(t *testing.T) {
}

func TestExcludeRulesEmpty(t *testing.T) {
processAssertSame(t, NewExcludeRules(nil, nil, nil), newIssueFromTextTestCase("test"))
processAssertSame(t, NewExcludeRules(nil, "" /* path prefix */, nil, nil), newIssueFromTextTestCase("test"))
}

func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Expand Down Expand Up @@ -129,7 +129,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Linters: []string{"lll"},
},
},
}, lineCache, nil)
}, "" /* path prefix */, lineCache, nil)

cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
Expand Down Expand Up @@ -175,7 +175,7 @@ func TestExcludeRulesCaseSensitiveText(t *testing.T) {
Linters: []string{"linter"},
},
},
}, nil, nil)
}, "" /* path prefix */, nil, nil)
texts := []string{"exclude", "excLude", "1", "", "exclud", "notexclude"}
var issues []result.Issue
for _, t := range texts {
Expand All @@ -196,5 +196,5 @@ func TestExcludeRulesCaseSensitiveText(t *testing.T) {
}

func TestExcludeRulesCaseSensitiveEmpty(t *testing.T) {
processAssertSame(t, NewExcludeRulesCaseSensitive(nil, nil, nil), newIssueFromTextTestCase("test"))
processAssertSame(t, NewExcludeRulesCaseSensitive(nil, "" /* path prefix */, nil, nil), newIssueFromTextTestCase("test"))
}
2 changes: 1 addition & 1 deletion pkg/result/processors/severity_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) {
ruleSeverity = rule.severity
}

if rule.match(i, p.lineCache, p.log) {
if rule.match(i, "" /* TODO? path prefix */, p.lineCache, p.log) {
i.Severity = ruleSeverity
return i
}
Expand Down

0 comments on commit 9664d89

Please sign in to comment.