From e6657e868ee3a6dca13c80737f9615c0e045108d Mon Sep 17 00:00:00 2001 From: golangci Date: Tue, 8 May 2018 12:28:36 +0300 Subject: [PATCH] don't analyze tests by default --- internal/commands/run.go | 6 ++--- pkg/config/config.go | 2 ++ pkg/fsutils/fsutils.go | 4 ++-- pkg/fsutils/path_resolver.go | 8 ++++++- pkg/fsutils/path_resolver_test.go | 37 +++++++++++++++++++++++++------ pkg/golinters/golint.go | 2 +- 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/internal/commands/run.go b/internal/commands/run.go index d0da61fa..6d9c6bf8 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -88,6 +88,7 @@ func (e *Executor) initRun() { runCmd.Flags().BoolVarP(&rc.Diff, "new", "n", false, "Show only new issues: if there are unstaged changes or untracked files, only those changes are shown, else only changes in HEAD~ are shown") runCmd.Flags().StringVar(&rc.DiffFromRevision, "new-from-rev", "", "Show only new issues created after git revision `REV`") runCmd.Flags().StringVar(&rc.DiffPatchFilePath, "new-from-patch", "", "Show only new issues created in git patch with file path `PATH`") + runCmd.Flags().BoolVar(&rc.AnalyzeTests, "tests", false, "Analyze tests (*_test.go)") } func isFullImportNeeded(linters []pkg.Linter) bool { @@ -128,8 +129,7 @@ func loadWholeAppIfNeeded(ctx context.Context, linters []pkg.Linter, cfg *config Build: &bctx, AllowErrors: true, // Try to analyze event partially } - const needTests = true // TODO: configure and take into account in paths resolver - rest, err := loadcfg.FromArgs(paths.MixedPaths(), needTests) + rest, err := loadcfg.FromArgs(paths.MixedPaths(), cfg.AnalyzeTests) if err != nil { return nil, nil, fmt.Errorf("can't parepare load config with paths: %s", err) } @@ -162,7 +162,7 @@ func buildLintCtx(ctx context.Context, linters []pkg.Linter, cfg *config.Config) args = []string{"./..."} } - paths, err := fsutils.GetPathsForAnalysis(ctx, args) + paths, err := fsutils.GetPathsForAnalysis(ctx, args, cfg.Run.AnalyzeTests) if err != nil { return nil, err } diff --git a/pkg/config/config.go b/pkg/config/config.go index d0be7471..5e9cd512 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -92,6 +92,8 @@ type Run struct { // nolint:maligned DiffFromRevision string DiffPatchFilePath string Diff bool + + AnalyzeTests bool } type Config struct { diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index 00e963eb..09100f5b 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -55,7 +55,7 @@ func processPaths(root string, paths []string, maxPaths int) ([]string, error) { return ret, nil } -func GetPathsForAnalysis(ctx context.Context, inputPaths []string) (ret *ProjectPaths, err error) { +func GetPathsForAnalysis(ctx context.Context, inputPaths []string, includeTests bool) (ret *ProjectPaths, err error) { defer func(startedAt time.Time) { if ret != nil { logrus.Infof("Found paths for analysis for %s: %s", time.Since(startedAt), ret.MixedPaths()) @@ -69,7 +69,7 @@ func GetPathsForAnalysis(ctx context.Context, inputPaths []string) (ret *Project } excludeDirs := []string{"vendor", "testdata", "examples", "Godeps", "builtin"} - pr := NewPathResolver(excludeDirs, []string{".go"}) + pr := NewPathResolver(excludeDirs, []string{".go"}, includeTests) paths, err := pr.Resolve(inputPaths...) if err != nil { return nil, fmt.Errorf("can't resolve paths: %s", err) diff --git a/pkg/fsutils/path_resolver.go b/pkg/fsutils/path_resolver.go index ba110508..3e826bfd 100644 --- a/pkg/fsutils/path_resolver.go +++ b/pkg/fsutils/path_resolver.go @@ -11,6 +11,7 @@ import ( type PathResolver struct { excludeDirs map[string]bool allowedFileExtensions map[string]bool + includeTests bool } type pathResolveState struct { @@ -56,7 +57,7 @@ func (s pathResolveState) toResult() *PathResolveResult { return res } -func NewPathResolver(excludeDirs, allowedFileExtensions []string) *PathResolver { +func NewPathResolver(excludeDirs, allowedFileExtensions []string, includeTests bool) *PathResolver { excludeDirsMap := map[string]bool{} for _, dir := range excludeDirs { excludeDirsMap[dir] = true @@ -70,6 +71,7 @@ func NewPathResolver(excludeDirs, allowedFileExtensions []string) *PathResolver return &PathResolver{ excludeDirs: excludeDirsMap, allowedFileExtensions: allowedFileExtensionsMap, + includeTests: includeTests, } } @@ -89,6 +91,10 @@ func (pr PathResolver) isIgnoredDir(dir string) bool { } func (pr PathResolver) isAllowedFile(path string) bool { + if !pr.includeTests && strings.HasSuffix(path, "_test.go") { + return false + } + return pr.allowedFileExtensions[filepath.Ext(path)] } diff --git a/pkg/fsutils/path_resolver_test.go b/pkg/fsutils/path_resolver_test.go index 35b5364d..e8cb4c51 100644 --- a/pkg/fsutils/path_resolver_test.go +++ b/pkg/fsutils/path_resolver_test.go @@ -54,7 +54,7 @@ func prepareFS(t *testing.T, paths ...string) *fsPreparer { } func newPR() *PathResolver { - return NewPathResolver([]string{}, []string{}) + return NewPathResolver([]string{}, []string{}, false) } func TestPathResolverNoPaths(t *testing.T) { @@ -72,11 +72,12 @@ func TestPathResolverNotExistingPath(t *testing.T) { func TestPathResolverCommonCases(t *testing.T) { type testCase struct { - name string - prepare []string - resolve []string - expFiles []string - expDirs []string + name string + prepare []string + resolve []string + expFiles []string + expDirs []string + includeTests bool } testCases := []testCase{ @@ -154,6 +155,28 @@ func TestPathResolverCommonCases(t *testing.T) { resolve: []string{"./..."}, expDirs: []string{"."}, }, + { + name: "include tests", + prepare: []string{"a/b.go", "a/b_test.go"}, + resolve: []string{"./..."}, + expDirs: []string{".", "a"}, + expFiles: []string{"a/b.go", "a/b_test.go"}, + includeTests: true, + }, + { + name: "exclude tests", + prepare: []string{"a/b.go", "a/b_test.go"}, + resolve: []string{"./..."}, + expDirs: []string{".", "a"}, + expFiles: []string{"a/b.go"}, + }, + { + name: "exclude tests except explicitly set", + prepare: []string{"a/b.go", "a/b_test.go", "a/c_test.go"}, + resolve: []string{"./...", "a/c_test.go"}, + expDirs: []string{".", "a"}, + expFiles: []string{"a/b.go", "a/c_test.go"}, + }, } for _, tc := range testCases { @@ -161,7 +184,7 @@ func TestPathResolverCommonCases(t *testing.T) { fp := prepareFS(t, tc.prepare...) defer fp.clean() - pr := NewPathResolver([]string{"vendor"}, []string{".go"}) + pr := NewPathResolver([]string{"vendor"}, []string{".go"}, tc.includeTests) res, err := pr.Resolve(tc.resolve...) assert.NoError(t, err) diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 04fc003a..5e96a7db 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -20,7 +20,7 @@ func (Golint) Name() string { func (g Golint) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { var issues []result.Issue if lintCtx.Paths.IsDirsRun { - for _, path := range lintCtx.Paths.Dirs { + for _, path := range lintCtx.Paths.Dirs { // TODO: support exclusion of test files i, err := lintDir(path, lintCtx.RunCfg().Golint.MinConfidence) if err != nil { // TODO: skip and warn