Skip to content

Commit

Permalink
Fix #126: fix working with symlinks
Browse files Browse the repository at this point in the history
  • Loading branch information
jirfag committed Jul 29, 2018
1 parent ca558ca commit 973c9fd
Show file tree
Hide file tree
Showing 15 changed files with 132 additions and 89 deletions.
2 changes: 1 addition & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

62 changes: 61 additions & 1 deletion pkg/fsutils/fsutils.go
Expand Up @@ -4,22 +4,82 @@ import (
"fmt"
"os"
"path/filepath"
"sync"
)

func IsDir(filename string) bool {
fi, err := os.Stat(filename)
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,
Expand Down
12 changes: 8 additions & 4 deletions pkg/golinters/govet.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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
}
Expand All @@ -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, "")
}
9 changes: 8 additions & 1 deletion pkg/goutils/goutils.go
Expand Up @@ -4,8 +4,11 @@ import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"sync"

"github.com/golangci/golangci-lint/pkg/fsutils"
)

var discoverGoRootOnce sync.Once
Expand Down Expand Up @@ -40,11 +43,15 @@ func InGoRoot() (bool, error) {
return false, err
}

wd, err := os.Getwd()
wd, err := fsutils.Getwd()
if err != nil {
return false, err
}

// TODO: strip, then add slashes
return strings.HasPrefix(wd, goroot), nil
}

func IsCgoFilename(f string) bool {
return filepath.Base(f) == "C"
}
27 changes: 11 additions & 16 deletions 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"
)
Expand Down Expand Up @@ -65,27 +65,22 @@ 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())
if pos.Filename == "" {
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{
Expand Down
19 changes: 6 additions & 13 deletions pkg/lint/load.go
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/packages/package.go
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/packages/resolver.go
Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/packages/resolver_test.go
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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...)
Expand Down
3 changes: 2 additions & 1 deletion 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"
)

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

Expand Down
6 changes: 3 additions & 3 deletions pkg/result/processors/path_prettifier.go
Expand Up @@ -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"
)

Expand All @@ -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))
}
Expand All @@ -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
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/result/processors/utils.go
Expand Up @@ -2,7 +2,6 @@ package processors

import (
"fmt"
"path/filepath"

"github.com/golangci/golangci-lint/pkg/result"
)
Expand Down Expand Up @@ -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"
}

0 comments on commit 973c9fd

Please sign in to comment.