Skip to content

fix: consider path prefix when matching path patterns #3571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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