diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index a955075b..169aa5d7 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -54,7 +54,7 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, g processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least processors.NewCgo(goenv), skipFilesProcessor, - skipDirsProcessor, + skipDirsProcessor, // must be after path prettifier processors.NewAutogeneratedExclude(astCache), processors.NewExclude(excludeTotalPattern), @@ -205,6 +205,7 @@ func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { go func() { sw := timeutils.NewStopwatch("processing", r.Log) + var issuesBefore, issuesAfter int defer close(outCh) for res := range inCh { @@ -214,7 +215,9 @@ func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { } if len(res.issues) != 0 { + issuesBefore += len(res.issues) res.issues = r.processIssues(res.issues, sw) + issuesAfter += len(res.issues) outCh <- res } } @@ -228,6 +231,9 @@ func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { }) } + if issuesBefore != issuesAfter { + r.Log.Infof("Issues before processing: %d, after processing: %d", issuesBefore, issuesAfter) + } sw.PrintStages() }() diff --git a/pkg/result/processors/skip_dirs.go b/pkg/result/processors/skip_dirs.go index 2e4d263e..e60bb579 100644 --- a/pkg/result/processors/skip_dirs.go +++ b/pkg/result/processors/skip_dirs.go @@ -3,7 +3,6 @@ package processors import ( "path/filepath" "regexp" - "sort" "strings" "github.com/pkg/errors" @@ -13,19 +12,15 @@ import ( ) type SkipDirs struct { - patterns []*regexp.Regexp - log logutils.Log - skippedDirs map[string]bool - sortedAbsArgs []string + patterns []*regexp.Regexp + log logutils.Log + skippedDirs map[string][]string // regexp to dir mapping + absArgsDirs []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] } +const goFileSuffix = ".go" func NewSkipDirs(patterns []string, log logutils.Log, runArgs []string) (*SkipDirs, error) { var patternsRe []*regexp.Regexp @@ -40,24 +35,25 @@ func NewSkipDirs(patterns []string, log logutils.Log, runArgs []string) (*SkipDi if len(runArgs) == 0 { runArgs = append(runArgs, "./...") } - var sortedAbsArgs []string + var absArgsDirs []string for _, arg := range runArgs { - if filepath.Base(arg) == "..." { + base := filepath.Base(arg) + if base == "..." || strings.HasSuffix(base, goFileSuffix) { 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) + absArgsDirs = append(absArgsDirs, absArg) } - sort.Sort(sortedByLenStrings(sortedAbsArgs)) return &SkipDirs{ - patterns: patternsRe, - log: log, - skippedDirs: map[string]bool{}, - sortedAbsArgs: sortedAbsArgs, + patterns: patternsRe, + log: log, + skippedDirs: map[string][]string{}, + absArgsDirs: absArgsDirs, }, nil } @@ -73,38 +69,38 @@ func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) { 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 == "" { + if filepath.IsAbs(i.FilePath()) { + p.log.Warnf("Got abs path in skip dirs processor, it should be relative") return true } - if strings.HasSuffix(filepath.Base(relIssuePath), ".go") { - relIssuePath = filepath.Dir(relIssuePath) + issueRelDir := filepath.Dir(i.FilePath()) + issueAbsDir, err := filepath.Abs(issueRelDir) + if err != nil { + p.log.Warnf("Can't abs-ify path %q: %s", issueRelDir, err) + return true } + for _, absArgDir := range p.absArgsDirs { + if absArgDir == issueAbsDir { + p.log.Infof("Pass issue in file %s because it's dir was explicitly set in arg %s", i.FilePath(), absArgDir) + // we must not skip issues if they are from explicitly set dirs + // even if they match skip patterns + return true + } + } + + // We use issueRelDir for matching: it's the relative to the current + // work dir path of directory of source file with the issue. It can lead + // to unexpected behavior if we're analyzing files out of current work dir. + // The alternative solution is to find relative to args path, but it has + // disadvantages (https://github.com/golangci/golangci-lint/pull/313). + for _, pattern := range p.patterns { - if pattern.MatchString(relIssuePath) { - p.skippedDirs[relIssuePath] = true + if pattern.MatchString(issueRelDir) { + ps := pattern.String() + p.skippedDirs[ps] = append(p.skippedDirs[ps], issueRelDir) return false } } @@ -113,11 +109,7 @@ func (p *SkipDirs) shouldPassIssue(i *result.Issue) bool { } 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) + for pattern, dirs := range p.skippedDirs { + p.log.Infof("Skipped by pattern %s dirs: %s", pattern, dirs) } } diff --git a/test/run_test.go b/test/run_test.go index 837ec2d0..c5c4818f 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -56,15 +56,21 @@ func TestUnsafeOk(t *testing.T) { testshared.NewLintRunner(t).Run("--enable-all", getTestDataDir("unsafe")).ExpectNoIssues() } -func TestSkippedDirs(t *testing.T) { - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "--skip-dirs", "skip_me", "-Egolint", - getTestDataDir("skipdirs", "...")) +func TestSkippedDirsNoMatchArg(t *testing.T) { + dir := getTestDataDir("skipdirs", "skip_me", "nested") + r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "--skip-dirs", dir, "-Egolint", dir) r.ExpectExitCode(exitcodes.IssuesFound). - ExpectOutputEq("testdata/skipdirs/examples_no_skip/with_issue.go:8:9: if block ends with " + + ExpectOutputEq("testdata/skipdirs/skip_me/nested/with_issue.go:8:9: if block ends with " + "a return statement, so drop this else and outdent its block (golint)\n") } +func TestSkippedDirsTestdata(t *testing.T) { + r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "-Egolint", getTestDataDir("skipdirs", "...")) + + r.ExpectNoIssues() // all was skipped because in testdata +} + func TestDeadcodeNoFalsePositivesInMainPkg(t *testing.T) { testshared.NewLintRunner(t).Run("--no-config", "--disable-all", "-Edeadcode", getTestDataDir("deadcode_main_pkg")).ExpectNoIssues() } diff --git a/test/testshared/testshared.go b/test/testshared/testshared.go index ecd44739..27c12160 100644 --- a/test/testshared/testshared.go +++ b/test/testshared/testshared.go @@ -46,8 +46,8 @@ type RunResult struct { } func (r *RunResult) ExpectNoIssues() { - assert.Equal(r.t, "", r.output, r.exitCode) - assert.Equal(r.t, exitcodes.Success, r.exitCode, r.output) + assert.Equal(r.t, "", r.output, "exit code is %d", r.exitCode) + assert.Equal(r.t, exitcodes.Success, r.exitCode, "output is %s", r.output) } func (r *RunResult) ExpectExitCode(possibleCodes ...int) *RunResult {