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
This commit is contained in:
Denis Isaev 2018-06-10 17:10:56 +03:00
parent f5a9bbb140
commit 2b587b63d6
No known key found for this signature in database
GPG Key ID: A36A0EC8E27A1A01
59 changed files with 755 additions and 512 deletions

@ -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

16
Gopkg.lock generated

@ -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

@ -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

@ -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

@ -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

@ -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",
},

@ -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()

@ -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)
}
}
}

@ -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
}

@ -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
}

@ -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 {

@ -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.LintASTFiles(files, fset)
if err != nil {
return nil, fmt.Errorf("can't lint %d files: %s", len(files), err)
}
l := new(lintAPI.Linter)
ps, err := l.LintFiles(files)
if err != nil {
return nil, fmt.Errorf("can't lint files %s: %s", filenames, 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{

@ -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
}

@ -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
}

@ -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
}

@ -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
}

@ -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

@ -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,

@ -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")
}
}

8
pkg/packages/exclude.go Normal file

@ -0,0 +1,8 @@
package packages
var StdExcludeDirRegexps = []string{
"vendor$", "third_party$",
"testdata$", "examples$",
"Godeps$",
"builtin$",
}

28
pkg/packages/package.go Normal file

@ -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
}

71
pkg/packages/program.go Normal file

@ -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
}

206
pkg/packages/resolver.go Normal file

@ -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
}

@ -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")
}
})
}

@ -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
}
}

@ -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) {

12
pkg/timeutils/track.go Normal file

@ -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))
}

@ -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)
}

@ -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)

@ -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) {

@ -1,3 +1,4 @@
// args: -Edepguard --depguard.include-go-root --depguard.packages='log'
package testdata
import (

@ -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...)

@ -1,3 +1,4 @@
// args: -Eerrcheck
package testdata
import (

@ -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() {

@ -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"

@ -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
}
}

@ -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"
}

@ -1,3 +1,4 @@
// args: -Egoimports
package testdata
import (

@ -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{}

@ -1,3 +1,4 @@
// args: -Egovet --govet.check-shadowing=true
package testdata
import (

@ -1,3 +1,4 @@
// args: -Eineffassign
package testdata
func _() {

@ -1,3 +1,4 @@
// args: -Einterfacer
package testdata
import "io"

@ -1,3 +1,4 @@
// args: -Emaligned
package testdata
type BadAlignedStruct struct { // ERROR "struct of size 24 bytes could be of size 16 bytes"

@ -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"
}

@ -1,3 +1,4 @@
// args: -Etypecheck
package testdata
fun NotCompiles() { // ERROR "expected declaration, found 'IDENT' fun"

@ -1,3 +1,4 @@
// args: -Etypecheck
package testdata
func TypeCheckBadCalls() {

@ -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"
}

1
test/testdata/symlink_loop/pkg/subpkg vendored Symbolic link

@ -0,0 +1 @@
../

@ -0,0 +1 @@
package p

@ -1,6 +1,7 @@
// args: -Eunconvert
package testdata
import "log" // nolint:depguard
import "log"
func Unconvert() {
a := 1

@ -1,3 +1,4 @@
// args: -Evarcheck
package testdata
var v string // nolint:megacheck // ERROR "`v` is unused"
var v string // ERROR "`v` is unused"

1
test/testdata/withxtest/p.go vendored Normal file

@ -0,0 +1 @@
package p

11
test/testdata/withxtest/p_test.go vendored Normal file

@ -0,0 +1,11 @@
package p_test
import "fmt"
func WithGolintIssues(b bool) { //nolint:megacheck
if b {
return
} else {
fmt.Print("1")
}
}

@ -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)
}
}

@ -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.")