Skip to content

Commit

Permalink
#45: fix no results for gocyclo
Browse files Browse the repository at this point in the history
  • Loading branch information
golangcidev committed May 30, 2018
1 parent 80a5ff2 commit 0225bc0
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 10 deletions.
32 changes: 28 additions & 4 deletions pkg/astcache/astcache.go
Original file line number Diff line number Diff line change
@@ -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
}

Expand All @@ -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 {
Expand All @@ -63,6 +86,7 @@ func LoadFromFiles(files []string) *Cache {
F: f,
Fset: fset,
err: err,
Name: filePath,
}
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/commands/run_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
}
20 changes: 15 additions & 5 deletions pkg/enabled_linters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions pkg/enabled_linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os/exec"
"path/filepath"
"runtime"
"sort"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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)
}
}

0 comments on commit 0225bc0

Please sign in to comment.