Skip to content

Commit 21ff49c

Browse files
author
golangci
authored
Merge pull request #46 from golangci/support/fix-no-results-for-gocyclo
#45: fix no results for gocyclo
2 parents 80a5ff2 + ef81b99 commit 21ff49c

File tree

6 files changed

+193
-52
lines changed

6 files changed

+193
-52
lines changed

pkg/astcache/astcache.go

+28-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
package astcache
22

33
import (
4+
"fmt"
45
"go/ast"
56
"go/parser"
67
"go/token"
8+
"os"
9+
"path/filepath"
710

11+
"github.com/sirupsen/logrus"
812
"golang.org/x/tools/go/loader"
913
)
1014

1115
type File struct {
1216
F *ast.File
1317
Fset *token.FileSet
18+
Name string
1419
err error
1520
}
1621

@@ -34,22 +39,40 @@ func (c *Cache) prepareValidFiles() {
3439
c.s = files
3540
}
3641

37-
func LoadFromProgram(prog *loader.Program) *Cache {
42+
func LoadFromProgram(prog *loader.Program) (*Cache, error) {
3843
c := &Cache{
3944
m: map[string]*File{},
4045
}
46+
47+
root, err := os.Getwd()
48+
if err != nil {
49+
return nil, fmt.Errorf("can't get working dir: %s", err)
50+
}
51+
4152
for _, pkg := range prog.InitialPackages() {
4253
for _, f := range pkg.Files {
43-
pos := prog.Fset.Position(0)
44-
c.m[pos.Filename] = &File{
54+
pos := prog.Fset.Position(f.Pos())
55+
if pos.Filename == "" {
56+
continue
57+
}
58+
59+
relPath, err := filepath.Rel(root, pos.Filename)
60+
if err != nil {
61+
logrus.Warnf("Can't get relative path for %s and %s: %s",
62+
root, pos.Filename, err)
63+
continue
64+
}
65+
66+
c.m[relPath] = &File{
4567
F: f,
4668
Fset: prog.Fset,
69+
Name: relPath,
4770
}
4871
}
4972
}
5073

5174
c.prepareValidFiles()
52-
return c
75+
return c, nil
5376
}
5477

5578
func LoadFromFiles(files []string) *Cache {
@@ -63,6 +86,7 @@ func LoadFromFiles(files []string) *Cache {
6386
F: f,
6487
Fset: fset,
6588
err: err,
89+
Name: filePath,
6690
}
6791
}
6892

pkg/commands/root.go

+46-42
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,50 @@ import (
1212
"github.com/spf13/cobra"
1313
)
1414

15+
func (e *Executor) persistentPostRun(cmd *cobra.Command, args []string) {
16+
if e.cfg.Run.PrintVersion {
17+
fmt.Fprintf(printers.StdOut, "golangci-lint has version %s built from %s on %s\n", e.version, e.commit, e.date)
18+
os.Exit(0)
19+
}
20+
21+
runtime.GOMAXPROCS(e.cfg.Run.Concurrency)
22+
23+
log.SetFlags(0) // don't print time
24+
if e.cfg.Run.IsVerbose {
25+
logrus.SetLevel(logrus.InfoLevel)
26+
} else {
27+
logrus.SetLevel(logrus.WarnLevel)
28+
}
29+
30+
if e.cfg.Run.CPUProfilePath != "" {
31+
f, err := os.Create(e.cfg.Run.CPUProfilePath)
32+
if err != nil {
33+
logrus.Fatal(err)
34+
}
35+
if err := pprof.StartCPUProfile(f); err != nil {
36+
logrus.Fatal(err)
37+
}
38+
}
39+
}
40+
41+
func (e *Executor) persistentPreRun(cmd *cobra.Command, args []string) {
42+
if e.cfg.Run.CPUProfilePath != "" {
43+
pprof.StopCPUProfile()
44+
}
45+
if e.cfg.Run.MemProfilePath != "" {
46+
f, err := os.Create(e.cfg.Run.MemProfilePath)
47+
if err != nil {
48+
logrus.Fatal(err)
49+
}
50+
runtime.GC() // get up-to-date statistics
51+
if err := pprof.WriteHeapProfile(f); err != nil {
52+
logrus.Fatal("could not write memory profile: ", err)
53+
}
54+
}
55+
56+
os.Exit(e.exitCode)
57+
}
58+
1559
func (e *Executor) initRoot() {
1660
rootCmd := &cobra.Command{
1761
Use: "golangci-lint",
@@ -22,48 +66,8 @@ func (e *Executor) initRoot() {
2266
logrus.Fatal(err)
2367
}
2468
},
25-
PersistentPreRun: func(cmd *cobra.Command, args []string) {
26-
if e.cfg.Run.PrintVersion {
27-
fmt.Fprintf(printers.StdOut, "golangci-lint has version %s built from %s on %s\n", e.version, e.commit, e.date)
28-
os.Exit(0)
29-
}
30-
31-
runtime.GOMAXPROCS(e.cfg.Run.Concurrency)
32-
33-
log.SetFlags(0) // don't print time
34-
if e.cfg.Run.IsVerbose {
35-
logrus.SetLevel(logrus.InfoLevel)
36-
} else {
37-
logrus.SetLevel(logrus.WarnLevel)
38-
}
39-
40-
if e.cfg.Run.CPUProfilePath != "" {
41-
f, err := os.Create(e.cfg.Run.CPUProfilePath)
42-
if err != nil {
43-
logrus.Fatal(err)
44-
}
45-
if err := pprof.StartCPUProfile(f); err != nil {
46-
logrus.Fatal(err)
47-
}
48-
}
49-
},
50-
PersistentPostRun: func(cmd *cobra.Command, args []string) {
51-
if e.cfg.Run.CPUProfilePath != "" {
52-
pprof.StopCPUProfile()
53-
}
54-
if e.cfg.Run.MemProfilePath != "" {
55-
f, err := os.Create(e.cfg.Run.MemProfilePath)
56-
if err != nil {
57-
logrus.Fatal(err)
58-
}
59-
runtime.GC() // get up-to-date statistics
60-
if err := pprof.WriteHeapProfile(f); err != nil {
61-
logrus.Fatal("could not write memory profile: ", err)
62-
}
63-
}
64-
65-
os.Exit(e.exitCode)
66-
},
69+
PersistentPreRun: e.persistentPostRun,
70+
PersistentPostRun: e.persistentPreRun,
6771
}
6872
rootCmd.PersistentFlags().BoolVarP(&e.cfg.Run.IsVerbose, "verbose", "v", false, "verbose output")
6973
rootCmd.PersistentFlags().StringVar(&e.cfg.Run.CPUProfilePath, "cpu-profile-path", "", "Path to CPU profile output file")

pkg/commands/run.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,10 @@ func buildLintCtx(ctx context.Context, linters []pkg.Linter, cfg *config.Config)
254254

255255
var astCache *astcache.Cache
256256
if prog != nil {
257-
astCache = astcache.LoadFromProgram(prog)
257+
astCache, err = astcache.LoadFromProgram(prog)
258+
if err != nil {
259+
return nil, err
260+
}
258261
} else {
259262
astCache = astcache.LoadFromFiles(paths.Files)
260263
}

pkg/commands/run_test.go

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package commands
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/golangci/golangci-lint/pkg"
8+
"github.com/golangci/golangci-lint/pkg/astcache"
9+
"github.com/golangci/golangci-lint/pkg/config"
10+
"github.com/golangci/golangci-lint/pkg/fsutils"
11+
"github.com/golangci/golangci-lint/pkg/golinters"
12+
"github.com/stretchr/testify/assert"
13+
)
14+
15+
func TestASTCacheLoading(t *testing.T) {
16+
ctx := context.Background()
17+
linters := []pkg.Linter{golinters.Errcheck{}}
18+
19+
inputPaths := []string{"./...", "./", "./run.go", "run.go"}
20+
for _, inputPath := range inputPaths {
21+
paths, err := fsutils.GetPathsForAnalysis(ctx, []string{inputPath}, true)
22+
assert.NoError(t, err)
23+
assert.NotEmpty(t, paths.Files)
24+
25+
prog, _, err := loadWholeAppIfNeeded(ctx, linters, &config.Run{
26+
AnalyzeTests: true,
27+
}, paths)
28+
assert.NoError(t, err)
29+
30+
astCacheFromProg, err := astcache.LoadFromProgram(prog)
31+
assert.NoError(t, err)
32+
33+
astCacheFromFiles := astcache.LoadFromFiles(paths.Files)
34+
35+
filesFromProg := astCacheFromProg.GetAllValidFiles()
36+
filesFromFiles := astCacheFromFiles.GetAllValidFiles()
37+
if len(filesFromProg) != len(filesFromFiles) {
38+
t.Logf("files: %s", paths.Files)
39+
t.Logf("from prog:")
40+
for _, f := range filesFromProg {
41+
t.Logf("%+v", *f)
42+
}
43+
t.Logf("from files:")
44+
for _, f := range filesFromFiles {
45+
t.Logf("%+v", *f)
46+
}
47+
t.Fatalf("lengths differ")
48+
}
49+
50+
if len(filesFromProg) != len(paths.Files) {
51+
t.Fatalf("filesFromProg differ from path.Files")
52+
}
53+
}
54+
}

pkg/enabled_linters.go

+15-5
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,7 @@ func GetAllLintersForPreset(p string) []Linter {
294294
return ret
295295
}
296296

297-
func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocyclo
298-
lcfg := &cfg.Linters
299-
297+
func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []Linter) map[string]Linter { // nolint:gocyclo
300298
resultLintersSet := map[string]Linter{}
301299
switch {
302300
case len(lcfg.Presets) != 0:
@@ -306,7 +304,7 @@ func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocy
306304
case lcfg.DisableAll:
307305
break
308306
default:
309-
resultLintersSet = lintersToMap(getAllEnabledByDefaultLinters())
307+
resultLintersSet = lintersToMap(enabledByDefaultLinters)
310308
}
311309

312310
// --presets can only add linters to default set
@@ -332,12 +330,24 @@ func getEnabledLintersSet(cfg *config.Config) map[string]Linter { // nolint:gocy
332330
}
333331

334332
for _, name := range lcfg.Disable {
333+
if name == "megacheck" {
334+
for _, ln := range getAllMegacheckSubLinterNames() {
335+
delete(resultLintersSet, ln)
336+
}
337+
}
335338
delete(resultLintersSet, name)
336339
}
337340

338341
return resultLintersSet
339342
}
340343

344+
func getAllMegacheckSubLinterNames() []string {
345+
unusedName := golinters.Megacheck{UnusedEnabled: true}.Name()
346+
gosimpleName := golinters.Megacheck{GosimpleEnabled: true}.Name()
347+
staticcheckName := golinters.Megacheck{StaticcheckEnabled: true}.Name()
348+
return []string{unusedName, gosimpleName, staticcheckName}
349+
}
350+
341351
func optimizeLintersSet(linters map[string]Linter) {
342352
unusedName := golinters.Megacheck{UnusedEnabled: true}.Name()
343353
gosimpleName := golinters.Megacheck{GosimpleEnabled: true}.Name()
@@ -375,7 +385,7 @@ func GetEnabledLinters(cfg *config.Config) ([]Linter, error) {
375385
return nil, err
376386
}
377387

378-
resultLintersSet := getEnabledLintersSet(cfg)
388+
resultLintersSet := getEnabledLintersSet(&cfg.Linters, getAllEnabledByDefaultLinters())
379389
optimizeLintersSet(resultLintersSet)
380390

381391
var resultLinters []Linter

pkg/enabled_linters_test.go

+46
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os/exec"
99
"path/filepath"
1010
"runtime"
11+
"sort"
1112
"strconv"
1213
"strings"
1314
"sync"
@@ -370,3 +371,48 @@ func BenchmarkWithGometalinter(b *testing.B) {
370371
compare(b, runGometalinter, runGolangciLint, bc.name, "", lc/1000)
371372
}
372373
}
374+
375+
func TestGetEnabledLintersSet(t *testing.T) {
376+
type cs struct {
377+
cfg config.Linters
378+
name string // test case name
379+
def []string // enabled by default linters
380+
exp []string // alphabetically ordered enabled linter names
381+
}
382+
cases := []cs{
383+
{
384+
cfg: config.Linters{
385+
Disable: []string{"megacheck"},
386+
},
387+
name: "disable all linters from megacheck",
388+
def: getAllMegacheckSubLinterNames(),
389+
},
390+
{
391+
cfg: config.Linters{
392+
Disable: []string{"staticcheck"},
393+
},
394+
name: "disable only staticcheck",
395+
def: getAllMegacheckSubLinterNames(),
396+
exp: []string{"gosimple", "unused"},
397+
},
398+
}
399+
400+
for _, c := range cases {
401+
t.Run(c.name, func(t *testing.T) {
402+
defaultLinters := []Linter{}
403+
for _, ln := range c.def {
404+
defaultLinters = append(defaultLinters, getLinterByName(ln))
405+
}
406+
els := getEnabledLintersSet(&c.cfg, defaultLinters)
407+
var enabledLinters []string
408+
for ln := range els {
409+
enabledLinters = append(enabledLinters, ln)
410+
}
411+
412+
sort.Strings(enabledLinters)
413+
sort.Strings(c.exp)
414+
415+
assert.Equal(t, c.exp, enabledLinters)
416+
})
417+
}
418+
}

0 commit comments

Comments
 (0)