Skip to content

Commit faf5560

Browse files
pohlypohly
committed
exclude+severity rules: fix path matching when path prefix is used
When someone invokes golangci-lint inside a project, they can set the path prefix to make the output look like the invocation had been done in the root. But this prefix was ignored when checking path patters of exclude and severity rules, so those only worked when golangci-lint was invoked in the same (root?) directory. The underlying problem is that the rules from the configuration get checked before updating the path associated with the issues in the path fixer. There's probably a reason for doing it in this order, so to make the prefix work, it now gets passed down into rule processors and added there to the issue path before checking against the path pattern. This could have been done through a separate parameter, but bundling it in a new fsutils helper struct made the change a bit smaller.
1 parent 6437429 commit faf5560

File tree

7 files changed

+124
-35
lines changed

7 files changed

+124
-35
lines changed

Diff for: ‎pkg/lint/runner.go

+12-8
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
6565
}
6666
}
6767

68+
// Some processors need to add the path prefix when working with issue
69+
// paths because they get invoked before the path prefixer.
70+
files := fsutils.NewFiles(lineCache, cfg.Output.PathPrefix)
71+
6872
return &Runner{
6973
Processors: []processors.Processor{
7074
processors.NewCgo(goenv),
@@ -83,7 +87,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
8387
processors.NewIdentifierMarker(),
8488

8589
getExcludeProcessor(&cfg.Issues),
86-
getExcludeRulesProcessor(&cfg.Issues, log, lineCache),
90+
getExcludeRulesProcessor(&cfg.Issues, log, files),
8791
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),
8892

8993
processors.NewUniqByLine(cfg),
@@ -93,7 +97,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
9397
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg),
9498
processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
9599
processors.NewPathShortener(),
96-
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
100+
getSeverityRulesProcessor(&cfg.Severity, log, files),
97101
processors.NewPathPrefixer(cfg.Output.PathPrefix),
98102
processors.NewSortResults(cfg),
99103
},
@@ -260,7 +264,7 @@ func getExcludeProcessor(cfg *config.Issues) processors.Processor {
260264
return excludeProcessor
261265
}
262266

263-
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
267+
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsutils.Files) processors.Processor {
264268
var excludeRules []processors.ExcludeRule
265269
for _, r := range cfg.ExcludeRules {
266270
excludeRules = append(excludeRules, processors.ExcludeRule{
@@ -288,21 +292,21 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
288292
if cfg.ExcludeCaseSensitive {
289293
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
290294
excludeRules,
291-
lineCache,
295+
files,
292296
log.Child(logutils.DebugKeyExcludeRules),
293297
)
294298
} else {
295299
excludeRulesProcessor = processors.NewExcludeRules(
296300
excludeRules,
297-
lineCache,
301+
files,
298302
log.Child(logutils.DebugKeyExcludeRules),
299303
)
300304
}
301305

302306
return excludeRulesProcessor
303307
}
304308

305-
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
309+
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor {
306310
var severityRules []processors.SeverityRule
307311
for _, r := range cfg.Rules {
308312
severityRules = append(severityRules, processors.SeverityRule{
@@ -321,14 +325,14 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache
321325
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
322326
cfg.Default,
323327
severityRules,
324-
lineCache,
328+
files,
325329
log.Child(logutils.DebugKeySeverityRules),
326330
)
327331
} else {
328332
severityRulesProcessor = processors.NewSeverityRules(
329333
cfg.Default,
330334
severityRules,
331-
lineCache,
335+
files,
332336
log.Child(logutils.DebugKeySeverityRules),
333337
)
334338
}

Diff for: ‎pkg/result/processors/base_rule.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ func (r *baseRule) isEmpty() bool {
2626
return r.text == nil && r.source == nil && r.path == nil && len(r.linters) == 0
2727
}
2828

29-
func (r *baseRule) match(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool {
29+
func (r *baseRule) match(issue *result.Issue, files *fsutils.Files, log logutils.Log) bool {
3030
if r.isEmpty() {
3131
return false
3232
}
3333
if r.text != nil && !r.text.MatchString(issue.Text) {
3434
return false
3535
}
36-
if r.path != nil && !r.path.MatchString(issue.FilePath()) {
36+
if r.path != nil && !r.path.MatchString(files.WithPathPrefix(issue.FilePath())) {
3737
return false
3838
}
3939
if len(r.linters) != 0 && !r.matchLinter(issue) {
4040
return false
4141
}
4242

4343
// the most heavyweight checking last
44-
if r.source != nil && !r.matchSource(issue, lineCache, log) {
44+
if r.source != nil && !r.matchSource(issue, files.LineCache, log) {
4545
return false
4646
}
4747

Diff for: ‎pkg/result/processors/exclude_rules.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ type ExcludeRule struct {
1717
}
1818

1919
type ExcludeRules struct {
20-
rules []excludeRule
21-
lineCache *fsutils.LineCache
22-
log logutils.Log
20+
rules []excludeRule
21+
files *fsutils.Files
22+
log logutils.Log
2323
}
2424

25-
func NewExcludeRules(rules []ExcludeRule, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRules {
25+
func NewExcludeRules(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRules {
2626
r := &ExcludeRules{
27-
lineCache: lineCache,
28-
log: log,
27+
files: files,
28+
log: log,
2929
}
3030
r.rules = createRules(rules, "(?i)")
3131

@@ -59,7 +59,7 @@ func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) {
5959
return filterIssues(issues, func(i *result.Issue) bool {
6060
for _, rule := range p.rules {
6161
rule := rule
62-
if rule.match(i, p.lineCache, p.log) {
62+
if rule.match(i, p.files, p.log) {
6363
return false
6464
}
6565
}
@@ -76,10 +76,10 @@ type ExcludeRulesCaseSensitive struct {
7676
*ExcludeRules
7777
}
7878

79-
func NewExcludeRulesCaseSensitive(rules []ExcludeRule, lineCache *fsutils.LineCache, log logutils.Log) *ExcludeRulesCaseSensitive {
79+
func NewExcludeRulesCaseSensitive(rules []ExcludeRule, files *fsutils.Files, log logutils.Log) *ExcludeRulesCaseSensitive {
8080
r := &ExcludeRules{
81-
lineCache: lineCache,
82-
log: log,
81+
files: files,
82+
log: log,
8383
}
8484
r.rules = createRules(rules, "")
8585

Diff for: ‎pkg/result/processors/exclude_rules_test.go

+43-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package processors
22

33
import (
4+
"path"
45
"path/filepath"
56
"testing"
67

@@ -12,6 +13,8 @@ import (
1213

1314
func TestExcludeRulesMultiple(t *testing.T) {
1415
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
16+
files := fsutils.NewFiles(lineCache, "")
17+
1518
p := NewExcludeRules([]ExcludeRule{
1619
{
1720
BaseRule: BaseRule{
@@ -37,7 +40,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
3740
Linters: []string{"lll"},
3841
},
3942
},
40-
}, lineCache, nil)
43+
}, files, nil)
4144

4245
cases := []issueTestCase{
4346
{Path: "e.go", Text: "exclude", Linter: "linter"},
@@ -70,6 +73,43 @@ func TestExcludeRulesMultiple(t *testing.T) {
7073
assert.Equal(t, expectedCases, resultingCases)
7174
}
7275

76+
func TestExcludeRulesPathPrefix(t *testing.T) {
77+
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
78+
pathPrefix := path.Join("some", "dir")
79+
files := fsutils.NewFiles(lineCache, pathPrefix)
80+
81+
p := NewExcludeRules([]ExcludeRule{
82+
{
83+
BaseRule: BaseRule{
84+
Path: `some/dir/e\.go`,
85+
},
86+
},
87+
}, files, nil)
88+
89+
cases := []issueTestCase{
90+
{Path: "e.go"},
91+
{Path: "other.go"},
92+
}
93+
var issues []result.Issue
94+
for _, c := range cases {
95+
issues = append(issues, newIssueFromIssueTestCase(c))
96+
}
97+
processedIssues := process(t, p, issues...)
98+
var resultingCases []issueTestCase
99+
for _, i := range processedIssues {
100+
resultingCases = append(resultingCases, issueTestCase{
101+
Path: i.FilePath(),
102+
Linter: i.FromLinter,
103+
Text: i.Text,
104+
Line: i.Line(),
105+
})
106+
}
107+
expectedCases := []issueTestCase{
108+
{Path: "other.go"},
109+
}
110+
assert.Equal(t, expectedCases, resultingCases)
111+
}
112+
73113
func TestExcludeRulesText(t *testing.T) {
74114
p := NewExcludeRules([]ExcludeRule{
75115
{
@@ -104,6 +144,7 @@ func TestExcludeRulesEmpty(t *testing.T) {
104144

105145
func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
106146
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
147+
files := fsutils.NewFiles(lineCache, "")
107148
p := NewExcludeRulesCaseSensitive([]ExcludeRule{
108149
{
109150
BaseRule: BaseRule{
@@ -129,7 +170,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
129170
Linters: []string{"lll"},
130171
},
131172
},
132-
}, lineCache, nil)
173+
}, files, nil)
133174

134175
cases := []issueTestCase{
135176
{Path: "e.go", Text: "exclude", Linter: "linter"},

Diff for: ‎pkg/result/processors/path_prefixer.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
package processors
22

33
import (
4-
"path/filepath"
5-
4+
"github.com/golangci/golangci-lint/pkg/fsutils"
65
"github.com/golangci/golangci-lint/pkg/result"
76
)
87

@@ -27,7 +26,7 @@ func (*PathPrefixer) Name() string {
2726
func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) {
2827
if p.prefix != "" {
2928
for i := range issues {
30-
issues[i].Pos.Filename = filepath.Join(p.prefix, issues[i].Pos.Filename)
29+
issues[i].Pos.Filename = fsutils.WithPathPrefix(p.prefix, issues[i].Pos.Filename)
3130
}
3231
}
3332
return issues, nil

Diff for: ‎pkg/result/processors/severity_rules.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ type SeverityRule struct {
2121
type SeverityRules struct {
2222
defaultSeverity string
2323
rules []severityRule
24-
lineCache *fsutils.LineCache
24+
files *fsutils.Files
2525
log logutils.Log
2626
}
2727

28-
func NewSeverityRules(defaultSeverity string, rules []SeverityRule, lineCache *fsutils.LineCache, log logutils.Log) *SeverityRules {
28+
func NewSeverityRules(defaultSeverity string, rules []SeverityRule, files *fsutils.Files, log logutils.Log) *SeverityRules {
2929
r := &SeverityRules{
30-
lineCache: lineCache,
30+
files: files,
3131
log: log,
3232
defaultSeverity: defaultSeverity,
3333
}
@@ -70,7 +70,7 @@ func (p SeverityRules) Process(issues []result.Issue) ([]result.Issue, error) {
7070
ruleSeverity = rule.severity
7171
}
7272

73-
if rule.match(i, p.lineCache, p.log) {
73+
if rule.match(i, p.files, p.log) {
7474
i.Severity = ruleSeverity
7575
return i
7676
}
@@ -90,9 +90,9 @@ type SeverityRulesCaseSensitive struct {
9090
}
9191

9292
func NewSeverityRulesCaseSensitive(defaultSeverity string, rules []SeverityRule,
93-
lineCache *fsutils.LineCache, log logutils.Log) *SeverityRulesCaseSensitive {
93+
files *fsutils.Files, log logutils.Log) *SeverityRulesCaseSensitive {
9494
r := &SeverityRules{
95-
lineCache: lineCache,
95+
files: files,
9696
log: log,
9797
defaultSeverity: defaultSeverity,
9898
}

Diff for: ‎pkg/result/processors/severity_rules_test.go

+48-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package processors
22

33
import (
4+
"path"
45
"path/filepath"
56
"testing"
67

@@ -14,6 +15,7 @@ import (
1415

1516
func TestSeverityRulesMultiple(t *testing.T) {
1617
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
18+
files := fsutils.NewFiles(lineCache, "")
1719
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{})
1820
p := NewSeverityRules("error", []SeverityRule{
1921
{
@@ -64,7 +66,7 @@ func TestSeverityRulesMultiple(t *testing.T) {
6466
{
6567
Severity: "info",
6668
},
67-
}, lineCache, log)
69+
}, files, log)
6870

6971
cases := []issueTestCase{
7072
{Path: "ssl.go", Text: "ssl", Linter: "gosec"},
@@ -104,6 +106,47 @@ func TestSeverityRulesMultiple(t *testing.T) {
104106
assert.Equal(t, expectedCases, resultingCases)
105107
}
106108

109+
func TestSeverityRulesPathPrefix(t *testing.T) {
110+
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
111+
pathPrefix := path.Join("some", "dir")
112+
files := fsutils.NewFiles(lineCache, pathPrefix)
113+
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{})
114+
p := NewSeverityRules("error", []SeverityRule{
115+
{
116+
Severity: "info",
117+
BaseRule: BaseRule{
118+
Text: "some",
119+
Path: `some/dir/e\.go`,
120+
},
121+
},
122+
}, files, log)
123+
124+
cases := []issueTestCase{
125+
{Path: "e.go", Text: "some", Linter: "linter"},
126+
{Path: "other.go", Text: "some", Linter: "linter"},
127+
}
128+
var issues []result.Issue
129+
for _, c := range cases {
130+
issues = append(issues, newIssueFromIssueTestCase(c))
131+
}
132+
processedIssues := process(t, p, issues...)
133+
var resultingCases []issueTestCase
134+
for _, i := range processedIssues {
135+
resultingCases = append(resultingCases, issueTestCase{
136+
Path: i.FilePath(),
137+
Linter: i.FromLinter,
138+
Text: i.Text,
139+
Line: i.Line(),
140+
Severity: i.Severity,
141+
})
142+
}
143+
expectedCases := []issueTestCase{
144+
{Path: "e.go", Text: "some", Linter: "linter", Severity: "info"},
145+
{Path: "other.go", Text: "some", Linter: "linter", Severity: "error"},
146+
}
147+
assert.Equal(t, expectedCases, resultingCases)
148+
}
149+
107150
func TestSeverityRulesText(t *testing.T) {
108151
p := NewSeverityRules("", []SeverityRule{
109152
{
@@ -134,8 +177,9 @@ func TestSeverityRulesText(t *testing.T) {
134177

135178
func TestSeverityRulesOnlyDefault(t *testing.T) {
136179
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
180+
files := fsutils.NewFiles(lineCache, "")
137181
log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &report.Data{})
138-
p := NewSeverityRules("info", []SeverityRule{}, lineCache, log)
182+
p := NewSeverityRules("info", []SeverityRule{}, files, log)
139183

140184
cases := []issueTestCase{
141185
{Path: "ssl.go", Text: "ssl", Linter: "gosec"},
@@ -169,6 +213,7 @@ func TestSeverityRulesEmpty(t *testing.T) {
169213

170214
func TestSeverityRulesCaseSensitive(t *testing.T) {
171215
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
216+
files := fsutils.NewFiles(lineCache, "")
172217
p := NewSeverityRulesCaseSensitive("error", []SeverityRule{
173218
{
174219
Severity: "info",
@@ -177,7 +222,7 @@ func TestSeverityRulesCaseSensitive(t *testing.T) {
177222
Linters: []string{"gosec", "someotherlinter"},
178223
},
179224
},
180-
}, lineCache, nil)
225+
}, files, nil)
181226

182227
cases := []issueTestCase{
183228
{Path: "e.go", Text: "ssL", Linter: "gosec"},

0 commit comments

Comments
 (0)