From 2b587b63d6ae9c86f68fbcf27471a335469adfc5 Mon Sep 17 00:00:00 2001 From: Denis Isaev Date: Sun, 10 Jun 2018 17:10:56 +0300 Subject: [PATCH] Load AST for fast linters in different way. Use build.Import instead of manual parser.ParseFile and paths traversal. It allows: 1. support build tags for all linters. 2. analyze files only for current GOOS/GOARCH: less false-positives. 3. analyze xtest packages (*_test) by golint: upstream golint and gometalinter can't do it! And don't break analysis on the first xtest package like it was before. 4. proper handling of xtest packages for linters like goconst where package boundary is important: less false-positives is expected. Also: 1. reuse AST parsing for golint and goconst: minor speedup. 2. allow to specify path (not only name) regexp for --skip-files and --skip-dirs 3. add more default exclude filters for golint about commits: `(comment on exported (method|function)|should have( a package)? comment|comment should be of the form)` 4. print skipped dir in verbose (-v) mode 5. refactor per-linter tests: declare arguments in comments, run only one linter and in combination with slow linter --- .golangci.example.yml | 4 +- Gopkg.lock | 16 +- Makefile | 3 +- README.md | 8 +- pkg/commands/run.go | 6 +- pkg/config/config.go | 2 +- pkg/fsutils/fsutils.go | 130 ----------- pkg/fsutils/path_resolver.go | 216 ------------------ pkg/golinters/dupl.go | 2 +- pkg/golinters/goconst.go | 18 +- pkg/golinters/gofmt.go | 2 +- pkg/golinters/golint.go | 32 ++- pkg/golinters/govet.go | 4 +- pkg/golinters/ineffassign.go | 2 +- pkg/golinters/utils.go | 26 +++ pkg/lint/astcache/astcache.go | 17 +- pkg/lint/linter/context.go | 4 +- pkg/lint/load.go | 92 +++++--- pkg/lint/load_test.go | 20 +- pkg/packages/exclude.go | 8 + pkg/packages/package.go | 28 +++ pkg/packages/program.go | 71 ++++++ pkg/packages/resolver.go | 206 +++++++++++++++++ .../resolver_test.go} | 107 +++++++-- pkg/result/processors/skip_files.go | 4 +- pkg/result/processors/skip_files_test.go | 18 +- pkg/timeutils/track.go | 12 + test/linters_test.go | 72 ++++-- test/run_test.go | 5 + test/testdata/deadcode.go | 5 +- test/testdata/depguard.go | 1 + test/testdata/dupl.go | 5 +- test/testdata/errcheck.go | 1 + test/testdata/gas.go | 3 +- test/testdata/goconst.go | 5 +- test/testdata/gocyclo.go | 7 +- test/testdata/gofmt.go | 3 +- test/testdata/goimports.go | 1 + test/testdata/golint.go | 3 +- test/testdata/govet.go | 1 + test/testdata/ineffassign.go | 1 + test/testdata/interfacer.go | 1 + test/testdata/maligned.go | 1 + test/testdata/megacheck.go | 3 +- test/testdata/{ => notcompiles}/typecheck.go | 1 + .../typecheck_many_issues.go | 1 + test/testdata/structcheck.go | 5 +- test/testdata/symlink_loop/pkg/subpkg | 1 + test/testdata/symlink_loop/realpkg/p.go | 1 + test/testdata/unconvert.go | 3 +- test/testdata/varcheck.go | 3 +- test/testdata/withxtest/p.go | 1 + test/testdata/withxtest/p_test.go | 11 + vendor/github.com/golangci/goconst/parser.go | 21 +- .../lint => golangci/lint-1}/.travis.yml | 0 .../lint => golangci/lint-1}/CONTRIBUTING.md | 0 .../{golang/lint => golangci/lint-1}/LICENSE | 0 .../lint => golangci/lint-1}/README.md | 0 .../{golang/lint => golangci/lint-1}/lint.go | 41 ++++ 59 files changed, 754 insertions(+), 511 deletions(-) delete mode 100644 pkg/fsutils/path_resolver.go create mode 100644 pkg/packages/exclude.go create mode 100644 pkg/packages/package.go create mode 100644 pkg/packages/program.go create mode 100644 pkg/packages/resolver.go rename pkg/{fsutils/path_resolver_test.go => packages/resolver_test.go} (62%) create mode 100644 pkg/timeutils/track.go rename test/testdata/{ => notcompiles}/typecheck.go (81%) rename test/testdata/{ => notcompiles}/typecheck_many_issues.go (93%) create mode 120000 test/testdata/symlink_loop/pkg/subpkg create mode 100644 test/testdata/symlink_loop/realpkg/p.go create mode 100644 test/testdata/withxtest/p.go create mode 100644 test/testdata/withxtest/p_test.go rename vendor/github.com/{golang/lint => golangci/lint-1}/.travis.yml (100%) rename vendor/github.com/{golang/lint => golangci/lint-1}/CONTRIBUTING.md (100%) rename vendor/github.com/{golang/lint => golangci/lint-1}/LICENSE (100%) rename vendor/github.com/{golang/lint => golangci/lint-1}/README.md (100%) rename vendor/github.com/{golang/lint => golangci/lint-1}/lint.go (97%) diff --git a/.golangci.example.yml b/.golangci.example.yml index dcd760fc7fd2..c0dafcb4b9ed 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -6,9 +6,11 @@ run: build-tags: - mytag skip-dirs: - - external_libs + - src/external_libs + - autogenerated_by_my_lib skip-files: - ".*\\.pb\\.go$" + - lib/bad.go output: format: colored-line-number diff --git a/Gopkg.lock b/Gopkg.lock index 556c6f14be9c..d803005e7fbe 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -56,12 +56,6 @@ revision = "1adfc126b41513cc696b209667c8656ea7aac67c" version = "v1.0.0" -[[projects]] - branch = "master" - name = "github.com/golang/lint" - packages = ["."] - revision = "470b6b0bb3005eda157f0275e2e4895055396a81" - [[projects]] branch = "master" name = "github.com/golangci/check" @@ -103,7 +97,7 @@ branch = "master" name = "github.com/golangci/goconst" packages = ["."] - revision = "b67b9035d29a7561902d87eb3757dd490b31c1b1" + revision = "041c5f2b40f3dd334a4a6ee6a3f84ca3fc70680a" [[projects]] branch = "master" @@ -142,6 +136,12 @@ packages = ["."] revision = "e1cc50c0cfa0e058f40ced1b3d54b86014440c91" +[[projects]] + branch = "master" + name = "github.com/golangci/lint-1" + packages = ["."] + revision = "4bf9709227d15e5f14c84a381e315a4695c4a372" + [[projects]] branch = "master" name = "github.com/golangci/maligned" @@ -416,6 +416,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "832e18c919daf77730cda91687bfd2d0b0b70fe6283c38b33e48ed89c2fd2def" + inputs-digest = "4558f81cfd21ec5b1f937b518089a231eb05212cf9545af1fc3df3dd00a7fa22" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Makefile b/Makefile index 351db7042e2a..2ca1b1751f59 100644 --- a/Makefile +++ b/Makefile @@ -3,8 +3,7 @@ test: golangci-lint run -v golangci-lint run --fast --no-config -v golangci-lint run --no-config -v - golangci-lint run --fast --no-config -v ./test/testdata/typecheck.go - go test -v -race ./... + go test -v ./... assets: svg-term --cast=183662 --out docs/demo.svg --window --width 110 --height 30 --from 2000 --to 20000 --profile Dracula --term iterm2 diff --git a/README.md b/README.md index cbb4e04a5b8e..49416b194aec 100644 --- a/README.md +++ b/README.md @@ -235,14 +235,14 @@ Flags: --print-issued-lines Print lines of code with issue (default true) --print-linter-name Print linter name in issue line (default true) --issues-exit-code int Exit code when issues were found (default 1) - --build-tags strings Build tags (not all linters support them) + --build-tags strings Build tags --deadline duration Deadline for total work (default 1m0s) --tests Analyze tests (*_test.go) (default true) --print-resources-usage Print avg and max memory usage of golangci-lint and total time -c, --config PATH Read config from file path PATH --no-config Don't read config - --skip-dirs strings Regexps of directory names to skip - --skip-files strings Regexps of file names to skip + --skip-dirs strings Regexps of directories to skip + --skip-files strings Regexps of files to skip -E, --enable strings Enable specific linter -D, --disable strings Disable specific linter --enable-all Enable all linters @@ -255,7 +255,7 @@ Flags: - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked # golint: Annoying issue about not having a comment. The rare codebase has such comments - - (should have comment|comment on exported method|should have a package comment) + - (comment on exported (method|function)|should have( a package)? comment|comment should be of the form) # golint: False positive when tests are defined in package 'test' - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 32c90694a5b1..328be40ffadd 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -65,14 +65,14 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config) { rc := &cfg.Run fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", 1, wh("Exit code when issues were found")) - fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags (not all linters support them)")) + fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) fs.DurationVar(&rc.Deadline, "deadline", time.Minute, wh("Deadline for total work")) fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, wh("Print avg and max memory usage of golangci-lint and total time")) fs.StringVarP(&rc.Config, "config", "c", "", wh("Read config from file path `PATH`")) fs.BoolVar(&rc.NoConfig, "no-config", false, wh("Don't read config")) - fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directory names to skip")) - fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of file names to skip")) + fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) + fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) // Linters settings config lsc := &cfg.LintersSettings diff --git a/pkg/config/config.go b/pkg/config/config.go index 30cff3fb688a..888d8b34d4ce 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -28,7 +28,7 @@ var DefaultExcludePatterns = []ExcludePattern{ Why: "Almost all programs ignore errors on these functions and in most cases it's ok", }, { - Pattern: "(should have comment|comment on exported method|should have a package comment)", + Pattern: "(comment on exported (method|function)|should have( a package)? comment|comment should be of the form)", Linter: "golint", Why: "Annoying issue about not having a comment. The rare codebase has such comments", }, diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index 561f9ffd8308..4c3a0db1bc8e 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -1,139 +1,9 @@ package fsutils import ( - "context" - "fmt" - "go/build" "os" - "path" - "path/filepath" - "strings" - "time" - - "github.com/sirupsen/logrus" ) -var stdExcludeDirRegexps = []string{ - "^vendor$", "^third_party$", - "^testdata$", "^examples$", - "^Godeps$", - "^builtin$", -} - -func GetProjectRoot() string { - return path.Join(build.Default.GOPATH, "src", "github.com", "golangci", "golangci-worker") -} - -type ProjectPaths struct { - Files []string - Dirs []string - IsDirsRun bool -} - -func (p ProjectPaths) MixedPaths() []string { - if p.IsDirsRun { - return p.Dirs - } - - return p.Files -} - -func (p ProjectPaths) FilesGrouppedByDirs() [][]string { - dirToFiles := map[string][]string{} - for _, f := range p.Files { - dir := filepath.Dir(f) - dirToFiles[dir] = append(dirToFiles[dir], f) - } - - ret := [][]string{} - for _, files := range dirToFiles { - ret = append(ret, files) - } - return ret -} - -func processPaths(root string, paths []string, maxPaths int) ([]string, error) { - if len(paths) > maxPaths { - logrus.Warnf("Gofmt: got too much paths (%d), analyze first %d", len(paths), maxPaths) - paths = paths[:maxPaths] - } - - ret := []string{} - for _, p := range paths { - if !filepath.IsAbs(p) { - ret = append(ret, p) - continue - } - - relPath, err := filepath.Rel(root, p) - if err != nil { - return nil, fmt.Errorf("can't get relative path for path %s and root %s: %s", - p, root, err) - } - ret = append(ret, relPath) - } - - return ret, nil -} - -func processResolvedPaths(paths *PathResolveResult) (*ProjectPaths, error) { - root, err := os.Getwd() - if err != nil { - return nil, fmt.Errorf("can't get working dir: %s", err) - } - - files, err := processPaths(root, paths.Files(), 10000) - if err != nil { - return nil, fmt.Errorf("can't process resolved files: %s", err) - } - - dirs, err := processPaths(root, paths.Dirs(), 1000) - if err != nil { - return nil, fmt.Errorf("can't process resolved dirs: %s", err) - } - - for i := range dirs { - dir := dirs[i] - if dir != "." && !filepath.IsAbs(dir) { - dirs[i] = "./" + dir - } - } - - return &ProjectPaths{ - Files: files, - Dirs: dirs, - IsDirsRun: len(dirs) != 0, - }, nil -} - -func GetPathsForAnalysis(ctx context.Context, inputPaths []string, includeTests bool, skipDirRegexps []string) (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()) - } - }(time.Now()) - - for _, path := range inputPaths { - if strings.HasSuffix(path, ".go") && len(inputPaths) != 1 { - return nil, fmt.Errorf("specific files for analysis are allowed only if one file is set") - } - } - - // TODO: don't analyze skipped files also, when be able to do it - excludeDirs := append([]string{}, stdExcludeDirRegexps...) - excludeDirs = append(excludeDirs, skipDirRegexps...) - pr, err := NewPathResolver(excludeDirs, []string{".go"}, includeTests) - if err != nil { - return nil, fmt.Errorf("can't make path resolver: %s", err) - } - paths, err := pr.Resolve(inputPaths...) - if err != nil { - return nil, fmt.Errorf("can't resolve paths %v: %s", inputPaths, err) - } - - return processResolvedPaths(paths) -} - func IsDir(filename string) bool { fi, err := os.Stat(filename) return err == nil && fi.IsDir() diff --git a/pkg/fsutils/path_resolver.go b/pkg/fsutils/path_resolver.go deleted file mode 100644 index 9c77287daf1c..000000000000 --- a/pkg/fsutils/path_resolver.go +++ /dev/null @@ -1,216 +0,0 @@ -package fsutils - -import ( - "fmt" - "os" - "path/filepath" - "regexp" - "sort" - "strings" -) - -type PathResolver struct { - excludeDirs map[string]*regexp.Regexp - allowedFileExtensions map[string]bool - includeTests bool -} - -type pathResolveState struct { - files map[string]bool - dirs map[string]bool -} - -func (s *pathResolveState) addFile(path string) { - s.files[filepath.Clean(path)] = true -} - -func (s *pathResolveState) addDir(path string) { - s.dirs[filepath.Clean(path)] = true -} - -type PathResolveResult struct { - files []string - dirs []string -} - -func (prr PathResolveResult) Files() []string { - return prr.files -} - -func (prr PathResolveResult) Dirs() []string { - return prr.dirs -} - -func (s pathResolveState) toResult() *PathResolveResult { - res := &PathResolveResult{ - files: []string{}, - dirs: []string{}, - } - for f := range s.files { - res.files = append(res.files, f) - } - for d := range s.dirs { - res.dirs = append(res.dirs, d) - } - - sort.Strings(res.files) - sort.Strings(res.dirs) - return res -} - -func NewPathResolver(excludeDirs, allowedFileExtensions []string, includeTests bool) (*PathResolver, error) { - excludeDirsMap := map[string]*regexp.Regexp{} - for _, dir := range excludeDirs { - re, err := regexp.Compile(dir) - if err != nil { - return nil, fmt.Errorf("can't compile regexp %q: %s", dir, err) - } - - excludeDirsMap[dir] = re - } - - allowedFileExtensionsMap := map[string]bool{} - for _, fe := range allowedFileExtensions { - allowedFileExtensionsMap[fe] = true - } - - return &PathResolver{ - excludeDirs: excludeDirsMap, - allowedFileExtensions: allowedFileExtensionsMap, - includeTests: includeTests, - }, nil -} - -func (pr PathResolver) isIgnoredDir(dir string) bool { - dirName := filepath.Base(filepath.Clean(dir)) // ignore dirs on any depth level - - // https://github.com/golang/dep/issues/298 - // https://github.com/tools/godep/issues/140 - if strings.HasPrefix(dirName, ".") && dirName != "." && dirName != ".." { - return true - } - if strings.HasPrefix(dirName, "_") { - return true - } - - for _, dirExludeRe := range pr.excludeDirs { - if dirExludeRe.MatchString(dirName) { - return true - } - } - - return false -} - -func (pr PathResolver) isAllowedFile(path string) bool { - if !pr.includeTests && strings.HasSuffix(path, "_test.go") { - return false - } - - return pr.allowedFileExtensions[filepath.Ext(path)] -} - -func (pr PathResolver) resolveRecursively(root string, state *pathResolveState) error { - walkErr := filepath.Walk(root, func(p string, i os.FileInfo, err error) error { - if err != nil { - return err - } - - if i.IsDir() { - if pr.isIgnoredDir(p) { - return filepath.SkipDir - } - state.addDir(p) - return nil - } - - if pr.isAllowedFile(p) { - state.addFile(p) - } - return nil - }) - - if walkErr != nil { - return fmt.Errorf("can't walk dir %s: %s", root, walkErr) - } - - return nil -} - -func (pr PathResolver) resolveDir(root string, state *pathResolveState) error { - walkErr := filepath.Walk(root, func(p string, i os.FileInfo, err error) error { - if err != nil { - return err - } - - if i.IsDir() { - if filepath.Clean(p) != filepath.Clean(root) { - return filepath.SkipDir - } - state.addDir(p) - return nil - } - - if pr.isAllowedFile(p) { - state.addFile(p) - } - return nil - }) - - if walkErr != nil { - return fmt.Errorf("can't walk dir %s: %s", root, walkErr) - } - - return nil -} - -func (pr PathResolver) Resolve(paths ...string) (*PathResolveResult, error) { - if len(paths) == 0 { - return nil, fmt.Errorf("no paths are set") - } - - state := &pathResolveState{ - files: map[string]bool{}, - dirs: map[string]bool{}, - } - for _, path := range paths { - if strings.HasSuffix(path, "/...") { - if err := pr.resolveRecursively(filepath.Dir(path), state); err != nil { - return nil, fmt.Errorf("can't recursively resolve %s: %s", path, err) - } - continue - } - - fi, err := os.Stat(path) - if err != nil { - return nil, fmt.Errorf("can't find path %s: %s", path, err) - } - - if fi.IsDir() { - if err := pr.resolveDir(path, state); err != nil { - return nil, fmt.Errorf("can't resolve dir %s: %s", path, err) - } - continue - } - - state.addFile(path) - } - - state.excludeDirsWithoutGoFiles() - - return state.toResult(), nil -} - -func (s *pathResolveState) excludeDirsWithoutGoFiles() { - dirToFiles := map[string]bool{} - for f := range s.files { - dir := filepath.Dir(f) - dirToFiles[dir] = true - } - - for dir := range s.dirs { - if !dirToFiles[dir] { // no go files in this dir - delete(s.dirs, dir) - } - } -} diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index 2784f3446a4b..0ede2d5037b8 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -21,7 +21,7 @@ func (Dupl) Desc() string { } func (d Dupl) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues, err := duplAPI.Run(lintCtx.Paths.Files, lintCtx.Settings().Dupl.Threshold) + issues, err := duplAPI.Run(lintCtx.PkgProgram.Files(lintCtx.Cfg.Run.AnalyzeTests), lintCtx.Settings().Dupl.Threshold) if err != nil { return nil, err } diff --git a/pkg/golinters/goconst.go b/pkg/golinters/goconst.go index 653561ebb8d2..fdc7c63b39e7 100644 --- a/pkg/golinters/goconst.go +++ b/pkg/golinters/goconst.go @@ -21,12 +21,18 @@ func (Goconst) Desc() string { func (lint Goconst) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { var goconstIssues []goconstAPI.Issue - // TODO: make it cross-package: pass package names inside goconst - for _, files := range lintCtx.Paths.FilesGrouppedByDirs() { - issues, err := goconstAPI.Run(files, true, - lintCtx.Settings().Goconst.MinStringLen, - lintCtx.Settings().Goconst.MinOccurrencesCount, - ) + cfg := goconstAPI.Config{ + MatchWithConstants: true, + MinStringLength: lintCtx.Settings().Goconst.MinStringLen, + MinOccurrences: lintCtx.Settings().Goconst.MinOccurrencesCount, + } + for _, pkg := range lintCtx.PkgProgram.Packages() { + files, fset, err := getASTFilesForPkg(lintCtx, &pkg) + if err != nil { + return nil, err + } + + issues, err := goconstAPI.Run(files, fset, &cfg) if err != nil { return nil, err } diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 773405dc2057..4c9f60399822 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -105,7 +105,7 @@ func (g Gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { func (g Gofmt) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { var issues []result.Issue - for _, f := range lintCtx.Paths.Files { + for _, f := range lintCtx.PkgProgram.Files(lintCtx.Cfg.Run.AnalyzeTests) { var diff []byte var err error if g.UseGoimports { diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 76ff23cbf449..32cbcfeef6b3 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -3,12 +3,13 @@ package golinters import ( "context" "fmt" - "io/ioutil" + "go/ast" + "go/token" - lintAPI "github.com/golang/lint" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" + lintAPI "github.com/golangci/lint-1" ) type Golint struct{} @@ -24,8 +25,13 @@ func (Golint) Desc() string { func (g Golint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { var issues []result.Issue var lintErr error - for _, pkgFiles := range lintCtx.Paths.FilesGrouppedByDirs() { - i, err := g.lintFiles(lintCtx.Settings().Golint.MinConfidence, pkgFiles...) + for _, pkg := range lintCtx.PkgProgram.Packages() { + files, fset, err := getASTFilesForPkg(lintCtx, &pkg) + if err != nil { + return nil, err + } + + i, err := g.lintPkg(lintCtx.Settings().Golint.MinConfidence, files, fset) if err != nil { lintErr = err continue @@ -39,26 +45,18 @@ func (g Golint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issu return issues, nil } -func (g Golint) lintFiles(minConfidence float64, filenames ...string) ([]result.Issue, error) { - files := make(map[string][]byte) - for _, filename := range filenames { - src, err := ioutil.ReadFile(filename) - if err != nil { - return nil, fmt.Errorf("can't read file %s: %s", filename, err) - } - files[filename] = src - } - +func (g Golint) lintPkg(minConfidence float64, files []*ast.File, fset *token.FileSet) ([]result.Issue, error) { l := new(lintAPI.Linter) - ps, err := l.LintFiles(files) + ps, err := l.LintASTFiles(files, fset) if err != nil { - return nil, fmt.Errorf("can't lint files %s: %s", filenames, err) + return nil, fmt.Errorf("can't lint %d files: %s", len(files), err) } + if len(ps) == 0 { return nil, nil } - issues := make([]result.Issue, 0, len(ps)) //This is worst case + issues := make([]result.Issue, 0, len(ps)) // This is worst case for _, p := range ps { if p.Confidence >= minConfidence { issues = append(issues, result.Issue{ diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 5dad22ac3c7e..768138d4753e 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -21,8 +21,8 @@ func (Govet) Desc() string { func (g Govet) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { // TODO: check .S asm files: govet can do it if pass dirs var govetIssues []govetAPI.Issue - for _, files := range lintCtx.Paths.FilesGrouppedByDirs() { - issues, err := govetAPI.Run(files, lintCtx.Settings().Govet.CheckShadowing) + for _, pkg := range lintCtx.PkgProgram.Packages() { + issues, err := govetAPI.Run(pkg.Files(lintCtx.Cfg.Run.AnalyzeTests), lintCtx.Settings().Govet.CheckShadowing) if err != nil { return nil, err } diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go index 3d13ddf351c7..a18e76314dbc 100644 --- a/pkg/golinters/ineffassign.go +++ b/pkg/golinters/ineffassign.go @@ -20,7 +20,7 @@ func (Ineffassign) Desc() string { } func (lint Ineffassign) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { - issues := ineffassignAPI.Run(lintCtx.Paths.Files) + issues := ineffassignAPI.Run(lintCtx.PkgProgram.Files(lintCtx.Cfg.Run.AnalyzeTests)) if len(issues) == 0 { return nil, nil } diff --git a/pkg/golinters/utils.go b/pkg/golinters/utils.go index 19512cf28130..21dd29bed10f 100644 --- a/pkg/golinters/utils.go +++ b/pkg/golinters/utils.go @@ -2,8 +2,13 @@ package golinters import ( "fmt" + "go/ast" + "go/token" "strings" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/packages" + "github.com/golangci/golangci-lint/pkg/config" ) @@ -22,3 +27,24 @@ func formatCodeBlock(code string, cfg *config.Config) string { return fmt.Sprintf("```\n%s\n```", code) } + +func getASTFilesForPkg(ctx *linter.Context, pkg *packages.Package) ([]*ast.File, *token.FileSet, error) { + filenames := pkg.Files(ctx.Cfg.Run.AnalyzeTests) + files := make([]*ast.File, 0, len(filenames)) + var fset *token.FileSet + for _, filename := range filenames { + f := ctx.ASTCache.Get(filename) + if f == nil { + return nil, nil, fmt.Errorf("no AST for file %s in cache", filename) + } + + if f.Err != nil { + return nil, nil, fmt.Errorf("can't load AST for file %s: %s", f.Name, f.Err) + } + + files = append(files, f.F) + fset = f.Fset + } + + return files, fset, nil +} diff --git a/pkg/lint/astcache/astcache.go b/pkg/lint/astcache/astcache.go index c9fcd3a6eac9..9415652474bc 100644 --- a/pkg/lint/astcache/astcache.go +++ b/pkg/lint/astcache/astcache.go @@ -16,7 +16,7 @@ type File struct { F *ast.File Fset *token.FileSet Name string - err error + Err error } type Cache struct { @@ -24,6 +24,10 @@ type Cache struct { s []*File } +func (c Cache) Get(filename string) *File { + return c.m[filepath.Clean(filename)] +} + func (c Cache) GetAllValidFiles() []*File { return c.s } @@ -31,7 +35,7 @@ func (c Cache) GetAllValidFiles() []*File { func (c *Cache) prepareValidFiles() { files := make([]*File, 0, len(c.m)) for _, f := range c.m { - if f.err != nil || f.F == nil { + if f.Err != nil || f.F == nil { continue } files = append(files, f) @@ -75,21 +79,24 @@ func LoadFromProgram(prog *loader.Program) (*Cache, error) { return c, nil } -func LoadFromFiles(files []string) *Cache { +func LoadFromFiles(files []string) (*Cache, error) { c := &Cache{ m: map[string]*File{}, } + fset := token.NewFileSet() for _, filePath := range files { + filePath = filepath.Clean(filePath) + f, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) // comments needed by e.g. golint c.m[filePath] = &File{ F: f, Fset: fset, - err: err, + Err: err, Name: filePath, } } c.prepareValidFiles() - return c + return c, nil } diff --git a/pkg/lint/linter/context.go b/pkg/lint/linter/context.go index 173b25361397..7a9684e48cc2 100644 --- a/pkg/lint/linter/context.go +++ b/pkg/lint/linter/context.go @@ -3,13 +3,13 @@ package linter import ( "github.com/golangci/go-tools/ssa" "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/lint/astcache" + "github.com/golangci/golangci-lint/pkg/packages" "golang.org/x/tools/go/loader" ) type Context struct { - Paths *fsutils.ProjectPaths + PkgProgram *packages.Program Cfg *config.Config Program *loader.Program SSAProgram *ssa.Program diff --git a/pkg/lint/load.go b/pkg/lint/load.go index 420f0b44cf4c..a7d54fec708d 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -4,35 +4,23 @@ import ( "context" "fmt" "go/build" + "go/parser" "os" "os/exec" + "path/filepath" "strings" "time" "github.com/golangci/go-tools/ssa" "github.com/golangci/go-tools/ssa/ssautil" "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/packages" "github.com/sirupsen/logrus" "golang.org/x/tools/go/loader" ) -type Context struct { - Paths *fsutils.ProjectPaths - Cfg *config.Config - Program *loader.Program - SSAProgram *ssa.Program - LoaderConfig *loader.Config - ASTCache *astcache.Cache - NotCompilingPackages []*loader.PackageInfo -} - -func (c *Context) Settings() *config.LintersSettings { - return &c.Cfg.LintersSettings -} - func isFullImportNeeded(linters []linter.Config) bool { for _, linter := range linters { if linter.NeedsProgramLoading() { @@ -53,7 +41,30 @@ func isSSAReprNeeded(linters []linter.Config) bool { return false } -func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *config.Run, paths *fsutils.ProjectPaths) (*loader.Program, *loader.Config, error) { +func normalizePaths(paths []string) ([]string, error) { + root, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("can't get working dir: %s", err) + } + + ret := make([]string, 0, len(paths)) + for _, p := range paths { + if filepath.IsAbs(p) { + relPath, err := filepath.Rel(root, p) + if err != nil { + return nil, fmt.Errorf("can't get relative path for path %s and root %s: %s", + p, root, err) + } + p = relPath + } + + ret = append(ret, "./"+p) + } + + return ret, nil +} + +func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *config.Run, pkgProg *packages.Program) (*loader.Program, *loader.Config, error) { if !isFullImportNeeded(linters) { return nil, nil, nil } @@ -63,13 +74,27 @@ func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *con logrus.Infof("Program loading took %s", time.Since(startedAt)) }() - bctx := build.Default - bctx.BuildTags = append(bctx.BuildTags, cfg.BuildTags...) + bctx := pkgProg.BuildContext() loadcfg := &loader.Config{ - Build: &bctx, - AllowErrors: true, // Try to analyze event partially + Build: bctx, + AllowErrors: true, // Try to analyze partially + ParserMode: parser.ParseComments, // AST will be reused by linters } - rest, err := loadcfg.FromArgs(paths.MixedPaths(), cfg.AnalyzeTests) + + var loaderArgs []string + dirs := pkgProg.Dirs() + if len(dirs) != 0 { + loaderArgs = dirs // dirs run + } else { + loaderArgs = pkgProg.Files(cfg.AnalyzeTests) // files run + } + + nLoaderArgs, err := normalizePaths(loaderArgs) + if err != nil { + return nil, nil, err + } + + rest, err := loadcfg.FromArgs(nLoaderArgs, cfg.AnalyzeTests) if err != nil { return nil, nil, fmt.Errorf("can't parepare load config with paths: %s", err) } @@ -79,7 +104,7 @@ func loadWholeAppIfNeeded(ctx context.Context, linters []linter.Config, cfg *con prog, err := loadcfg.Load() if err != nil { - return nil, nil, fmt.Errorf("can't load program from paths %v: %s", paths.MixedPaths(), err) + return nil, nil, fmt.Errorf("can't load program from paths %v: %s", loaderArgs, err) } return prog, loadcfg, nil @@ -138,6 +163,7 @@ func separateNotCompilingPackages(lintCtx *linter.Context) { } } +//nolint:gocyclo func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Config) (*linter.Context, error) { // Set GOROOT to have working cross-compilation: cross-compiled binaries // have invalid GOROOT. XXX: can't use runtime.GOROOT(). @@ -147,19 +173,25 @@ func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Confi } os.Setenv("GOROOT", goroot) build.Default.GOROOT = goroot - logrus.Infof("set GOROOT=%q", goroot) args := cfg.Run.Args if len(args) == 0 { args = []string{"./..."} } - paths, err := fsutils.GetPathsForAnalysis(ctx, args, cfg.Run.AnalyzeTests, cfg.Run.SkipDirs) + skipDirs := append([]string{}, packages.StdExcludeDirRegexps...) + skipDirs = append(skipDirs, cfg.Run.SkipDirs...) + r, err := packages.NewResolver(cfg.Run.BuildTags, skipDirs) if err != nil { return nil, err } - prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, paths) + pkgProg, err := r.Resolve(args...) + if err != nil { + return nil, err + } + + prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, pkgProg) if err != nil { return nil, err } @@ -172,15 +204,15 @@ func LoadContext(ctx context.Context, linters []linter.Config, cfg *config.Confi var astCache *astcache.Cache if prog != nil { astCache, err = astcache.LoadFromProgram(prog) - if err != nil { - return nil, err - } } else { - astCache = astcache.LoadFromFiles(paths.Files) + astCache, err = astcache.LoadFromFiles(pkgProg.Files(cfg.Run.AnalyzeTests)) + } + if err != nil { + return nil, err } ret := &linter.Context{ - Paths: paths, + PkgProgram: pkgProg, Cfg: cfg, Program: prog, SSAProgram: ssaProg, diff --git a/pkg/lint/load_test.go b/pkg/lint/load_test.go index 099d028abacd..ad231a83b17c 100644 --- a/pkg/lint/load_test.go +++ b/pkg/lint/load_test.go @@ -5,9 +5,9 @@ import ( "testing" "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/packages" "github.com/stretchr/testify/assert" ) @@ -19,24 +19,30 @@ func TestASTCacheLoading(t *testing.T) { inputPaths := []string{"./...", "./", "./load.go", "load.go"} for _, inputPath := range inputPaths { - paths, err := fsutils.GetPathsForAnalysis(ctx, []string{inputPath}, true, nil) + r, err := packages.NewResolver(nil, nil) assert.NoError(t, err) - assert.NotEmpty(t, paths.Files) + + pkgProg, err := r.Resolve(inputPath) + assert.NoError(t, err) + + assert.NoError(t, err) + assert.NotEmpty(t, pkgProg.Files(true)) prog, _, err := loadWholeAppIfNeeded(ctx, linters, &config.Run{ AnalyzeTests: true, - }, paths) + }, pkgProg) assert.NoError(t, err) astCacheFromProg, err := astcache.LoadFromProgram(prog) assert.NoError(t, err) - astCacheFromFiles := astcache.LoadFromFiles(paths.Files) + astCacheFromFiles, err := astcache.LoadFromFiles(pkgProg.Files(true)) + assert.NoError(t, err) filesFromProg := astCacheFromProg.GetAllValidFiles() filesFromFiles := astCacheFromFiles.GetAllValidFiles() if len(filesFromProg) != len(filesFromFiles) { - t.Logf("files: %s", paths.Files) + t.Logf("files: %s", pkgProg.Files(true)) t.Logf("from prog:") for _, f := range filesFromProg { t.Logf("%+v", *f) @@ -48,7 +54,7 @@ func TestASTCacheLoading(t *testing.T) { t.Fatalf("lengths differ") } - if len(filesFromProg) != len(paths.Files) { + if len(filesFromProg) != len(pkgProg.Files(true)) { t.Fatalf("filesFromProg differ from path.Files") } } diff --git a/pkg/packages/exclude.go b/pkg/packages/exclude.go new file mode 100644 index 000000000000..be57ebd92fd4 --- /dev/null +++ b/pkg/packages/exclude.go @@ -0,0 +1,8 @@ +package packages + +var StdExcludeDirRegexps = []string{ + "vendor$", "third_party$", + "testdata$", "examples$", + "Godeps$", + "builtin$", +} diff --git a/pkg/packages/package.go b/pkg/packages/package.go new file mode 100644 index 000000000000..8dfae0ff4a6c --- /dev/null +++ b/pkg/packages/package.go @@ -0,0 +1,28 @@ +package packages + +import ( + "go/build" + "path/filepath" +) + +type Package struct { + bp *build.Package + + isFake bool +} + +func (pkg *Package) Files(includeTest bool) []string { + pkgFiles := append([]string{}, pkg.bp.GoFiles...) + + // TODO: add cgo files + if includeTest { + pkgFiles = append(pkgFiles, pkg.bp.TestGoFiles...) + pkgFiles = append(pkgFiles, pkg.bp.XTestGoFiles...) + } + + for i, f := range pkgFiles { + pkgFiles[i] = filepath.Join(pkg.bp.Dir, f) + } + + return pkgFiles +} diff --git a/pkg/packages/program.go b/pkg/packages/program.go new file mode 100644 index 000000000000..d268994446d4 --- /dev/null +++ b/pkg/packages/program.go @@ -0,0 +1,71 @@ +package packages + +import ( + "fmt" + "go/build" +) + +type Program struct { + packages []Package + + bctx build.Context +} + +func (p *Program) String() string { + files := p.Files(true) + if len(files) == 1 { + return files[0] + } + + return fmt.Sprintf("%s", p.Dirs()) +} + +func (p *Program) BuildContext() *build.Context { + return &p.bctx +} + +func (p Program) Packages() []Package { + return p.packages +} + +func (p *Program) addPackage(pkg *Package) { + packagesToAdd := []Package{*pkg} + if len(pkg.bp.XTestGoFiles) != 0 { + // create separate package because xtest files have different package name + xbp := build.Package{ + Dir: pkg.bp.Dir, + ImportPath: pkg.bp.ImportPath + "_test", + XTestGoFiles: pkg.bp.XTestGoFiles, + XTestImportPos: pkg.bp.XTestImportPos, + XTestImports: pkg.bp.XTestImports, + } + packagesToAdd = append(packagesToAdd, Package{ + bp: &xbp, + }) + pkg.bp.XTestGoFiles = nil + pkg.bp.XTestImportPos = nil + pkg.bp.XTestImports = nil + } + + p.packages = append(p.packages, packagesToAdd...) +} + +func (p *Program) Files(includeTest bool) []string { + var ret []string + for _, pkg := range p.packages { + ret = append(ret, pkg.Files(includeTest)...) + } + + return ret +} + +func (p *Program) Dirs() []string { + var ret []string + for _, pkg := range p.packages { + if !pkg.isFake { + ret = append(ret, pkg.bp.Dir) + } + } + + return ret +} diff --git a/pkg/packages/resolver.go b/pkg/packages/resolver.go new file mode 100644 index 000000000000..fcbedd9fcd88 --- /dev/null +++ b/pkg/packages/resolver.go @@ -0,0 +1,206 @@ +package packages + +import ( + "fmt" + "go/build" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/sirupsen/logrus" +) + +type Resolver struct { + excludeDirs map[string]*regexp.Regexp + buildTags []string + + skippedDirs []string +} + +func NewResolver(buildTags, excludeDirs []string) (*Resolver, error) { + excludeDirsMap := map[string]*regexp.Regexp{} + for _, dir := range excludeDirs { + re, err := regexp.Compile(dir) + if err != nil { + return nil, fmt.Errorf("can't compile regexp %q: %s", dir, err) + } + + excludeDirsMap[dir] = re + } + + return &Resolver{ + excludeDirs: excludeDirsMap, + buildTags: buildTags, + }, nil +} + +func (r Resolver) isIgnoredDir(dir string) bool { + cleanName := filepath.Clean(dir) + + dirName := filepath.Base(cleanName) + + // https://github.com/golang/dep/issues/298 + // https://github.com/tools/godep/issues/140 + if strings.HasPrefix(dirName, ".") && dirName != "." && dirName != ".." { + return true + } + if strings.HasPrefix(dirName, "_") { + return true + } + + for _, dirExludeRe := range r.excludeDirs { + if dirExludeRe.MatchString(cleanName) { + return true + } + } + + return false +} + +func (r *Resolver) resolveRecursively(root string, prog *Program) error { + // import root + if err := r.resolveDir(root, prog); err != nil { + return err + } + + fis, err := ioutil.ReadDir(root) + if err != nil { + return fmt.Errorf("can't resolve dir %s: %s", root, err) + } + // TODO: pass cached fis to build.Context + + for _, fi := range fis { + if !fi.IsDir() { + // ignore files: they were already imported by resolveDir(root) + continue + } + + subdir := filepath.Join(root, fi.Name()) + + if r.isIgnoredDir(subdir) { + r.skippedDirs = append(r.skippedDirs, subdir) + continue + } + + if err := r.resolveRecursively(subdir, prog); err != nil { + return err + } + } + + return nil +} + +func (r Resolver) resolveDir(dir string, prog *Program) error { + // TODO: fork build.Import to reuse AST parsing + bp, err := prog.bctx.ImportDir(dir, build.ImportComment|build.IgnoreVendor) + if err != nil { + if _, nogo := err.(*build.NoGoError); nogo { + // Don't complain if the failure is due to no Go source files. + return nil + } + + return fmt.Errorf("can't resolve dir %s: %s", dir, err) + } + + pkg := Package{ + bp: bp, + } + prog.addPackage(&pkg) + return nil +} + +func (r Resolver) addFakePackage(filePath string, prog *Program) { + // Don't take build tags, is it test file or not, etc + // into account. If user explicitly wants to analyze this file + // do it. + p := Package{ + bp: &build.Package{ + GoFiles: []string{filePath}, + }, + isFake: true, + } + prog.addPackage(&p) +} + +func (r Resolver) Resolve(paths ...string) (prog *Program, err error) { + startedAt := time.Now() + defer func() { + logrus.Infof("Paths resolving took %s: %s", time.Since(startedAt), prog) + }() + + if len(paths) == 0 { + return nil, fmt.Errorf("no paths are set") + } + + bctx := build.Default + bctx.BuildTags = append(bctx.BuildTags, r.buildTags...) + prog = &Program{ + bctx: bctx, + } + + root, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("can't get working dir: %s", err) + } + + for _, path := range paths { + if err := r.resolvePath(path, prog, root); err != nil { + return nil, err + } + } + + if len(r.skippedDirs) != 0 { + logrus.Infof("Skipped dirs: %s", r.skippedDirs) + } + + return prog, nil +} + +func (r *Resolver) resolvePath(path string, prog *Program, root string) error { + needRecursive := strings.HasSuffix(path, "/...") + if needRecursive { + path = filepath.Dir(path) + } + + evalPath, err := filepath.EvalSymlinks(path) + if err != nil { + return fmt.Errorf("can't eval symlinks for path %s: %s", path, err) + } + path = evalPath + + if filepath.IsAbs(path) { + var relPath string + relPath, err = filepath.Rel(root, path) + if err != nil { + return fmt.Errorf("can't get relative path for path %s and root %s: %s", + path, root, err) + } + path = relPath + } + + if needRecursive { + if err = r.resolveRecursively(path, prog); err != nil { + return fmt.Errorf("can't recursively resolve %s: %s", path, err) + } + + return nil + } + + fi, err := os.Stat(path) + if err != nil { + return fmt.Errorf("can't find path %s: %s", path, err) + } + + if fi.IsDir() { + if err := r.resolveDir(path, prog); err != nil { + return fmt.Errorf("can't resolve dir %s: %s", path, err) + } + return nil + } + + r.addFakePackage(path, prog) + return nil +} diff --git a/pkg/fsutils/path_resolver_test.go b/pkg/packages/resolver_test.go similarity index 62% rename from pkg/fsutils/path_resolver_test.go rename to pkg/packages/resolver_test.go index ae3d383afd24..f5adf45ddd7d 100644 --- a/pkg/fsutils/path_resolver_test.go +++ b/pkg/packages/resolver_test.go @@ -1,12 +1,14 @@ -package fsutils +package packages_test import ( "io/ioutil" "os" "path/filepath" + "sort" "strings" "testing" + "github.com/golangci/golangci-lint/pkg/packages" "github.com/stretchr/testify/assert" ) @@ -42,7 +44,8 @@ func prepareFS(t *testing.T, paths ...string) *fsPreparer { continue } - err = ioutil.WriteFile(p, []byte("test"), os.ModePerm) + goFile := "package p\n" + err = ioutil.WriteFile(p, []byte(goFile), os.ModePerm) assert.NoError(t, err) } @@ -53,24 +56,19 @@ func prepareFS(t *testing.T, paths ...string) *fsPreparer { } } -func newPR(t *testing.T) *PathResolver { - pr, err := NewPathResolver([]string{}, []string{}, false) +func newTestResolver(t *testing.T, excludeDirs []string) *packages.Resolver { + r, err := packages.NewResolver(nil, excludeDirs) assert.NoError(t, err) - return pr -} - -func TestPathResolverNoPaths(t *testing.T) { - _, err := newPR(t).Resolve() - assert.EqualError(t, err, "no paths are set") + return r } func TestPathResolverNotExistingPath(t *testing.T) { fp := prepareFS(t) defer fp.clean() - _, err := newPR(t).Resolve("a") - assert.EqualError(t, err, "can't find path a: stat a: no such file or directory") + _, err := newTestResolver(t, nil).Resolve("a") + assert.EqualError(t, err, "can't eval symlinks for path a: lstat a: no such file or directory") } func TestPathResolverCommonCases(t *testing.T) { @@ -103,12 +101,31 @@ func TestPathResolverCommonCases(t *testing.T) { resolve: []string{"./..."}, }, { - name: "vendor implicitely resolved", + name: "nested vendor is excluded", + prepare: []string{"d/vendor/a.go"}, + resolve: []string{"./..."}, + }, + { + name: "vendor dir is excluded by regexp, not the exact match", + prepare: []string{"vendors/a.go", "novendor/b.go"}, + resolve: []string{"./..."}, + expDirs: []string{"vendors"}, + expFiles: []string{"vendors/a.go"}, + }, + { + name: "vendor explicitly resolved", prepare: []string{"vendor/a.go"}, resolve: []string{"./vendor"}, expDirs: []string{"vendor"}, expFiles: []string{"vendor/a.go"}, }, + { + name: "nested vendor explicitly resolved", + prepare: []string{"d/vendor/a.go"}, + resolve: []string{"d/vendor"}, + expDirs: []string{"d/vendor"}, + expFiles: []string{"d/vendor/a.go"}, + }, { name: "extensions filter recursively", prepare: []string{"a/b.go", "a/c.txt", "d.go", "e.csv"}, @@ -131,10 +148,10 @@ func TestPathResolverCommonCases(t *testing.T) { expFiles: []string{"a/c.go"}, }, { - name: "implicitely resolved files", + name: "explicitly resolved files", prepare: []string{"a/b/c.go", "a/d.txt"}, resolve: []string{"./a/...", "a/d.txt"}, - expDirs: []string{"a", "a/b"}, + expDirs: []string{"a/b"}, expFiles: []string{"a/b/c.go", "a/d.txt"}, }, { @@ -149,6 +166,32 @@ func TestPathResolverCommonCases(t *testing.T) { expDirs: []string{"ok"}, expFiles: []string{"ok/b.go"}, }, + { + name: "exclude path, not name", + prepare: []string{"ex/clude/me/a.go", "c/d.go"}, + resolve: []string{"./..."}, + expDirs: []string{"c"}, + expFiles: []string{"c/d.go"}, + }, + { + name: "exclude partial path", + prepare: []string{"prefix/ex/clude/me/a.go", "prefix/ex/clude/me/subdir/c.go", "prefix/b.go"}, + resolve: []string{"./..."}, + expDirs: []string{"prefix"}, + expFiles: []string{"prefix/b.go"}, + }, + { + name: "don't exclude file instead of dir", + prepare: []string{"a/exclude.go"}, + resolve: []string{"a"}, + expDirs: []string{"a"}, + expFiles: []string{"a/exclude.go"}, + }, + { + name: "don't exclude file instead of dir: check dir is excluded", + prepare: []string{"a/exclude.go/b.go"}, + resolve: []string{"a/..."}, + }, { name: "ignore _*", prepare: []string{"_any/a.go"}, @@ -191,9 +234,11 @@ func TestPathResolverCommonCases(t *testing.T) { expFiles: []string{"a/c/d.go", "e.go"}, }, { - name: "vendor dir is excluded by regexp, not the exact match", - prepare: []string{"vendors/a.go", "novendor/b.go"}, - resolve: []string{"./..."}, + name: "resolve absolute paths", + prepare: []string{"a/b.go", "a/c.txt", "d.go", "e.csv"}, + resolve: []string{"${CWD}/..."}, + expDirs: []string{".", "a"}, + expFiles: []string{"a/b.go", "d.go"}, }, } @@ -202,22 +247,34 @@ func TestPathResolverCommonCases(t *testing.T) { fp := prepareFS(t, tc.prepare...) defer fp.clean() - pr, err := NewPathResolver([]string{"vendor"}, []string{".go"}, tc.includeTests) - assert.NoError(t, err) + for i, rp := range tc.resolve { + tc.resolve[i] = strings.Replace(rp, "${CWD}", fp.root, -1) + } - res, err := pr.Resolve(tc.resolve...) + r := newTestResolver(t, []string{"vendor$", "ex/clude/me", "exclude"}) + + prog, err := r.Resolve(tc.resolve...) assert.NoError(t, err) + assert.NotNil(t, prog) + + progFiles := prog.Files(tc.includeTests) + sort.StringSlice(progFiles).Sort() + sort.StringSlice(tc.expFiles).Sort() + + progDirs := prog.Dirs() + sort.StringSlice(progDirs).Sort() + sort.StringSlice(tc.expDirs).Sort() if tc.expFiles == nil { - assert.Empty(t, res.files) + assert.Empty(t, progFiles) } else { - assert.Equal(t, tc.expFiles, res.files) + assert.Equal(t, tc.expFiles, progFiles, "files") } if tc.expDirs == nil { - assert.Empty(t, res.dirs) + assert.Empty(t, progDirs) } else { - assert.Equal(t, tc.expDirs, res.dirs) + assert.Equal(t, tc.expDirs, progDirs, "dirs") } }) } diff --git a/pkg/result/processors/skip_files.go b/pkg/result/processors/skip_files.go index 5b9fa652fa37..92fd1a29be33 100644 --- a/pkg/result/processors/skip_files.go +++ b/pkg/result/processors/skip_files.go @@ -2,7 +2,6 @@ package processors import ( "fmt" - "path/filepath" "regexp" "github.com/golangci/golangci-lint/pkg/result" @@ -39,9 +38,8 @@ func (p SkipFiles) Process(issues []result.Issue) ([]result.Issue, error) { } return filterIssues(issues, func(i *result.Issue) bool { - fileName := filepath.Base(i.FilePath()) for _, p := range p.patterns { - if p.MatchString(fileName) { + if p.MatchString(i.FilePath()) { return false } } diff --git a/pkg/result/processors/skip_files_test.go b/pkg/result/processors/skip_files_test.go index b416cab9187b..df557a055fa1 100644 --- a/pkg/result/processors/skip_files_test.go +++ b/pkg/result/processors/skip_files_test.go @@ -23,17 +23,23 @@ func newTestSkipFiles(t *testing.T, patterns ...string) *SkipFiles { } func TestSkipFiles(t *testing.T) { - p := newTestSkipFiles(t) - processAssertSame(t, p, newFileIssue("any.go")) + processAssertSame(t, newTestSkipFiles(t), newFileIssue("any.go")) - p = newTestSkipFiles(t, "file") - processAssertEmpty(t, p, + processAssertEmpty(t, newTestSkipFiles(t, "file"), newFileIssue("file.go"), newFileIssue("file"), newFileIssue("nofile.go")) - p = newTestSkipFiles(t, ".*") - processAssertEmpty(t, p, newFileIssue("any.go")) + processAssertEmpty(t, newTestSkipFiles(t, ".*"), newFileIssue("any.go")) + + processAssertEmpty(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/c.go")) + processAssertSame(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/d.go")) + + processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.pb.go")) + processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.go")) + + processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.pb.go")) + processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.go")) } func TestSkipFilesInvalidPattern(t *testing.T) { diff --git a/pkg/timeutils/track.go b/pkg/timeutils/track.go new file mode 100644 index 000000000000..279e4e7b4d65 --- /dev/null +++ b/pkg/timeutils/track.go @@ -0,0 +1,12 @@ +package timeutils + +import ( + "fmt" + "time" + + "github.com/sirupsen/logrus" +) + +func Track(from time.Time, format string, args ...interface{}) { + logrus.Infof("%s took %s", fmt.Sprintf(format, args...), time.Since(from)) +} diff --git a/test/linters_test.go b/test/linters_test.go index 3488ac2414cb..6fb8d3bba853 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -2,9 +2,11 @@ package test import ( "bytes" + "io/ioutil" "os/exec" "path/filepath" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -21,9 +23,9 @@ func runGoErrchk(c *exec.Cmd, t *testing.T) { const testdataDir = "testdata" const binName = "golangci-lint" -func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { - t.Log(filepath.Join(testdataDir, "*.go")) - sources, err := filepath.Glob(filepath.Join(testdataDir, "*.go")) +func testSourcesFromDir(t *testing.T, dir string) { + t.Log(filepath.Join(dir, "*.go")) + sources, err := filepath.Glob(filepath.Join(dir, "*.go")) assert.NoError(t, err) assert.NotEmpty(t, sources) @@ -38,20 +40,64 @@ func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { } } +func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { + testSourcesFromDir(t, testdataDir) +} + +func TestTypecheck(t *testing.T) { + testSourcesFromDir(t, filepath.Join(testdataDir, "notcompiles")) +} + func testOneSource(t *testing.T, sourcePath string) { goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk") - cmd := exec.Command(goErrchkBin, binName, "run", + args := []string{ + binName, "run", "--no-config", - "--enable-all", - "--dupl.threshold=20", - "--gocyclo.min-complexity=20", + "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - "--print-welcome=false", - "--govet.check-shadowing=true", - "--depguard.include-go-root", - "--depguard.packages='log'", - sourcePath) - runGoErrchk(cmd, t) + } + + for _, addArg := range []string{"", "-Etypecheck"} { + caseArgs := append([]string{}, args...) + caseArgs = append(caseArgs, getAdditionalArgs(t, sourcePath)...) + if addArg != "" { + caseArgs = append(caseArgs, addArg) + } + + caseArgs = append(caseArgs, sourcePath) + + cmd := exec.Command(goErrchkBin, caseArgs...) + t.Log(caseArgs) + runGoErrchk(cmd, t) + } +} + +func getAdditionalArgs(t *testing.T, sourcePath string) []string { + data, err := ioutil.ReadFile(sourcePath) + assert.NoError(t, err) + + lines := strings.SplitN(string(data), "\n", 2) + firstLine := lines[0] + + parts := strings.Split(firstLine, "args:") + if len(parts) == 1 { + return nil + } + + return strings.Split(parts[len(parts)-1], " ") +} + +func TestGolintConsumesXTestFiles(t *testing.T) { + dir := filepath.Join(testdataDir, "withxtest") + const expIssue = "if block ends with a return statement, so drop this else and outdent its block" + + out, ec := runGolangciLint(t, "--no-config", "--disable-all", "-Egolint", dir) + assert.Equal(t, 1, ec) + assert.Contains(t, out, expIssue) + + out, ec = runGolangciLint(t, "--no-config", "--disable-all", "-Egolint", filepath.Join(dir, "p_test.go")) + assert.Equal(t, 1, ec) + assert.Contains(t, out, expIssue) } diff --git a/test/run_test.go b/test/run_test.go index b1af7ed8cae6..5f2b7a33979a 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -37,6 +37,11 @@ func TestCongratsMessageIfNoIssues(t *testing.T) { checkNoIssuesRun(t, out, exitCode) } +func TestSymlinkLoop(t *testing.T) { + out, exitCode := runGolangciLint(t, filepath.Join(testdataDir, "symlink_loop", "...")) + checkNoIssuesRun(t, out, exitCode) +} + func TestDeadline(t *testing.T) { out, exitCode := runGolangciLint(t, "--deadline=1ms", "../...") assert.Equal(t, 4, exitCode) diff --git a/test/testdata/deadcode.go b/test/testdata/deadcode.go index ffa7c1d7052a..a4172e6a0078 100644 --- a/test/testdata/deadcode.go +++ b/test/testdata/deadcode.go @@ -1,13 +1,14 @@ +// args: -Edeadcode package testdata var y int -var unused int // nolint:megacheck // ERROR "`unused` is unused" +var unused int // ERROR "`unused` is unused" func f(x int) { } -func g(x int) { // nolint:megacheck // ERROR "`g` is unused" +func g(x int) { // ERROR "`g` is unused" } func H(x int) { diff --git a/test/testdata/depguard.go b/test/testdata/depguard.go index 9ed1640ab9bb..11efdce7bd82 100644 --- a/test/testdata/depguard.go +++ b/test/testdata/depguard.go @@ -1,3 +1,4 @@ +// args: -Edepguard --depguard.include-go-root --depguard.packages='log' package testdata import ( diff --git a/test/testdata/dupl.go b/test/testdata/dupl.go index 9fe413ee67ad..9e44ede4c150 100644 --- a/test/testdata/dupl.go +++ b/test/testdata/dupl.go @@ -1,3 +1,4 @@ +// args: -Edupl --dupl.threshold=20 package testdata type DuplLogger struct{} @@ -9,7 +10,7 @@ func (DuplLogger) level() int { func (DuplLogger) Debug(args ...interface{}) {} func (DuplLogger) Info(args ...interface{}) {} -func (logger *DuplLogger) First(args ...interface{}) { // ERROR "12-21 lines are duplicate of `testdata/dupl.go:23-32`" +func (logger *DuplLogger) First(args ...interface{}) { // ERROR "13-22 lines are duplicate of `testdata/dupl.go:24-33`" if logger.level() >= 0 { logger.Debug(args...) logger.Debug(args...) @@ -20,7 +21,7 @@ func (logger *DuplLogger) First(args ...interface{}) { // ERROR "12-21 lines are } } -func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "23-32 lines are duplicate of `testdata/dupl.go:12-21`" +func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "24-33 lines are duplicate of `testdata/dupl.go:13-22`" if logger.level() >= 1 { logger.Info(args...) logger.Info(args...) diff --git a/test/testdata/errcheck.go b/test/testdata/errcheck.go index 2adf6a0be211..1411c12b1c1d 100644 --- a/test/testdata/errcheck.go +++ b/test/testdata/errcheck.go @@ -1,3 +1,4 @@ +// args: -Eerrcheck package testdata import ( diff --git a/test/testdata/gas.go b/test/testdata/gas.go index 6ba96de3bd6c..f446fbd52d9b 100644 --- a/test/testdata/gas.go +++ b/test/testdata/gas.go @@ -1,8 +1,9 @@ +// args: -Egas package testdata import ( "crypto/md5" // ERROR "G501: Blacklisted import crypto/md5: weak cryptographic primitive" - "log" // nolint:depguard + "log" ) func Gas() { diff --git a/test/testdata/goconst.go b/test/testdata/goconst.go index 6d881321f543..b022b17f9b70 100644 --- a/test/testdata/goconst.go +++ b/test/testdata/goconst.go @@ -1,8 +1,9 @@ +// args: -Egoconst package testdata import "fmt" -func GoconstA() { // nolint:dupl +func GoconstA() { a := "needconst" // ERROR "string `needconst` has 5 occurrences, make it a constant" fmt.Print(a) b := "needconst" @@ -20,7 +21,7 @@ func GoconstB() { const AlreadyHasConst = "alreadyhasconst" -func GoconstC() { // nolint:dupl +func GoconstC() { a := "alreadyhasconst" // ERROR "string `alreadyhasconst` has 3 occurrences, but such constant `AlreadyHasConst` already exists" fmt.Print(a) b := "alreadyhasconst" diff --git a/test/testdata/gocyclo.go b/test/testdata/gocyclo.go index b1a9983ba73c..0c93dce5fe4a 100644 --- a/test/testdata/gocyclo.go +++ b/test/testdata/gocyclo.go @@ -1,15 +1,16 @@ +// args: -Egocyclo --gocyclo.min-complexity=20 package testdata func GocycloBigComplexity(s string) { // ERROR "cyclomatic complexity .* of func .* is high .*" - if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { // nolint:dupl + if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { return } - if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { // nolint:dupl + if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { return } - if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { // nolint:dupl + if s == "1" || s == "2" || s == "3" || s == "4" || s == "5" || s == "6" || s == "7" { return } } diff --git a/test/testdata/gofmt.go b/test/testdata/gofmt.go index c0370875cc9b..382ccc368013 100644 --- a/test/testdata/gofmt.go +++ b/test/testdata/gofmt.go @@ -1,8 +1,9 @@ +// args: -Egofmt package testdata import "fmt" func GofmtNotSimplified() { var x []string - fmt.Print(x[1:len(x)]) // nolint:megacheck // ERROR "File is not gofmt-ed with -s" + fmt.Print(x[1:len(x)]) // ERROR "File is not gofmt-ed with -s" } diff --git a/test/testdata/goimports.go b/test/testdata/goimports.go index c530933f85e3..e4c92eb3a8c1 100644 --- a/test/testdata/goimports.go +++ b/test/testdata/goimports.go @@ -1,3 +1,4 @@ +// args: -Egoimports package testdata import ( diff --git a/test/testdata/golint.go b/test/testdata/golint.go index 0a99f9787fb3..14e0e0f6d0dd 100644 --- a/test/testdata/golint.go +++ b/test/testdata/golint.go @@ -1,3 +1,4 @@ +// args: -Egolint package testdata var Go_lint string // ERROR "don't use underscores in Go names; var Go_lint should be GoLint" @@ -11,7 +12,7 @@ type ExportedStructWithNoComment struct{} type ExportedInterfaceWithNoComment interface{} -// Bad comment // ERROR "comment on exported function ExportedFuncWithBadComment should be of the form .ExportedFuncWithBadComment \.\.\.." +// Bad comment func ExportedFuncWithBadComment() {} type GolintTest struct{} diff --git a/test/testdata/govet.go b/test/testdata/govet.go index 1474f9d2ec78..e8c210e4aa8b 100644 --- a/test/testdata/govet.go +++ b/test/testdata/govet.go @@ -1,3 +1,4 @@ +// args: -Egovet --govet.check-shadowing=true package testdata import ( diff --git a/test/testdata/ineffassign.go b/test/testdata/ineffassign.go index 7b33adc85e6c..96932a3b489d 100644 --- a/test/testdata/ineffassign.go +++ b/test/testdata/ineffassign.go @@ -1,3 +1,4 @@ +// args: -Eineffassign package testdata func _() { diff --git a/test/testdata/interfacer.go b/test/testdata/interfacer.go index 1d232585b00e..e1a1334644cd 100644 --- a/test/testdata/interfacer.go +++ b/test/testdata/interfacer.go @@ -1,3 +1,4 @@ +// args: -Einterfacer package testdata import "io" diff --git a/test/testdata/maligned.go b/test/testdata/maligned.go index d77e706a4525..a6930d12e72a 100644 --- a/test/testdata/maligned.go +++ b/test/testdata/maligned.go @@ -1,3 +1,4 @@ +// args: -Emaligned package testdata type BadAlignedStruct struct { // ERROR "struct of size 24 bytes could be of size 16 bytes" diff --git a/test/testdata/megacheck.go b/test/testdata/megacheck.go index 5bd963ecddb5..b3b5d84feefb 100644 --- a/test/testdata/megacheck.go +++ b/test/testdata/megacheck.go @@ -1,6 +1,7 @@ +// args: -Emegacheck package testdata func Megacheck() { var x int - x = x // nolint:ineffassign // ERROR "self-assignment of x to x" + x = x // ERROR "self-assignment of x to x" } diff --git a/test/testdata/typecheck.go b/test/testdata/notcompiles/typecheck.go similarity index 81% rename from test/testdata/typecheck.go rename to test/testdata/notcompiles/typecheck.go index a31d55387174..6cd5d42b17fb 100644 --- a/test/testdata/typecheck.go +++ b/test/testdata/notcompiles/typecheck.go @@ -1,3 +1,4 @@ +// args: -Etypecheck package testdata fun NotCompiles() { // ERROR "expected declaration, found 'IDENT' fun" diff --git a/test/testdata/typecheck_many_issues.go b/test/testdata/notcompiles/typecheck_many_issues.go similarity index 93% rename from test/testdata/typecheck_many_issues.go rename to test/testdata/notcompiles/typecheck_many_issues.go index 9de48deb0ed8..69d09b3b8170 100644 --- a/test/testdata/typecheck_many_issues.go +++ b/test/testdata/notcompiles/typecheck_many_issues.go @@ -1,3 +1,4 @@ +// args: -Etypecheck package testdata func TypeCheckBadCalls() { diff --git a/test/testdata/structcheck.go b/test/testdata/structcheck.go index 3e1f35ea6630..e169207e4f0d 100644 --- a/test/testdata/structcheck.go +++ b/test/testdata/structcheck.go @@ -1,5 +1,6 @@ +// args: -Estructcheck package testdata -type t struct { // nolint:megacheck // ERROR "`t` is unused" - unusedField int // nolint:megacheck // ERROR "`unusedField` is unused" +type t struct { + unusedField int // ERROR "`unusedField` is unused" } diff --git a/test/testdata/symlink_loop/pkg/subpkg b/test/testdata/symlink_loop/pkg/subpkg new file mode 120000 index 000000000000..b870225aa053 --- /dev/null +++ b/test/testdata/symlink_loop/pkg/subpkg @@ -0,0 +1 @@ +../ \ No newline at end of file diff --git a/test/testdata/symlink_loop/realpkg/p.go b/test/testdata/symlink_loop/realpkg/p.go new file mode 100644 index 000000000000..c89cd18d0fe7 --- /dev/null +++ b/test/testdata/symlink_loop/realpkg/p.go @@ -0,0 +1 @@ +package p diff --git a/test/testdata/unconvert.go b/test/testdata/unconvert.go index 97399673cef3..c5feab2ffad4 100644 --- a/test/testdata/unconvert.go +++ b/test/testdata/unconvert.go @@ -1,6 +1,7 @@ +// args: -Eunconvert package testdata -import "log" // nolint:depguard +import "log" func Unconvert() { a := 1 diff --git a/test/testdata/varcheck.go b/test/testdata/varcheck.go index 914dd7b66378..6aa9aac12c76 100644 --- a/test/testdata/varcheck.go +++ b/test/testdata/varcheck.go @@ -1,3 +1,4 @@ +// args: -Evarcheck package testdata -var v string // nolint:megacheck // ERROR "`v` is unused" +var v string // ERROR "`v` is unused" diff --git a/test/testdata/withxtest/p.go b/test/testdata/withxtest/p.go new file mode 100644 index 000000000000..c89cd18d0fe7 --- /dev/null +++ b/test/testdata/withxtest/p.go @@ -0,0 +1 @@ +package p diff --git a/test/testdata/withxtest/p_test.go b/test/testdata/withxtest/p_test.go new file mode 100644 index 000000000000..94031e8e9148 --- /dev/null +++ b/test/testdata/withxtest/p_test.go @@ -0,0 +1,11 @@ +package p_test + +import "fmt" + +func WithGolintIssues(b bool) { //nolint:megacheck + if b { + return + } else { + fmt.Print("1") + } +} diff --git a/vendor/github.com/golangci/goconst/parser.go b/vendor/github.com/golangci/goconst/parser.go index f1bc4b28e24c..ab5f99a5073d 100644 --- a/vendor/github.com/golangci/goconst/parser.go +++ b/vendor/github.com/golangci/goconst/parser.go @@ -142,26 +142,27 @@ type Issue struct { MatchingConst string } -func Run(files []string, matchWithConstants bool, minStringLength, minOccurrences int) ([]Issue, error) { - p := New("", "", false, matchWithConstants, false, minStringLength) +type Config struct { + MatchWithConstants bool + MinStringLength int + MinOccurrences int +} + +func Run(files []*ast.File, fset *token.FileSet, cfg *Config) ([]Issue, error) { + p := New("", "", false, cfg.MatchWithConstants, false, cfg.MinStringLength) var issues []Issue - fset := token.NewFileSet() - for _, path := range files { - f, err := parser.ParseFile(fset, path, nil, 0) - if err != nil { - return nil, err - } + for _, f := range files { ast.Walk(&treeVisitor{ fileSet: fset, packageName: "", - fileName: path, + fileName: "", p: p, }, f) } for str, item := range p.strs { // Filter out items whose occurrences don't match the min value - if len(item) < minOccurrences { + if len(item) < cfg.MinOccurrences { delete(p.strs, str) } } diff --git a/vendor/github.com/golang/lint/.travis.yml b/vendor/github.com/golangci/lint-1/.travis.yml similarity index 100% rename from vendor/github.com/golang/lint/.travis.yml rename to vendor/github.com/golangci/lint-1/.travis.yml diff --git a/vendor/github.com/golang/lint/CONTRIBUTING.md b/vendor/github.com/golangci/lint-1/CONTRIBUTING.md similarity index 100% rename from vendor/github.com/golang/lint/CONTRIBUTING.md rename to vendor/github.com/golangci/lint-1/CONTRIBUTING.md diff --git a/vendor/github.com/golang/lint/LICENSE b/vendor/github.com/golangci/lint-1/LICENSE similarity index 100% rename from vendor/github.com/golang/lint/LICENSE rename to vendor/github.com/golangci/lint-1/LICENSE diff --git a/vendor/github.com/golang/lint/README.md b/vendor/github.com/golangci/lint-1/README.md similarity index 100% rename from vendor/github.com/golang/lint/README.md rename to vendor/github.com/golangci/lint-1/README.md diff --git a/vendor/github.com/golang/lint/lint.go b/vendor/github.com/golangci/lint-1/lint.go similarity index 97% rename from vendor/github.com/golang/lint/lint.go rename to vendor/github.com/golangci/lint-1/lint.go index 46bd45f4e2c5..0668635f30dc 100644 --- a/vendor/github.com/golang/lint/lint.go +++ b/vendor/github.com/golangci/lint-1/lint.go @@ -16,6 +16,7 @@ import ( "go/printer" "go/token" "go/types" + "io/ioutil" "regexp" "sort" "strconv" @@ -115,6 +116,46 @@ func (l *Linter) LintFiles(files map[string][]byte) ([]Problem, error) { return pkg.lint(), nil } +// LintFiles lints a set of files of a single package. +// The argument is a map of filename to source. +func (l *Linter) LintASTFiles(files []*ast.File, fset *token.FileSet) ([]Problem, error) { + pkg := &pkg{ + fset: fset, + files: make(map[string]*file), + } + var pkgName string + for _, f := range files { + filename := fset.Position(f.Pos()).Filename + if filename == "" { + return nil, fmt.Errorf("no file name for file %+v", f) + } + + if pkgName == "" { + pkgName = f.Name.Name + } else if f.Name.Name != pkgName { + return nil, fmt.Errorf("%s is in package %s, not %s", filename, f.Name.Name, pkgName) + } + + // TODO: reuse golangci-lint lines cache + src, err := ioutil.ReadFile(filename) + if err != nil { + return nil, fmt.Errorf("can't read file %s: %s", filename, err) + } + + pkg.files[filename] = &file{ + pkg: pkg, + f: f, + fset: pkg.fset, + src: src, + filename: filename, + } + } + if len(pkg.files) == 0 { + return nil, nil + } + return pkg.lint(), nil +} + var ( genHdr = []byte("// Code generated ") genFtr = []byte(" DO NOT EDIT.")