Fix #126: fix working with symlinks

This commit is contained in:
Denis Isaev 2018-07-29 21:45:19 +03:00 committed by Isaev Denis
parent ca558ca571
commit 973c9fdfd8
15 changed files with 132 additions and 89 deletions

2
Gopkg.lock generated
View File

@ -136,7 +136,7 @@
"lib/whitelist", "lib/whitelist",
] ]
pruneopts = "UT" pruneopts = "UT"
revision = "54e733a64097854cf4d8fa80ccf48012a487ae1a" revision = "e3b43b6cb1916e0000f244f26ee752733e544429"
[[projects]] [[projects]]
branch = "master" branch = "master"

View File

@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
"sync"
) )
func IsDir(filename string) bool { func IsDir(filename string) bool {
@ -11,15 +12,74 @@ func IsDir(filename string) bool {
return err == nil && fi.IsDir() 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) { func ShortestRelPath(path string, wd string) (string, error) {
if wd == "" { // get it if user don't have cached working dir if wd == "" { // get it if user don't have cached working dir
var err error var err error
wd, err = os.Getwd() wd, err = Getwd()
if err != nil { if err != nil {
return "", fmt.Errorf("can't get working directory: %s", err) 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: // 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; // 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, // it must have normalized path file.go, not ../test/file.go,

View File

@ -10,11 +10,11 @@ import (
"strings" "strings"
"time" "time"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/goutils"
"github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
"github.com/golangci/golangci-lint/pkg/result/processors"
"github.com/golangci/golangci-lint/pkg/timeutils" "github.com/golangci/golangci-lint/pkg/timeutils"
govetAPI "github.com/golangci/govet" govetAPI "github.com/golangci/govet"
) )
@ -82,7 +82,7 @@ func (g Govet) runOnInstalledPackages(ctx context.Context, lintCtx *linter.Conte
continue continue
} }
issues, err := govetAPI.Analyze(astFiles, fset, nil, issues, err := govetAPI.Analyze(astFiles, fset, nil,
lintCtx.Settings().Govet.CheckShadowing) lintCtx.Settings().Govet.CheckShadowing, getPath)
if err != nil { if err != nil {
return nil, err 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 { func filterFiles(files []*ast.File, fset *token.FileSet) []*ast.File {
newFiles := make([]*ast.File, 0, len(files)) newFiles := make([]*ast.File, 0, len(files))
for _, f := range 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) newFiles = append(newFiles, f)
} }
} }
@ -238,7 +238,7 @@ func (g Govet) runOnSourcePackages(_ context.Context, lintCtx *linter.Context) (
filteredFiles := filterFiles(pkg.Files, lintCtx.Program.Fset) filteredFiles := filterFiles(pkg.Files, lintCtx.Program.Fset)
issues, err := govetAPI.Analyze(filteredFiles, lintCtx.Program.Fset, pkg, issues, err := govetAPI.Analyze(filteredFiles, lintCtx.Program.Fset, pkg,
lintCtx.Settings().Govet.CheckShadowing) lintCtx.Settings().Govet.CheckShadowing, getPath)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -247,3 +247,7 @@ func (g Govet) runOnSourcePackages(_ context.Context, lintCtx *linter.Context) (
return govetIssues, nil return govetIssues, nil
} }
func getPath(f *ast.File, fset *token.FileSet) (string, error) {
return fsutils.ShortestRelPath(fset.Position(f.Pos()).Filename, "")
}

View File

@ -4,8 +4,11 @@ import (
"fmt" "fmt"
"os" "os"
"os/exec" "os/exec"
"path/filepath"
"strings" "strings"
"sync" "sync"
"github.com/golangci/golangci-lint/pkg/fsutils"
) )
var discoverGoRootOnce sync.Once var discoverGoRootOnce sync.Once
@ -40,7 +43,7 @@ func InGoRoot() (bool, error) {
return false, err return false, err
} }
wd, err := os.Getwd() wd, err := fsutils.Getwd()
if err != nil { if err != nil {
return false, err return false, err
} }
@ -48,3 +51,7 @@ func InGoRoot() (bool, error) {
// TODO: strip, then add slashes // TODO: strip, then add slashes
return strings.HasPrefix(wd, goroot), nil return strings.HasPrefix(wd, goroot), nil
} }
func IsCgoFilename(f string) bool {
return filepath.Base(f) == "C"
}

View File

@ -1,13 +1,13 @@
package astcache package astcache
import ( import (
"fmt"
"go/ast" "go/ast"
"go/parser" "go/parser"
"go/token" "go/token"
"os"
"path/filepath" "path/filepath"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/goutils"
"github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/logutils"
"golang.org/x/tools/go/loader" "golang.org/x/tools/go/loader"
) )
@ -65,11 +65,6 @@ func (c *Cache) prepareValidFiles() {
func LoadFromProgram(prog *loader.Program, log logutils.Log) (*Cache, error) { func LoadFromProgram(prog *loader.Program, log logutils.Log) (*Cache, error) {
c := NewCache(log) 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 _, pkg := range prog.InitialPackages() {
for _, f := range pkg.Files { for _, f := range pkg.Files {
pos := prog.Fset.Position(f.Pos()) pos := prog.Fset.Position(f.Pos())
@ -77,15 +72,15 @@ func LoadFromProgram(prog *loader.Program, log logutils.Log) (*Cache, error) {
continue continue
} }
path := pos.Filename if goutils.IsCgoFilename(pos.Filename) {
if filepath.IsAbs(path) { continue
relPath, err := filepath.Rel(root, pos.Filename) }
if err != nil {
c.log.Warnf("Can't get relative path for %s and %s: %s", path, err := fsutils.ShortestRelPath(pos.Filename, "")
root, pos.Filename, err) if err != nil {
continue c.log.Warnf("Can't get relative path for %s: %s",
} pos.Filename, err)
path = relPath continue
} }
c.m[path] = &File{ c.m[path] = &File{

View File

@ -10,6 +10,7 @@ import (
"strings" "strings"
"time" "time"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/goutils" "github.com/golangci/golangci-lint/pkg/goutils"
"github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/logutils"
@ -50,21 +51,13 @@ func isSSAReprNeeded(linters []linter.Config) bool {
} }
func normalizePaths(paths []string) ([]string, 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)) ret := make([]string, 0, len(paths))
for _, p := range paths { for _, p := range paths {
if filepath.IsAbs(p) { relPath, err := fsutils.ShortestRelPath(p, "")
relPath, err := filepath.Rel(root, p) if err != nil {
if err != nil { return nil, fmt.Errorf("can't get relative path for path %s: %s", p, err)
return nil, fmt.Errorf("can't get relative path for path %s and root %s: %s",
p, root, err)
}
p = relPath
} }
p = relPath
ret = append(ret, "./"+p) ret = append(ret, "./"+p)
} }
@ -78,7 +71,7 @@ func getCurrentProjectImportPath() (string, error) {
return "", fmt.Errorf("no GOPATH env variable") return "", fmt.Errorf("no GOPATH env variable")
} }
wd, err := os.Getwd() wd, err := fsutils.Getwd()
if err != nil { if err != nil {
return "", fmt.Errorf("can't get workind directory: %s", err) return "", fmt.Errorf("can't get workind directory: %s", err)
} }

View File

@ -4,7 +4,7 @@ import (
"go/build" "go/build"
"path/filepath" "path/filepath"
"github.com/golangci/golangci-lint/pkg/result/processors" "github.com/golangci/golangci-lint/pkg/goutils"
) )
type Package struct { type Package struct {
@ -17,7 +17,7 @@ type Package struct {
func (pkg *Package) Files(includeTest bool) []string { func (pkg *Package) Files(includeTest bool) []string {
var pkgFiles []string var pkgFiles []string
for _, f := range pkg.bp.GoFiles { 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 // skip cgo at all levels to prevent failures on file reading
pkgFiles = append(pkgFiles, f) pkgFiles = append(pkgFiles, f)
} }

View File

@ -36,7 +36,7 @@ func NewResolver(buildTags, excludeDirs []string, log logutils.Log) (*Resolver,
excludeDirsMap[dir] = re excludeDirsMap[dir] = re
} }
wd, err := os.Getwd() wd, err := fsutils.Getwd()
if err != nil { if err != nil {
return nil, fmt.Errorf("can't get working dir: %s", err) return nil, fmt.Errorf("can't get working dir: %s", err)
} }

View File

@ -8,6 +8,7 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/packages" "github.com/golangci/golangci-lint/pkg/packages"
"github.com/stretchr/testify/assert" "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") root, err := ioutil.TempDir("/tmp", "golangci.test.path_resolver")
assert.NoError(t, err) assert.NoError(t, err)
prevWD, err := os.Getwd() prevWD, err := fsutils.Getwd()
assert.NoError(t, err) assert.NoError(t, err)
err = os.Chdir(root) err = os.Chdir(root)
@ -243,6 +244,7 @@ func TestPathResolverCommonCases(t *testing.T) {
}, },
} }
fsutils.UseWdCache(false)
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
fp := prepareFS(t, tc.prepare...) fp := prepareFS(t, tc.prepare...)

View File

@ -1,6 +1,7 @@
package processors package processors
import ( import (
"github.com/golangci/golangci-lint/pkg/goutils"
"github.com/golangci/golangci-lint/pkg/result" "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 { return filterIssues(issues, func(i *result.Issue) bool {
// some linters (.e.g gas, deadcode) return incorrect filepaths for cgo issues, // some linters (.e.g gas, deadcode) return incorrect filepaths for cgo issues,
// it breaks next processing, so skip them // it breaks next processing, so skip them
return !IsCgoFilename(i.FilePath()) return !goutils.IsCgoFilename(i.FilePath())
}), nil }), nil
} }

View File

@ -2,9 +2,9 @@ package processors
import ( import (
"fmt" "fmt"
"os"
"path/filepath" "path/filepath"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
@ -15,7 +15,7 @@ type PathPrettifier struct {
var _ Processor = PathPrettifier{} var _ Processor = PathPrettifier{}
func NewPathPrettifier() *PathPrettifier { func NewPathPrettifier() *PathPrettifier {
root, err := os.Getwd() root, err := fsutils.Getwd()
if err != nil { if err != nil {
panic(fmt.Sprintf("Can't get working dir: %s", err)) 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 return i
} }
rel, err := filepath.Rel(p.root, i.FilePath()) rel, err := fsutils.ShortestRelPath(i.FilePath(), "")
if err != nil { if err != nil {
return i return i
} }

View File

@ -2,7 +2,6 @@ package processors
import ( import (
"fmt" "fmt"
"path/filepath"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
@ -45,7 +44,3 @@ func transformIssues(issues []result.Issue, transform func(i *result.Issue) *res
return retIssues return retIssues
} }
func IsCgoFilename(f string) bool {
return filepath.Base(f) == "C"
}

View File

@ -27,6 +27,10 @@ func init() {
// checkUnkeyedLiteral checks if a composite literal is a struct literal with // checkUnkeyedLiteral checks if a composite literal is a struct literal with
// unkeyed fields. // unkeyed fields.
func checkUnkeyedLiteral(f *File, node ast.Node) { func checkUnkeyedLiteral(f *File, node ast.Node) {
if strings.HasSuffix(f.name, "_test.go") {
return
}
cl := node.(*ast.CompositeLit) cl := node.(*ast.CompositeLit)
typ := f.pkg.types[cl].Type 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 { 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) { switch x := typ.(type) {
case *types.Struct: case *types.Struct:
// struct literals are local types // struct literals are local types

View File

@ -15,7 +15,7 @@ type Issue struct {
var foundIssues []Issue 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 foundIssues = nil
*source = false // import type data for "fmt" from installed packages *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 includesNonTest = true
} }
} }
pkg, err := doPackage(nil, pkgInfo, fset, files) pkg, err := doPackage(nil, pkgInfo, fset, files, pg)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -272,7 +272,7 @@ func main() {
} }
os.Exit(exitCode) 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") warnf("no files checked")
} }
os.Exit(exitCode) os.Exit(exitCode)
@ -345,7 +345,7 @@ func doPackageCfg(cfgFile string) {
stdImporter = &vcfg stdImporter = &vcfg
inittypes() inittypes()
mustTypecheck = true 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, // 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.TestGoFiles...) // These are also in the "foo" package.
names = append(names, pkg.SFiles...) names = append(names, pkg.SFiles...)
prefixDirectory(directory, names) 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. // Is there also a "foo_test" package? If so, do that one as well.
if len(pkg.XTestGoFiles) > 0 { if len(pkg.XTestGoFiles) > 0 {
names = pkg.XTestGoFiles names = pkg.XTestGoFiles
prefixDirectory(directory, names) prefixDirectory(directory, names)
doPackage(basePkg, nil, nil, nil) doPackage(basePkg, nil, nil, nil, nil)
} }
} }
@ -393,56 +393,29 @@ type Package struct {
typesPkg *types.Package typesPkg *types.Package
} }
func shortestRelPath(path string, wd string) (string, error) { type astFilePathGetter func(f *ast.File, fset *token.FileSet) (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
}
// doPackage analyzes the single package constructed from the named files. // 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. // 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 var files []*File
for _, parsedFile := range astFiles { for _, parsedFile := range astFiles {
name := fs.Position(parsedFile.Pos()).Filename name, err := pg(parsedFile, fs)
shortName, err := shortestRelPath(name, "")
if err != nil { if err != nil {
return nil, err return nil, err
} }
data, err := ioutil.ReadFile(shortName) data, err := ioutil.ReadFile(name)
if err != nil { 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{ files = append(files, &File{
fset: fs, fset: fs,
content: data, content: data,
name: shortName, name: name,
file: parsedFile, file: parsedFile,
dead: make(map[ast.Node]bool), dead: make(map[ast.Node]bool),
}) })