diff --git a/Gopkg.lock b/Gopkg.lock index bef8adcf..a772215d 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -136,7 +136,7 @@ "lib/whitelist", ] pruneopts = "UT" - revision = "54e733a64097854cf4d8fa80ccf48012a487ae1a" + revision = "e3b43b6cb1916e0000f244f26ee752733e544429" [[projects]] branch = "master" diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index 75cca99b..d2b880e7 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "sync" ) func IsDir(filename string) bool { @@ -11,15 +12,74 @@ func IsDir(filename string) bool { return err == nil && fi.IsDir() } +var cachedWd string +var cachedWdError error +var getWdOnce sync.Once +var useCache = true + +func UseWdCache(use bool) { + useCache = use +} + +func Getwd() (string, error) { + if !useCache { // for tests + return os.Getwd() + } + + getWdOnce.Do(func() { + cachedWd, cachedWdError = os.Getwd() + if cachedWdError != nil { + return + } + + evaledWd, err := EvalSymlinks(cachedWd) + if err != nil { + cachedWd, cachedWdError = "", fmt.Errorf("can't eval symlinks on wd %s: %s", cachedWd, err) + return + } + + cachedWd = evaledWd + }) + + return cachedWd, cachedWdError +} + +var evalSymlinkCache sync.Map + +type evalSymlinkRes struct { + path string + err error +} + +func EvalSymlinks(path string) (string, error) { + r, ok := evalSymlinkCache.Load(path) + if ok { + er := r.(evalSymlinkRes) + return er.path, er.err + } + + var er evalSymlinkRes + er.path, er.err = filepath.EvalSymlinks(path) + evalSymlinkCache.Store(path, er) + + return er.path, er.err +} + func ShortestRelPath(path string, wd string) (string, error) { if wd == "" { // get it if user don't have cached working dir var err error - wd, err = os.Getwd() + wd, err = Getwd() if err != nil { return "", fmt.Errorf("can't get working directory: %s", err) } } + evaledPath, err := EvalSymlinks(path) + if err != nil { + return "", fmt.Errorf("can't eval symlinks for path %s: %s", path, err) + } + path = evaledPath + // make path absolute and then relative to be able to fix this case: // we'are in /test dir, we want to normalize ../test, and have file file.go in this dir; // it must have normalized path file.go, not ../test/file.go, diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 80a86718..257cf1a4 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -10,11 +10,11 @@ import ( "strings" "time" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" - "github.com/golangci/golangci-lint/pkg/result/processors" "github.com/golangci/golangci-lint/pkg/timeutils" govetAPI "github.com/golangci/govet" ) @@ -82,7 +82,7 @@ func (g Govet) runOnInstalledPackages(ctx context.Context, lintCtx *linter.Conte continue } issues, err := govetAPI.Analyze(astFiles, fset, nil, - lintCtx.Settings().Govet.CheckShadowing) + lintCtx.Settings().Govet.CheckShadowing, getPath) if err != nil { return nil, err } @@ -220,7 +220,7 @@ func runGoCommand(ctx context.Context, log logutils.Log, args ...string) error { func filterFiles(files []*ast.File, fset *token.FileSet) []*ast.File { newFiles := make([]*ast.File, 0, len(files)) for _, f := range files { - if !processors.IsCgoFilename(fset.Position(f.Pos()).Filename) { + if !goutils.IsCgoFilename(fset.Position(f.Pos()).Filename) { newFiles = append(newFiles, f) } } @@ -238,7 +238,7 @@ func (g Govet) runOnSourcePackages(_ context.Context, lintCtx *linter.Context) ( filteredFiles := filterFiles(pkg.Files, lintCtx.Program.Fset) issues, err := govetAPI.Analyze(filteredFiles, lintCtx.Program.Fset, pkg, - lintCtx.Settings().Govet.CheckShadowing) + lintCtx.Settings().Govet.CheckShadowing, getPath) if err != nil { return nil, err } @@ -247,3 +247,7 @@ func (g Govet) runOnSourcePackages(_ context.Context, lintCtx *linter.Context) ( return govetIssues, nil } + +func getPath(f *ast.File, fset *token.FileSet) (string, error) { + return fsutils.ShortestRelPath(fset.Position(f.Pos()).Filename, "") +} diff --git a/pkg/goutils/goutils.go b/pkg/goutils/goutils.go index 7bd5bb3a..9e9d76ca 100644 --- a/pkg/goutils/goutils.go +++ b/pkg/goutils/goutils.go @@ -4,8 +4,11 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "sync" + + "github.com/golangci/golangci-lint/pkg/fsutils" ) var discoverGoRootOnce sync.Once @@ -40,7 +43,7 @@ func InGoRoot() (bool, error) { return false, err } - wd, err := os.Getwd() + wd, err := fsutils.Getwd() if err != nil { return false, err } @@ -48,3 +51,7 @@ func InGoRoot() (bool, error) { // TODO: strip, then add slashes return strings.HasPrefix(wd, goroot), nil } + +func IsCgoFilename(f string) bool { + return filepath.Base(f) == "C" +} diff --git a/pkg/lint/astcache/astcache.go b/pkg/lint/astcache/astcache.go index b7df0cf1..7d0e2ed8 100644 --- a/pkg/lint/astcache/astcache.go +++ b/pkg/lint/astcache/astcache.go @@ -1,13 +1,13 @@ package astcache import ( - "fmt" "go/ast" "go/parser" "go/token" - "os" "path/filepath" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/logutils" "golang.org/x/tools/go/loader" ) @@ -65,11 +65,6 @@ func (c *Cache) prepareValidFiles() { func LoadFromProgram(prog *loader.Program, log logutils.Log) (*Cache, error) { c := NewCache(log) - root, err := os.Getwd() - if err != nil { - return nil, fmt.Errorf("can't get working dir: %s", err) - } - for _, pkg := range prog.InitialPackages() { for _, f := range pkg.Files { pos := prog.Fset.Position(f.Pos()) @@ -77,15 +72,15 @@ func LoadFromProgram(prog *loader.Program, log logutils.Log) (*Cache, error) { continue } - path := pos.Filename - if filepath.IsAbs(path) { - relPath, err := filepath.Rel(root, pos.Filename) - if err != nil { - c.log.Warnf("Can't get relative path for %s and %s: %s", - root, pos.Filename, err) - continue - } - path = relPath + if goutils.IsCgoFilename(pos.Filename) { + continue + } + + path, err := fsutils.ShortestRelPath(pos.Filename, "") + if err != nil { + c.log.Warnf("Can't get relative path for %s: %s", + pos.Filename, err) + continue } c.m[path] = &File{ diff --git a/pkg/lint/load.go b/pkg/lint/load.go index 294c545e..3a6b3121 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/logutils" @@ -50,21 +51,13 @@ func isSSAReprNeeded(linters []linter.Config) bool { } 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 + relPath, err := fsutils.ShortestRelPath(p, "") + if err != nil { + return nil, fmt.Errorf("can't get relative path for path %s: %s", p, err) } + p = relPath ret = append(ret, "./"+p) } @@ -78,7 +71,7 @@ func getCurrentProjectImportPath() (string, error) { return "", fmt.Errorf("no GOPATH env variable") } - wd, err := os.Getwd() + wd, err := fsutils.Getwd() if err != nil { return "", fmt.Errorf("can't get workind directory: %s", err) } diff --git a/pkg/packages/package.go b/pkg/packages/package.go index 702cc965..6a4b2b5a 100644 --- a/pkg/packages/package.go +++ b/pkg/packages/package.go @@ -4,7 +4,7 @@ import ( "go/build" "path/filepath" - "github.com/golangci/golangci-lint/pkg/result/processors" + "github.com/golangci/golangci-lint/pkg/goutils" ) type Package struct { @@ -17,7 +17,7 @@ type Package struct { func (pkg *Package) Files(includeTest bool) []string { var pkgFiles []string for _, f := range pkg.bp.GoFiles { - if !processors.IsCgoFilename(f) { + if !goutils.IsCgoFilename(f) { // skip cgo at all levels to prevent failures on file reading pkgFiles = append(pkgFiles, f) } diff --git a/pkg/packages/resolver.go b/pkg/packages/resolver.go index 19455a77..29f8ad31 100644 --- a/pkg/packages/resolver.go +++ b/pkg/packages/resolver.go @@ -36,7 +36,7 @@ func NewResolver(buildTags, excludeDirs []string, log logutils.Log) (*Resolver, excludeDirsMap[dir] = re } - wd, err := os.Getwd() + wd, err := fsutils.Getwd() if err != nil { return nil, fmt.Errorf("can't get working dir: %s", err) } diff --git a/pkg/packages/resolver_test.go b/pkg/packages/resolver_test.go index 083c39f5..54e96f17 100644 --- a/pkg/packages/resolver_test.go +++ b/pkg/packages/resolver_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/packages" "github.com/stretchr/testify/assert" @@ -31,7 +32,7 @@ func prepareFS(t *testing.T, paths ...string) *fsPreparer { root, err := ioutil.TempDir("/tmp", "golangci.test.path_resolver") assert.NoError(t, err) - prevWD, err := os.Getwd() + prevWD, err := fsutils.Getwd() assert.NoError(t, err) err = os.Chdir(root) @@ -243,6 +244,7 @@ func TestPathResolverCommonCases(t *testing.T) { }, } + fsutils.UseWdCache(false) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { fp := prepareFS(t, tc.prepare...) diff --git a/pkg/result/processors/cgo.go b/pkg/result/processors/cgo.go index 6a6921ca..e0547ec6 100644 --- a/pkg/result/processors/cgo.go +++ b/pkg/result/processors/cgo.go @@ -1,6 +1,7 @@ package processors import ( + "github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -21,7 +22,7 @@ func (p Cgo) Process(issues []result.Issue) ([]result.Issue, error) { return filterIssues(issues, func(i *result.Issue) bool { // some linters (.e.g gas, deadcode) return incorrect filepaths for cgo issues, // it breaks next processing, so skip them - return !IsCgoFilename(i.FilePath()) + return !goutils.IsCgoFilename(i.FilePath()) }), nil } diff --git a/pkg/result/processors/path_prettifier.go b/pkg/result/processors/path_prettifier.go index 137f60d8..3a140999 100644 --- a/pkg/result/processors/path_prettifier.go +++ b/pkg/result/processors/path_prettifier.go @@ -2,9 +2,9 @@ package processors import ( "fmt" - "os" "path/filepath" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -15,7 +15,7 @@ type PathPrettifier struct { var _ Processor = PathPrettifier{} func NewPathPrettifier() *PathPrettifier { - root, err := os.Getwd() + root, err := fsutils.Getwd() if err != nil { panic(fmt.Sprintf("Can't get working dir: %s", err)) } @@ -34,7 +34,7 @@ func (p PathPrettifier) Process(issues []result.Issue) ([]result.Issue, error) { return i } - rel, err := filepath.Rel(p.root, i.FilePath()) + rel, err := fsutils.ShortestRelPath(i.FilePath(), "") if err != nil { return i } diff --git a/pkg/result/processors/utils.go b/pkg/result/processors/utils.go index c577f550..7c636a85 100644 --- a/pkg/result/processors/utils.go +++ b/pkg/result/processors/utils.go @@ -2,7 +2,6 @@ package processors import ( "fmt" - "path/filepath" "github.com/golangci/golangci-lint/pkg/result" ) @@ -45,7 +44,3 @@ func transformIssues(issues []result.Issue, transform func(i *result.Issue) *res return retIssues } - -func IsCgoFilename(f string) bool { - return filepath.Base(f) == "C" -} diff --git a/vendor/github.com/golangci/govet/composite.go b/vendor/github.com/golangci/govet/composite.go index 2e871022..ed0ea643 100644 --- a/vendor/github.com/golangci/govet/composite.go +++ b/vendor/github.com/golangci/govet/composite.go @@ -27,6 +27,10 @@ func init() { // checkUnkeyedLiteral checks if a composite literal is a struct literal with // unkeyed fields. func checkUnkeyedLiteral(f *File, node ast.Node) { + if strings.HasSuffix(f.name, "_test.go") { + return + } + cl := node.(*ast.CompositeLit) typ := f.pkg.types[cl].Type @@ -73,6 +77,15 @@ func checkUnkeyedLiteral(f *File, node ast.Node) { } func isLocalType(f *File, typ types.Type) bool { + structNameParts := strings.Split(typ.String(), ".") + if len(structNameParts) >= 2 { + structName := structNameParts[len(structNameParts)-1] + firstLetter := string(structName[0]) + if firstLetter == strings.ToLower(firstLetter) { + return true + } + } + switch x := typ.(type) { case *types.Struct: // struct literals are local types diff --git a/vendor/github.com/golangci/govet/golangci.go b/vendor/github.com/golangci/govet/golangci.go index a8b4ea45..42b77d34 100644 --- a/vendor/github.com/golangci/govet/golangci.go +++ b/vendor/github.com/golangci/govet/golangci.go @@ -15,7 +15,7 @@ type Issue struct { var foundIssues []Issue -func Analyze(files []*ast.File, fset *token.FileSet, pkgInfo *loader.PackageInfo, checkShadowing bool) ([]Issue, error) { +func Analyze(files []*ast.File, fset *token.FileSet, pkgInfo *loader.PackageInfo, checkShadowing bool, pg astFilePathGetter) ([]Issue, error) { foundIssues = nil *source = false // import type data for "fmt" from installed packages @@ -38,7 +38,7 @@ func Analyze(files []*ast.File, fset *token.FileSet, pkgInfo *loader.PackageInfo includesNonTest = true } } - pkg, err := doPackage(nil, pkgInfo, fset, files) + pkg, err := doPackage(nil, pkgInfo, fset, files, pg) if err != nil { return nil, err } diff --git a/vendor/github.com/golangci/govet/main.go b/vendor/github.com/golangci/govet/main.go index dea3ed71..8f2e949f 100644 --- a/vendor/github.com/golangci/govet/main.go +++ b/vendor/github.com/golangci/govet/main.go @@ -272,7 +272,7 @@ func main() { } os.Exit(exitCode) } - if pkg, _ := doPackage(nil, nil, nil, nil); pkg == nil { + if pkg, _ := doPackage(nil, nil, nil, nil, nil); pkg == nil { warnf("no files checked") } os.Exit(exitCode) @@ -345,7 +345,7 @@ func doPackageCfg(cfgFile string) { stdImporter = &vcfg inittypes() mustTypecheck = true - doPackage(nil, nil, nil, nil) + doPackage(nil, nil, nil, nil, nil) } // doPackageDir analyzes the single package found in the directory, if there is one, @@ -373,12 +373,12 @@ func doPackageDir(directory string) { names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package. names = append(names, pkg.SFiles...) prefixDirectory(directory, names) - basePkg, _ := doPackage(nil, nil, nil, nil) + basePkg, _ := doPackage(nil, nil, nil, nil, nil) // Is there also a "foo_test" package? If so, do that one as well. if len(pkg.XTestGoFiles) > 0 { names = pkg.XTestGoFiles prefixDirectory(directory, names) - doPackage(basePkg, nil, nil, nil) + doPackage(basePkg, nil, nil, nil, nil) } } @@ -393,56 +393,29 @@ type Package struct { typesPkg *types.Package } -func shortestRelPath(path string, wd string) (string, error) { - if wd == "" { // get it if user don't have cached working dir - var err error - wd, err = os.Getwd() - if err != nil { - return "", fmt.Errorf("can't get working directory: %s", err) - } - } - - // make path absolute and then relative to be able to fix this case: - // we'are in /test dir, we want to normalize ../test, and have file file.go in this dir; - // it must have normalized path file.go, not ../test/file.go, - var absPath string - if filepath.IsAbs(path) { - absPath = path - } else { - absPath = filepath.Join(wd, path) - } - - relPath, err := filepath.Rel(wd, absPath) - if err != nil { - return "", fmt.Errorf("can't get relative path for path %s and root %s: %s", - absPath, wd, err) - } - - return relPath, nil -} +type astFilePathGetter func(f *ast.File, fset *token.FileSet) (string, error) // doPackage analyzes the single package constructed from the named files. // It returns the parsed Package or nil if none of the files have been checked. -func doPackage(basePkg *Package, pkgInfo *loader.PackageInfo, fs *token.FileSet, astFiles []*ast.File) (*Package, error) { +func doPackage(basePkg *Package, pkgInfo *loader.PackageInfo, fs *token.FileSet, astFiles []*ast.File, pg astFilePathGetter) (*Package, error) { var files []*File for _, parsedFile := range astFiles { - name := fs.Position(parsedFile.Pos()).Filename - shortName, err := shortestRelPath(name, "") + name, err := pg(parsedFile, fs) if err != nil { return nil, err } - data, err := ioutil.ReadFile(shortName) + data, err := ioutil.ReadFile(name) if err != nil { - return nil, fmt.Errorf("can't read %q: %s", shortName, err) + return nil, fmt.Errorf("can't read %q: %s", name, err) } - checkBuildTag(shortName, data) + checkBuildTag(name, data) files = append(files, &File{ fset: fs, content: data, - name: shortName, + name: name, file: parsedFile, dead: make(map[ast.Node]bool), })