Sean Chittenden 441bdb33d1 Fix the skip-dirs option.
Prior to this change the SkipDir runner was not skipping files such as
`foo/bar/baz.go` with a `skip-dir` entry of `foo/bar` when the `run`
command was invoked with an argument of `foo/...`.  This is both a
surprising and problematic behavior change.

The pathology was:

1. `shouldPassIssue()` was receiving an input like `foo/bar/baz.go`
2. `shouldPassIssue()` would call `p.getLongestArgRelativeIssuePath()`
   which was returning `bar`, not `foo/bar` as expected.
3. The reason for this was because inside of
   `getLongestArgRelativeIssuePath()` it was trimming the prefix that
   matched the path prefix (e.g. `foo/`).

If you have the file structure:

  - foo/bar/baz.go
  - bur/bar/baz.go

There is no way to isolate `foo/bar` from `bur/baz` without strictly
controlling both your `skip-dirs` configuration and the arguments to
`run`.

The rest of the logic to skip files that don't match `run`'s argument
is valid, however the regexp should be evaluated based on the
`filepath.Dir()` of the input (e.g. `foo/bar`) and not the truncated
version of the issue's filepath.

Fixes: #301
2018-12-22 12:24:43 +03:00

124 lines
2.8 KiB
Go

package processors
import (
"path/filepath"
"regexp"
"sort"
"strings"
"github.com/pkg/errors"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
)
type SkipDirs struct {
patterns []*regexp.Regexp
log logutils.Log
skippedDirs map[string]bool
sortedAbsArgs []string
}
var _ Processor = SkipFiles{}
type sortedByLenStrings []string
func (s sortedByLenStrings) Len() int { return len(s) }
func (s sortedByLenStrings) Less(i, j int) bool { return len(s[i]) > len(s[j]) }
func (s sortedByLenStrings) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func NewSkipDirs(patterns []string, log logutils.Log, runArgs []string) (*SkipDirs, error) {
var patternsRe []*regexp.Regexp
for _, p := range patterns {
patternRe, err := regexp.Compile(p)
if err != nil {
return nil, errors.Wrapf(err, "can't compile regexp %q", p)
}
patternsRe = append(patternsRe, patternRe)
}
if len(runArgs) == 0 {
runArgs = append(runArgs, "./...")
}
var sortedAbsArgs []string
for _, arg := range runArgs {
if filepath.Base(arg) == "..." {
arg = filepath.Dir(arg)
}
absArg, err := filepath.Abs(arg)
if err != nil {
return nil, errors.Wrapf(err, "failed to abs-ify arg %q", arg)
}
sortedAbsArgs = append(sortedAbsArgs, absArg)
}
sort.Sort(sortedByLenStrings(sortedAbsArgs))
return &SkipDirs{
patterns: patternsRe,
log: log,
skippedDirs: map[string]bool{},
sortedAbsArgs: sortedAbsArgs,
}, nil
}
func (p SkipDirs) Name() string {
return "skip_dirs"
}
func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) {
if len(p.patterns) == 0 {
return issues, nil
}
return filterIssues(issues, p.shouldPassIssue), nil
}
func (p *SkipDirs) getLongestArgRelativeIssuePath(i *result.Issue) string {
issueAbsPath, err := filepath.Abs(i.FilePath())
if err != nil {
p.log.Warnf("Can't abs-ify path %q: %s", i.FilePath(), err)
return ""
}
for _, arg := range p.sortedAbsArgs {
if !strings.HasPrefix(issueAbsPath, arg) {
continue
}
return i.FilePath()
}
p.log.Infof("Issue path %q isn't relative to any of run args", i.FilePath())
return ""
}
func (p *SkipDirs) shouldPassIssue(i *result.Issue) bool {
relIssuePath := p.getLongestArgRelativeIssuePath(i)
if relIssuePath == "" {
return true
}
if strings.HasSuffix(filepath.Base(relIssuePath), ".go") {
relIssuePath = filepath.Dir(relIssuePath)
}
for _, pattern := range p.patterns {
if pattern.MatchString(relIssuePath) {
p.skippedDirs[relIssuePath] = true
return false
}
}
return true
}
func (p SkipDirs) Finish() {
if len(p.skippedDirs) != 0 {
var skippedDirs []string
for dir := range p.skippedDirs {
skippedDirs = append(skippedDirs, dir)
}
p.log.Infof("Skipped dirs: %s", skippedDirs)
}
}