Skip to content

Commit 3107130

Browse files
committed
config: fix path matching when path prefix is used
When someone invokes golangci-lint in a sub-directory, 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 patterns of exclude, severity, and skip files/dirs entries, so those with matching for directories only worked when golangci-lint was always invoked in the same 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. To make the prefix work, it now gets passed down into processors and added there to the issue path before checking against the path pattern. For exclude and severity rules, 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 f0dbc75 commit 3107130

13 files changed

+185
-47
lines changed

Diff for: .golangci.reference.yml

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ run:
2727
- mytag
2828

2929
# Which dirs to skip: issues from them won't be reported.
30-
# Can use regexp here: `generated.*`, regexp is applied on full path.
30+
# Can use regexp here: `generated.*`, regexp is applied on full path,
31+
# including the path prefix if one is set.
3132
# Default value is empty list,
3233
# but default dirs are skipped independently of this option's value (see skip-dirs-use-default).
3334
# "/" will be replaced by current OS file path separator to properly work on Windows.

Diff for: docs/src/docs/usage/false-positives.mdx

+5
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ issues:
6666

6767
Exclude issues in path by `run.skip-dirs`, `run.skip-files` or `issues.exclude-rules` config options.
6868

69+
Beware that the paths that get matched here are relative to the current working
70+
directory. When the configuration contains path patterns that check for specific
71+
directories, the `--path-prefix` parameter can be used to extend the paths
72+
before matching.
73+
6974
In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded:
7075

7176
```yml

Diff for: pkg/fsutils/files.go

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package fsutils
2+
3+
import "path/filepath"
4+
5+
// Files combines different operations related to handling file paths and
6+
// content.
7+
type Files struct {
8+
*LineCache
9+
pathPrefix string
10+
}
11+
12+
func NewFiles(lc *LineCache, pathPrefix string) *Files {
13+
return &Files{
14+
LineCache: lc,
15+
pathPrefix: pathPrefix,
16+
}
17+
}
18+
19+
// WithPathPrefix takes a path that is relative to the current directory (as
20+
// used in in issues) and adds the configured path prefix, if there is one. The
21+
// resulting path then can be shown to the user or compared against paths
22+
// specified in the configuration.
23+
func (f *Files) WithPathPrefix(relativePath string) string {
24+
return WithPathPrefix(f.pathPrefix, relativePath)
25+
}
26+
27+
// WithPathPrefix takes a path that is relative to the current directory (as
28+
// used in in issues) and adds the configured path prefix, if there is one. The
29+
// resulting path then can be shown to the user or compared against paths
30+
// specified in the configuration.
31+
func WithPathPrefix(pathPrefix, relativePath string) string {
32+
if pathPrefix == "" {
33+
return relativePath
34+
}
35+
return filepath.Join(pathPrefix, relativePath)
36+
}

Diff for: pkg/lint/runner.go

+15-10
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ type Runner struct {
2929

3030
func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lintersdb.EnabledSet,
3131
lineCache *fsutils.LineCache, dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
32-
skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles)
32+
// Beware that some processors need to add the path prefix when working
33+
// with paths because they get invoked before the path prefixer
34+
// (exclude and severity rules) or process other paths (skip files).
35+
files := fsutils.NewFiles(lineCache, cfg.Output.PathPrefix)
36+
37+
skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles, cfg.Output.PathPrefix)
3338
if err != nil {
3439
return nil, err
3540
}
@@ -38,7 +43,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
3843
if cfg.Run.UseDefaultSkipDirs {
3944
skipDirs = append(skipDirs, packages.StdExcludeDirRegexps...)
4045
}
41-
skipDirsProcessor, err := processors.NewSkipDirs(skipDirs, log.Child(logutils.DebugKeySkipDirs), cfg.Run.Args)
46+
skipDirsProcessor, err := processors.NewSkipDirs(skipDirs, log.Child(logutils.DebugKeySkipDirs), cfg.Run.Args, cfg.Output.PathPrefix)
4247
if err != nil {
4348
return nil, err
4449
}
@@ -82,7 +87,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
8287
processors.NewIdentifierMarker(),
8388

8489
getExcludeProcessor(&cfg.Issues),
85-
getExcludeRulesProcessor(&cfg.Issues, log, lineCache),
90+
getExcludeRulesProcessor(&cfg.Issues, log, files),
8691
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),
8792

8893
processors.NewUniqByLine(cfg),
@@ -92,7 +97,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
9297
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg),
9398
processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
9499
processors.NewPathShortener(),
95-
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
100+
getSeverityRulesProcessor(&cfg.Severity, log, files),
96101
processors.NewPathPrefixer(cfg.Output.PathPrefix),
97102
processors.NewSortResults(cfg),
98103
},
@@ -259,7 +264,7 @@ func getExcludeProcessor(cfg *config.Issues) processors.Processor {
259264
return excludeProcessor
260265
}
261266

262-
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 {
263268
var excludeRules []processors.ExcludeRule
264269
for _, r := range cfg.ExcludeRules {
265270
excludeRules = append(excludeRules, processors.ExcludeRule{
@@ -287,21 +292,21 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
287292
if cfg.ExcludeCaseSensitive {
288293
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
289294
excludeRules,
290-
lineCache,
295+
files,
291296
log.Child(logutils.DebugKeyExcludeRules),
292297
)
293298
} else {
294299
excludeRulesProcessor = processors.NewExcludeRules(
295300
excludeRules,
296-
lineCache,
301+
files,
297302
log.Child(logutils.DebugKeyExcludeRules),
298303
)
299304
}
300305

301306
return excludeRulesProcessor
302307
}
303308

304-
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 {
305310
var severityRules []processors.SeverityRule
306311
for _, r := range cfg.Rules {
307312
severityRules = append(severityRules, processors.SeverityRule{
@@ -320,14 +325,14 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache
320325
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
321326
cfg.Default,
322327
severityRules,
323-
lineCache,
328+
files,
324329
log.Child(logutils.DebugKeySeverityRules),
325330
)
326331
} else {
327332
severityRulesProcessor = processors.NewSeverityRules(
328333
cfg.Default,
329334
severityRules,
330-
lineCache,
335+
files,
331336
log.Child(logutils.DebugKeySeverityRules),
332337
)
333338
}

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
}

0 commit comments

Comments
 (0)