Skip to content

Commit

Permalink
config: fix path matching when path prefix is used
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pohly committed Mar 16, 2023
1 parent f0dbc75 commit 91e4b2e
Show file tree
Hide file tree
Showing 13 changed files with 181 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ run:
- mytag

# Which dirs to skip: issues from them won't be reported.
# Can use regexp here: `generated.*`, regexp is applied on full path.
# Can use regexp here: `generated.*`, regexp is applied on full path,
# including the path prefix if one is set.
# Default value is empty list,
# but default dirs are skipped independently of this option's value (see skip-dirs-use-default).
# "/" will be replaced by current OS file path separator to properly work on Windows.
Expand Down
4 changes: 4 additions & 0 deletions docs/src/docs/usage/false-positives.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ issues:

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

Beware that the paths that get matched here are relative to the current working directory.
When the configuration contains path patterns that check for specific directories,
the `--path-prefix` parameter can be used to extend the paths before matching.

In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded:

```yml
Expand Down
33 changes: 33 additions & 0 deletions pkg/fsutils/files.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package fsutils

import "path/filepath"

// Files combines different operations related to handling file paths and content.
type Files struct {
*LineCache
pathPrefix string
}

func NewFiles(lc *LineCache, pathPrefix string) *Files {
return &Files{
LineCache: lc,
pathPrefix: pathPrefix,
}
}

// WithPathPrefix takes a path that is relative to the current directory (as used in issues)
// and adds the configured path prefix, if there is one.
// The resulting path then can be shown to the user or compared against paths specified in the configuration.
func (f *Files) WithPathPrefix(relativePath string) string {
return WithPathPrefix(f.pathPrefix, relativePath)
}

// WithPathPrefix takes a path that is relative to the current directory (as used in issues)
// and adds the configured path prefix, if there is one.
// The resulting path then can be shown to the user or compared against paths specified in the configuration.
func WithPathPrefix(pathPrefix, relativePath string) string {
if pathPrefix == "" {
return relativePath
}
return filepath.Join(pathPrefix, relativePath)
}
25 changes: 15 additions & 10 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ type Runner struct {

func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lintersdb.EnabledSet,
lineCache *fsutils.LineCache, dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles)
// Beware that some processors need to add the path prefix when working with paths
// because they get invoked before the path prefixer (exclude and severity rules)
// or process other paths (skip files).
files := fsutils.NewFiles(lineCache, cfg.Output.PathPrefix)

skipFilesProcessor, err := processors.NewSkipFiles(cfg.Run.SkipFiles, cfg.Output.PathPrefix)
if err != nil {
return nil, err
}
Expand All @@ -38,7 +43,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
if cfg.Run.UseDefaultSkipDirs {
skipDirs = append(skipDirs, packages.StdExcludeDirRegexps...)
}
skipDirsProcessor, err := processors.NewSkipDirs(skipDirs, log.Child(logutils.DebugKeySkipDirs), cfg.Run.Args)
skipDirsProcessor, err := processors.NewSkipDirs(skipDirs, log.Child(logutils.DebugKeySkipDirs), cfg.Run.Args, cfg.Output.PathPrefix)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -82,7 +87,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, files),
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
Expand All @@ -92,7 +97,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg),
processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
processors.NewPathShortener(),
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
getSeverityRulesProcessor(&cfg.Severity, log, files),
processors.NewPathPrefixer(cfg.Output.PathPrefix),
processors.NewSortResults(cfg),
},
Expand Down Expand Up @@ -259,7 +264,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, files *fsutils.Files) processors.Processor {
var excludeRules []processors.ExcludeRule
for _, r := range cfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
Expand Down Expand Up @@ -287,21 +292,21 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
if cfg.ExcludeCaseSensitive {
excludeRulesProcessor = processors.NewExcludeRulesCaseSensitive(
excludeRules,
lineCache,
files,
log.Child(logutils.DebugKeyExcludeRules),
)
} else {
excludeRulesProcessor = processors.NewExcludeRules(
excludeRules,
lineCache,
files,
log.Child(logutils.DebugKeyExcludeRules),
)
}

return excludeRulesProcessor
}

func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache *fsutils.LineCache) processors.Processor {
func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fsutils.Files) processors.Processor {
var severityRules []processors.SeverityRule
for _, r := range cfg.Rules {
severityRules = append(severityRules, processors.SeverityRule{
Expand All @@ -320,14 +325,14 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache
severityRulesProcessor = processors.NewSeverityRulesCaseSensitive(
cfg.Default,
severityRules,
lineCache,
files,
log.Child(logutils.DebugKeySeverityRules),
)
} else {
severityRulesProcessor = processors.NewSeverityRules(
cfg.Default,
severityRules,
lineCache,
files,
log.Child(logutils.DebugKeySeverityRules),
)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/result/processors/base_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ 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, files *fsutils.Files, 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(files.WithPathPrefix(issue.FilePath())) {
return false
}
if len(r.linters) != 0 && !r.matchLinter(issue) {
return false
}

// the most heavyweight checking last
if r.source != nil && !r.matchSource(issue, lineCache, log) {
if r.source != nil && !r.matchSource(issue, files.LineCache, log) {
return false
}

Expand Down
20 changes: 10 additions & 10 deletions pkg/result/processors/exclude_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ type ExcludeRule struct {
}

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

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

Expand Down Expand Up @@ -59,7 +59,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.files, p.log) {
return false
}
}
Expand All @@ -76,10 +76,10 @@ type ExcludeRulesCaseSensitive struct {
*ExcludeRules
}

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

Expand Down
45 changes: 43 additions & 2 deletions pkg/result/processors/exclude_rules_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package processors

import (
"path"
"path/filepath"
"testing"

Expand All @@ -12,6 +13,8 @@ import (

func TestExcludeRulesMultiple(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "")

p := NewExcludeRules([]ExcludeRule{
{
BaseRule: BaseRule{
Expand All @@ -37,7 +40,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
Linters: []string{"lll"},
},
},
}, lineCache, nil)
}, files, nil)

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

func TestExcludeRulesPathPrefix(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
pathPrefix := path.Join("some", "dir")
files := fsutils.NewFiles(lineCache, pathPrefix)

p := NewExcludeRules([]ExcludeRule{
{
BaseRule: BaseRule{
Path: `some/dir/e\.go`,
},
},
}, files, nil)

cases := []issueTestCase{
{Path: "e.go"},
{Path: "other.go"},
}
var issues []result.Issue
for _, c := range cases {
issues = append(issues, newIssueFromIssueTestCase(c))
}
processedIssues := process(t, p, issues...)
var resultingCases []issueTestCase
for _, i := range processedIssues {
resultingCases = append(resultingCases, issueTestCase{
Path: i.FilePath(),
Linter: i.FromLinter,
Text: i.Text,
Line: i.Line(),
})
}
expectedCases := []issueTestCase{
{Path: "other.go"},
}
assert.Equal(t, expectedCases, resultingCases)
}

func TestExcludeRulesText(t *testing.T) {
p := NewExcludeRules([]ExcludeRule{
{
Expand Down Expand Up @@ -104,6 +144,7 @@ func TestExcludeRulesEmpty(t *testing.T) {

func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
lineCache := fsutils.NewLineCache(fsutils.NewFileCache())
files := fsutils.NewFiles(lineCache, "")
p := NewExcludeRulesCaseSensitive([]ExcludeRule{
{
BaseRule: BaseRule{
Expand All @@ -129,7 +170,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
Linters: []string{"lll"},
},
},
}, lineCache, nil)
}, files, nil)

cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
Expand Down
5 changes: 2 additions & 3 deletions pkg/result/processors/path_prefixer.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package processors

import (
"path/filepath"

"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/result"
)

Expand All @@ -27,7 +26,7 @@ func (*PathPrefixer) Name() string {
func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) {
if p.prefix != "" {
for i := range issues {
issues[i].Pos.Filename = filepath.Join(p.prefix, issues[i].Pos.Filename)
issues[i].Pos.Filename = fsutils.WithPathPrefix(p.prefix, issues[i].Pos.Filename)
}
}
return issues, nil
Expand Down
12 changes: 6 additions & 6 deletions pkg/result/processors/severity_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ type SeverityRule struct {
type SeverityRules struct {
defaultSeverity string
rules []severityRule
lineCache *fsutils.LineCache
files *fsutils.Files
log logutils.Log
}

func NewSeverityRules(defaultSeverity string, rules []SeverityRule, lineCache *fsutils.LineCache, log logutils.Log) *SeverityRules {
func NewSeverityRules(defaultSeverity string, rules []SeverityRule, files *fsutils.Files, log logutils.Log) *SeverityRules {
r := &SeverityRules{
lineCache: lineCache,
files: files,
log: log,
defaultSeverity: defaultSeverity,
}
Expand Down 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, p.files, p.log) {
i.Severity = ruleSeverity
return i
}
Expand All @@ -90,9 +90,9 @@ type SeverityRulesCaseSensitive struct {
}

func NewSeverityRulesCaseSensitive(defaultSeverity string, rules []SeverityRule,
lineCache *fsutils.LineCache, log logutils.Log) *SeverityRulesCaseSensitive {
files *fsutils.Files, log logutils.Log) *SeverityRulesCaseSensitive {
r := &SeverityRules{
lineCache: lineCache,
files: files,
log: log,
defaultSeverity: defaultSeverity,
}
Expand Down

0 comments on commit 91e4b2e

Please sign in to comment.