From 0225bc0c9241dae6d10b8e37a46d0ff24f679ae0 Mon Sep 17 00:00:00 2001 From: golangci Date: Wed, 30 May 2018 19:54:29 +0300 Subject: [PATCH] #45: fix no results for gocyclo --- pkg/astcache/astcache.go | 32 +++++++++++++++++++--- pkg/commands/run.go | 5 +++- pkg/commands/run_test.go | 54 +++++++++++++++++++++++++++++++++++++ pkg/enabled_linters.go | 20 ++++++++++---- pkg/enabled_linters_test.go | 44 ++++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 pkg/commands/run_test.go diff --git a/pkg/astcache/astcache.go b/pkg/astcache/astcache.go index 0f44151c28ee..c9fcd3a6eac9 100644 --- a/pkg/astcache/astcache.go +++ b/pkg/astcache/astcache.go @@ -1,16 +1,21 @@ package astcache import ( + "fmt" "go/ast" "go/parser" "go/token" + "os" + "path/filepath" + "github.com/sirupsen/logrus" "golang.org/x/tools/go/loader" ) type File struct { F *ast.File Fset *token.FileSet + Name string err error } @@ -34,22 +39,40 @@ func (c *Cache) prepareValidFiles() { c.s = files } -func LoadFromProgram(prog *loader.Program) *Cache { +func LoadFromProgram(prog *loader.Program) (*Cache, error) { c := &Cache{ m: map[string]*File{}, } + + 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(0) - c.m[pos.Filename] = &File{ + pos := prog.Fset.Position(f.Pos()) + if pos.Filename == "" { + continue + } + + relPath, err := filepath.Rel(root, pos.Filename) + if err != nil { + logrus.Warnf("Can't get relative path for %s and %s: %s", + root, pos.Filename, err) + continue + } + + c.m[relPath] = &File{ F: f, Fset: prog.Fset, + Name: relPath, } } } c.prepareValidFiles() - return c + return c, nil } func LoadFromFiles(files []string) *Cache { @@ -63,6 +86,7 @@ func LoadFromFiles(files []string) *Cache { F: f, Fset: fset, err: err, + Name: filePath, } } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index acf75ace0ca5..a2a9c1774522 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -254,7 +254,10 @@ func buildLintCtx(ctx context.Context, linters []pkg.Linter, cfg *config.Config) var astCache *astcache.Cache if prog != nil { - astCache = astcache.LoadFromProgram(prog) + astCache, err = astcache.LoadFromProgram(prog) + if err != nil { + return nil, err + } } else { astCache = astcache.LoadFromFiles(paths.Files) } diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go new file mode 100644 index 000000000000..53024ca09609 --- /dev/null +++ b/pkg/commands/run_test.go @@ -0,0 +1,54 @@ +package commands + +import ( + "context" + "testing" + + "github.com/golangci/golangci-lint/pkg" + "github.com/golangci/golangci-lint/pkg/astcache" + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/golinters" + "github.com/stretchr/testify/assert" +) + +func TestASTCacheLoading(t *testing.T) { + ctx := context.Background() + linters := []pkg.Linter{golinters.Errcheck{}} + + inputPaths := []string{"./...", "./", "./run.go", "run.go"} + for _, inputPath := range inputPaths { + paths, err := fsutils.GetPathsForAnalysis(ctx, []string{inputPath}, true) + assert.NoError(t, err) + assert.NotEmpty(t, paths.Files) + + prog, _, err := loadWholeAppIfNeeded(ctx, linters, &config.Run{ + AnalyzeTests: true, + }, paths) + assert.NoError(t, err) + + astCacheFromProg, err := astcache.LoadFromProgram(prog) + assert.NoError(t, err) + + astCacheFromFiles := astcache.LoadFromFiles(paths.Files) + + filesFromProg := astCacheFromProg.GetAllValidFiles() + filesFromFiles := astCacheFromFiles.GetAllValidFiles() + if len(filesFromProg) != len(filesFromFiles) { + t.Logf("files: %s", paths.Files) + t.Logf("from prog:") + for _, f := range filesFromProg { + t.Logf("%+v", *f) + } + t.Logf("from files:") + for _, f := range filesFromFiles { + t.Logf("%+v", *f) + } + t.Fatalf("lengths differ") + } + + if len(filesFromProg) != len(paths.Files) { + t.Fatalf("filesFromProg differ from path.Files") + } + } +} diff --git a/pkg/enabled_linters.go b/pkg/enabled_linters.go index a79328809c63..ddb016b5a4ee 100644 --- a/pkg/enabled_linters.go +++ b/pkg/enabled_linters.go @@ -294,9 +294,7 @@ func GetAllLintersForPreset(p string) []Linter { return ret } -func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocyclo - lcfg := &cfg.Linters - +func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []Linter) map[string]Linter { // nolint:gocyclo resultLintersSet := map[string]Linter{} switch { case len(lcfg.Presets) != 0: @@ -306,7 +304,7 @@ func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocy case lcfg.DisableAll: break default: - resultLintersSet = lintersToMap(getAllEnabledByDefaultLinters()) + resultLintersSet = lintersToMap(enabledByDefaultLinters) } // --presets can only add linters to default set @@ -332,12 +330,24 @@ func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocy } for _, name := range lcfg.Disable { + if name == "megacheck" { + for _, ln := range getAllMegacheckSubLinterNames() { + delete(resultLintersSet, ln) + } + } delete(resultLintersSet, name) } return resultLintersSet } +func getAllMegacheckSubLinterNames() []string { + unusedName := golinters.Megacheck{UnusedEnabled: true}.Name() + gosimpleName := golinters.Megacheck{GosimpleEnabled: true}.Name() + staticcheckName := golinters.Megacheck{StaticcheckEnabled: true}.Name() + return []string{unusedName, gosimpleName, staticcheckName} +} + func optimizeLintersSet(linters map[string]Linter) { unusedName := golinters.Megacheck{UnusedEnabled: true}.Name() gosimpleName := golinters.Megacheck{GosimpleEnabled: true}.Name() @@ -375,7 +385,7 @@ func GetEnabledLinters(cfg *config.Config) ([]Linter, error) { return nil, err } - resultLintersSet := getEnabledLintersSet(cfg) + resultLintersSet := getEnabledLintersSet(&cfg.Linters, getAllEnabledByDefaultLinters()) optimizeLintersSet(resultLintersSet) var resultLinters []Linter diff --git a/pkg/enabled_linters_test.go b/pkg/enabled_linters_test.go index 2ebf38ec415e..81766984568a 100644 --- a/pkg/enabled_linters_test.go +++ b/pkg/enabled_linters_test.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "runtime" + "sort" "strconv" "strings" "sync" @@ -370,3 +371,46 @@ func BenchmarkWithGometalinter(b *testing.B) { compare(b, runGometalinter, runGolangciLint, bc.name, "", lc/1000) } } + +func TestGetEnabledLintersSet(t *testing.T) { + type cs struct { + cfg config.Linters + name string // test case name + def []string // enabled by default linters + exp []string // alphabetically ordered enabled linter names + } + cases := []cs{ + { + cfg: config.Linters{ + Disable: []string{"megacheck"}, + }, + name: "disable all linters from megacheck", + def: getAllMegacheckSubLinterNames(), + }, + { + cfg: config.Linters{ + Disable: []string{"staticcheck"}, + }, + name: "disable only staticcheck", + def: getAllMegacheckSubLinterNames(), + exp: []string{"gosimple", "unused"}, + }, + } + + for _, c := range cases { + defaultLinters := []Linter{} + for _, ln := range c.def { + defaultLinters = append(defaultLinters, getLinterByName(ln)) + } + els := getEnabledLintersSet(&c.cfg, defaultLinters) + var enabledLinters []string + for ln := range els { + enabledLinters = append(enabledLinters, ln) + } + + sort.Strings(enabledLinters) + sort.Strings(c.exp) + + assert.Equal(t, c.exp, enabledLinters) + } +}