Skip to content

Commit d7853ea

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 a9acb8d commit d7853ea

File tree

7 files changed

+124
-35
lines changed

7 files changed

+124
-35
lines changed

‎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
},
@@ -259,7 +263,7 @@ func getExcludeProcessor(cfg *config.Issues) processors.Processor {
259263
return excludeProcessor
260264
}
261265

262-
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
266+
func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsutils.Files) processors.Processor {
263267
var excludeRules []processors.ExcludeRule
264268
for _, r := range cfg.ExcludeRules {
265269
excludeRules = append(excludeRules, processors.ExcludeRule{
@@ -287,21 +291,21 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
287291
if cfg.ExcludeCaseSensitive {
288292
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
289293
excludeRules,
290-
lineCache,
294+
files,
291295
log.Child(logutils.DebugKeyExcludeRules),
292296
)
293297
} else {
294298
excludeRulesProcessor = processors.NewExcludeRules(
295299
excludeRules,
296-
lineCache,
300+
files,
297301
log.Child(logutils.DebugKeyExcludeRules),
298302
)
299303
}
300304

301305
return excludeRulesProcessor
302306
}
303307

304-
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
308+
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor {
305309
var severityRules []processors.SeverityRule
306310
for _, r := range cfg.Rules {
307311
severityRules = append(severityRules, processors.SeverityRule{
@@ -320,14 +324,14 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache
320324
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
321325
cfg.Default,
322326
severityRules,
323-
lineCache,
327+
files,
324328
log.Child(logutils.DebugKeySeverityRules),
325329
)
326330
} else {
327331
severityRulesProcessor = processors.NewSeverityRules(
328332
cfg.Default,
329333
severityRules,
330-
lineCache,
334+
files,
331335
log.Child(logutils.DebugKeySeverityRules),
332336
)
333337
}

‎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

‎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

‎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"},

‎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

‎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
}

‎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)