Skip to content

Commit

Permalink
fix: consider path prefix when matching path patterns (#3571)
Browse files Browse the repository at this point in the history
  • Loading branch information
pohly authored Mar 17, 2023
1 parent e27b129 commit b40a544
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
Loading

0 comments on commit b40a544

Please sign in to comment.