diff --git a/Makefile b/Makefile index d89316bc..0f6d4424 100644 --- a/Makefile +++ b/Makefile @@ -4,5 +4,7 @@ test: golangci-lint run --fast --no-config -v golangci-lint run --fast --no-config -v golangci-lint run --no-config -v - golangci-lint run --fast --no-config -v ./pkg/testdata/with_issues/typecheck.go + golangci-lint run --fast --no-config -v ./test/testdata/typecheck.go go test -v -race ./... + +.PHONY: test \ No newline at end of file diff --git a/README.md b/README.md index 1746a6ae..253f8aa9 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ This issue is important because often you'd like to set concurrency to CPUs coun 3. It will take more time because of different usages and need of tracking of versions of `n` linters. # Performance -Benchmarks were executed on MacBook Pro (Retina, 13-inch, Late 2013), 2,4 GHz Intel Core i5, 8 GB 1600 MHz DDR3. It has 4 cores and concurrency for linters was default: number of cores. Benchmark runs and measures timings automatically, it's code is [here](https://github.com/golangci/golangci-lint/blob/master/pkg/enabled_linters_test.go) (`BenchmarkWithGometalinter`). +Benchmarks were executed on MacBook Pro (Retina, 13-inch, Late 2013), 2,4 GHz Intel Core i5, 8 GB 1600 MHz DDR3. It has 4 cores and concurrency for linters was default: number of cores. Benchmark runs and measures timings automatically, it's code is [here](https://github.com/golangci/golangci-lint/blob/master/test/bench.go) (`BenchmarkWithGometalinter`). We measure peak memory usage (RSS) by tracking of processes RSS every 5 ms. diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 0e52ed89..7adc4d9a 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -44,10 +44,10 @@ func (e Executor) executeLinters(cmd *cobra.Command, args []string) { color.Green("\nLinters presets:") for _, p := range pkg.AllPresets() { - linters := pkg.GetAllLintersForPreset(p) + linters := pkg.GetAllLinterConfigsForPreset(p) linterNames := []string{} - for _, linter := range linters { - linterNames = append(linterNames, linter.Name()) + for _, lc := range linters { + linterNames = append(linterNames, lc.Linter.Name()) } fmt.Fprintf(printers.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", ")) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index eb9fc6df..151e2dca 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -4,23 +4,17 @@ import ( "context" "errors" "fmt" - "go/build" "go/token" "io/ioutil" "log" "os" - "os/exec" "runtime" "strings" "time" - "github.com/golangci/go-tools/ssa" - "github.com/golangci/go-tools/ssa/ssautil" "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/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/printers" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result/processors" @@ -28,7 +22,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" - "golang.org/x/tools/go/loader" ) const ( @@ -126,170 +119,6 @@ func (e *Executor) initRun() { e.parseConfig(runCmd) } -func isFullImportNeeded(linters []pkg.Linter) bool { - for _, linter := range linters { - lc := pkg.GetLinterConfig(linter.Name()) - if lc.DoesFullImport { - return true - } - } - - return false -} - -func isSSAReprNeeded(linters []pkg.Linter) bool { - for _, linter := range linters { - lc := pkg.GetLinterConfig(linter.Name()) - if lc.NeedsSSARepr { - return true - } - } - - return false -} - -func loadWholeAppIfNeeded(ctx context.Context, linters []pkg.Linter, cfg *config.Run, paths *fsutils.ProjectPaths) (*loader.Program, *loader.Config, error) { - if !isFullImportNeeded(linters) { - return nil, nil, nil - } - - startedAt := time.Now() - defer func() { - logrus.Infof("Program loading took %s", time.Since(startedAt)) - }() - - bctx := build.Default - bctx.BuildTags = append(bctx.BuildTags, cfg.BuildTags...) - loadcfg := &loader.Config{ - Build: &bctx, - AllowErrors: true, // Try to analyze event partially - } - rest, err := loadcfg.FromArgs(paths.MixedPaths(), cfg.AnalyzeTests) - if err != nil { - return nil, nil, fmt.Errorf("can't parepare load config with paths: %s", err) - } - if len(rest) > 0 { - return nil, nil, fmt.Errorf("unhandled loading paths: %v", rest) - } - - prog, err := loadcfg.Load() - if err != nil { - return nil, nil, fmt.Errorf("can't load paths: %s", err) - } - - return prog, loadcfg, nil -} - -func buildSSAProgram(ctx context.Context, lprog *loader.Program) *ssa.Program { - startedAt := time.Now() - defer func() { - logrus.Infof("SSA repr building took %s", time.Since(startedAt)) - }() - - ssaProg := ssautil.CreateProgram(lprog, ssa.GlobalDebug) - ssaProg.Build() - return ssaProg -} - -func discoverGoRoot() (string, error) { - goroot := os.Getenv("GOROOT") - if goroot != "" { - return goroot, nil - } - - output, err := exec.Command("go", "env", "GOROOT").Output() - if err != nil { - return "", fmt.Errorf("can't execute go env GOROOT: %s", err) - } - - return strings.TrimSpace(string(output)), nil -} - -// separateNotCompilingPackages moves not compiling packages into separate slices: -// a lot of linters crash on such packages. Leave them only for those linters -// which can work with them. -func separateNotCompilingPackages(lintCtx *golinters.Context) { - prog := lintCtx.Program - - if prog.Created != nil { - compilingCreated := make([]*loader.PackageInfo, 0, len(prog.Created)) - for _, info := range prog.Created { - if len(info.Errors) != 0 { - lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info) - } else { - compilingCreated = append(compilingCreated, info) - } - } - prog.Created = compilingCreated - } - - if prog.Imported != nil { - for k, info := range prog.Imported { - if len(info.Errors) != 0 { - lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info) - delete(prog.Imported, k) - } - } - } -} - -func buildLintCtx(ctx context.Context, linters []pkg.Linter, cfg *config.Config) (*golinters.Context, error) { - // Set GOROOT to have working cross-compilation: cross-compiled binaries - // have invalid GOROOT. XXX: can't use runtime.GOROOT(). - goroot, err := discoverGoRoot() - if err != nil { - return nil, fmt.Errorf("can't discover GOROOT: %s", err) - } - os.Setenv("GOROOT", goroot) - build.Default.GOROOT = goroot - logrus.Infof("set GOROOT=%q", goroot) - - args := cfg.Run.Args - if len(args) == 0 { - args = []string{"./..."} - } - - paths, err := fsutils.GetPathsForAnalysis(ctx, args, cfg.Run.AnalyzeTests) - if err != nil { - return nil, err - } - - prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, paths) - if err != nil { - return nil, err - } - - var ssaProg *ssa.Program - if prog != nil && isSSAReprNeeded(linters) { - ssaProg = buildSSAProgram(ctx, prog) - } - - var astCache *astcache.Cache - if prog != nil { - astCache, err = astcache.LoadFromProgram(prog) - if err != nil { - return nil, err - } - } else { - astCache = astcache.LoadFromFiles(paths.Files) - } - - ret := &golinters.Context{ - Paths: paths, - Cfg: cfg, - Program: prog, - SSAProgram: ssaProg, - LoaderConfig: loaderConfig, - ASTCache: astCache, - } - - if prog != nil { - separateNotCompilingPackages(ret) - } - - return ret, nil -} - func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) { e.cfg.Run.Args = args @@ -298,7 +127,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul return nil, err } - lintCtx, err := buildLintCtx(ctx, linters, e.cfg) + ctxLinters := make([]lint.LinterConfig, 0, len(linters)) + for _, lc := range linters { + ctxLinters = append(ctxLinters, lint.LinterConfig(lc)) + } + lintCtx, err := lint.BuildContext(ctx, ctxLinters, e.cfg) if err != nil { return nil, err } @@ -315,7 +148,7 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul if lintCtx.Program != nil { fset = lintCtx.Program.Fset } - runner := pkg.SimpleRunner{ + runner := lint.SimpleRunner{ Processors: []processors.Processor{ processors.NewPathPrettifier(), // must be before diff processor at least processors.NewExclude(excludeTotalPattern), @@ -329,7 +162,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul }, } - return runner.Run(ctx, linters, lintCtx), nil + runLinters := make([]lint.RunnerLinterConfig, 0, len(linters)) + for _, lc := range linters { + runLinters = append(runLinters, lint.RunnerLinterConfig(lc)) + } + return runner.Run(ctx, runLinters, lintCtx), nil } func setOutputToDevNull() (savedStdout, savedStderr *os.File) { @@ -364,7 +201,7 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error { p = printers.NewText(e.cfg.Output.PrintIssuedLine, e.cfg.Output.Format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName) } - gotAnyIssues, err := p.Print(issues) + gotAnyIssues, err := p.Print(ctx, issues) if err != nil { return fmt.Errorf("can't print %d issues: %s", len(issues), err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 4de22e5c..a994ce7d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -21,6 +21,7 @@ var DefaultExcludePatterns = []string{ // golint "should have comment", "comment on exported method", + "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this", // gas "G103:", // Use of unsafe calls should be audited diff --git a/pkg/enabled_linters.go b/pkg/enabled_linters.go index ce3c9f02..814d3279 100644 --- a/pkg/enabled_linters.go +++ b/pkg/enabled_linters.go @@ -8,6 +8,7 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/sirupsen/logrus" ) @@ -33,7 +34,7 @@ func allPresetsSet() map[string]bool { } type LinterConfig struct { - Linter Linter + Linter lint.Linter EnabledByDefault bool DoesFullImport bool NeedsSSARepr bool @@ -62,7 +63,23 @@ func (lc LinterConfig) WithSpeed(speed int) LinterConfig { return lc } -func newLinterConfig(linter Linter) LinterConfig { +func (lc LinterConfig) NeedsProgramLoading() bool { + return lc.DoesFullImport +} + +func (lc LinterConfig) NeedsSSARepresentation() bool { + return lc.NeedsSSARepr +} + +func (lc LinterConfig) GetSpeed() int { + return lc.Speed +} + +func (lc LinterConfig) GetLinter() lint.Linter { + return lc.Linter +} + +func newLinterConfig(linter lint.Linter) LinterConfig { return LinterConfig{ Linter: linter, } @@ -71,7 +88,7 @@ func newLinterConfig(linter Linter) LinterConfig { var nameToLC map[string]LinterConfig var nameToLCOnce sync.Once -func GetLinterConfig(name string) *LinterConfig { +func getLinterConfig(name string) *LinterConfig { nameToLCOnce.Do(func() { nameToLC = make(map[string]LinterConfig) for _, lc := range GetAllSupportedLinterConfigs() { @@ -158,44 +175,22 @@ func GetAllSupportedLinterConfigs() []LinterConfig { }) } -func getAllSupportedLinters() []Linter { - var ret []Linter - for _, lc := range GetAllSupportedLinterConfigs() { - ret = append(ret, lc.Linter) - } - - return ret -} - -func getAllEnabledByDefaultLinters() []Linter { - var ret []Linter +func getAllEnabledByDefaultLinters() []LinterConfig { + var ret []LinterConfig for _, lc := range GetAllSupportedLinterConfigs() { if lc.EnabledByDefault { - ret = append(ret, lc.Linter) + ret = append(ret, lc) } } return ret } -var supportedLintersByName map[string]Linter -var linterByNameMapOnce sync.Once - -func getLinterByName(name string) Linter { - linterByNameMapOnce.Do(func() { - supportedLintersByName = make(map[string]Linter) - for _, lc := range GetAllSupportedLinterConfigs() { - supportedLintersByName[lc.Linter.Name()] = lc.Linter - } - }) - - return supportedLintersByName[name] -} - -func lintersToMap(linters []Linter) map[string]Linter { - ret := map[string]Linter{} - for _, linter := range linters { - ret[linter.Name()] = linter +func linterConfigsToMap(lcs []LinterConfig) map[string]*LinterConfig { + ret := map[string]*LinterConfig{} + for _, lc := range lcs { + lc := lc // local copy + ret[lc.Linter.Name()] = &lc } return ret @@ -205,7 +200,7 @@ func validateLintersNames(cfg *config.Linters) error { allNames := append([]string{}, cfg.Enable...) allNames = append(allNames, cfg.Disable...) for _, name := range allNames { - if getLinterByName(name) == nil { + if getLinterConfig(name) == nil { return fmt.Errorf("no such linter %q", name) } } @@ -281,12 +276,12 @@ func validateEnabledDisabledLintersConfig(cfg *config.Linters) error { return nil } -func GetAllLintersForPreset(p string) []Linter { - ret := []Linter{} +func GetAllLinterConfigsForPreset(p string) []LinterConfig { + ret := []LinterConfig{} for _, lc := range GetAllSupportedLinterConfigs() { for _, ip := range lc.InPresets { if p == ip { - ret = append(ret, lc.Linter) + ret = append(ret, lc) break } } @@ -295,23 +290,23 @@ func GetAllLintersForPreset(p string) []Linter { return ret } -func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []Linter) map[string]Linter { // nolint:gocyclo - resultLintersSet := map[string]Linter{} +func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []LinterConfig) map[string]*LinterConfig { // nolint:gocyclo + resultLintersSet := map[string]*LinterConfig{} switch { case len(lcfg.Presets) != 0: break // imply --disable-all case lcfg.EnableAll: - resultLintersSet = lintersToMap(getAllSupportedLinters()) + resultLintersSet = linterConfigsToMap(GetAllSupportedLinterConfigs()) case lcfg.DisableAll: break default: - resultLintersSet = lintersToMap(enabledByDefaultLinters) + resultLintersSet = linterConfigsToMap(enabledByDefaultLinters) } // --presets can only add linters to default set for _, p := range lcfg.Presets { - for _, linter := range GetAllLintersForPreset(p) { - resultLintersSet[linter.Name()] = linter + for _, lc := range GetAllLinterConfigsForPreset(p) { + resultLintersSet[lc.Linter.Name()] = &lc } } @@ -320,14 +315,14 @@ func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []Linter // It should be before --enable and --disable to be able to enable or disable specific linter. if lcfg.Fast { for name := range resultLintersSet { - if GetLinterConfig(name).DoesFullImport { + if getLinterConfig(name).DoesFullImport { delete(resultLintersSet, name) } } } for _, name := range lcfg.Enable { - resultLintersSet[name] = getLinterByName(name) + resultLintersSet[name] = getLinterConfig(name) } for _, name := range lcfg.Disable { @@ -339,6 +334,7 @@ func getEnabledLintersSet(lcfg *config.Linters, enabledByDefaultLinters []Linter delete(resultLintersSet, name) } + optimizeLintersSet(resultLintersSet) return resultLintersSet } @@ -349,7 +345,7 @@ func getAllMegacheckSubLinterNames() []string { return []string{unusedName, gosimpleName, staticcheckName} } -func optimizeLintersSet(linters map[string]Linter) { +func optimizeLintersSet(linters map[string]*LinterConfig) { unusedName := golinters.Megacheck{UnusedEnabled: true}.Name() gosimpleName := golinters.Megacheck{GosimpleEnabled: true}.Name() staticcheckName := golinters.Megacheck{StaticcheckEnabled: true}.Name() @@ -378,20 +374,21 @@ func optimizeLintersSet(linters map[string]Linter) { delete(linters, n) } - linters[m.Name()] = m + lc := *getLinterConfig("megacheck") + lc.Linter = m + linters[m.Name()] = &lc } -func GetEnabledLinters(cfg *config.Config) ([]Linter, error) { +func GetEnabledLinters(cfg *config.Config) ([]LinterConfig, error) { if err := validateEnabledDisabledLintersConfig(&cfg.Linters); err != nil { return nil, err } resultLintersSet := getEnabledLintersSet(&cfg.Linters, getAllEnabledByDefaultLinters()) - optimizeLintersSet(resultLintersSet) - var resultLinters []Linter - for _, linter := range resultLintersSet { - resultLinters = append(resultLinters, linter) + var resultLinters []LinterConfig + for _, lc := range resultLintersSet { + resultLinters = append(resultLinters, *lc) } verbosePrintLintersStatus(cfg, resultLinters) @@ -413,10 +410,10 @@ func uniqStrings(ss []string) []string { return ret } -func verbosePrintLintersStatus(cfg *config.Config, linters []Linter) { +func verbosePrintLintersStatus(cfg *config.Config, lcs []LinterConfig) { var linterNames []string - for _, linter := range linters { - linterNames = append(linterNames, linter.Name()) + for _, lc := range lcs { + linterNames = append(linterNames, lc.Linter.Name()) } logrus.Infof("Active linters: %s", linterNames) diff --git a/pkg/enabled_linters_test.go b/pkg/enabled_linters_test.go index 9bd197b3..cf795c60 100644 --- a/pkg/enabled_linters_test.go +++ b/pkg/enabled_linters_test.go @@ -1,379 +1,13 @@ package pkg import ( - "bytes" - "fmt" - "go/build" - "os" - "os/exec" - "path/filepath" - "runtime" "sort" - "strconv" - "strings" - "sync" - "syscall" "testing" - "time" "github.com/golangci/golangci-lint/pkg/config" - gops "github.com/mitchellh/go-ps" - "github.com/shirou/gopsutil/process" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) -var installOnce sync.Once - -func installBinary(t assert.TestingT) { - installOnce.Do(func() { - cmd := exec.Command("go", "install", filepath.Join("..", "cmd", binName)) - assert.NoError(t, cmd.Run(), "Can't go install %s", binName) - }) -} - -func runGoErrchk(c *exec.Cmd, t *testing.T) { - output, err := c.CombinedOutput() - assert.NoError(t, err, "Output:\n%s", output) - - // Can't check exit code: tool only prints to output - assert.False(t, bytes.Contains(output, []byte("BUG")), "Output:\n%s", output) -} - -const testdataDir = "testdata" - -var testdataWithIssuesDir = filepath.Join(testdataDir, "with_issues") - -const binName = "golangci-lint" - -func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { - t.Log(filepath.Join(testdataWithIssuesDir, "*.go")) - sources, err := filepath.Glob(filepath.Join(testdataWithIssuesDir, "*.go")) - assert.NoError(t, err) - assert.NotEmpty(t, sources) - - installBinary(t) - - for _, s := range sources { - s := s - t.Run(s, func(t *testing.T) { - t.Parallel() - testOneSource(t, s) - }) - } -} - -func TestDeadlineExitCode(t *testing.T) { - installBinary(t) - - exitCode := runGolangciLintGetExitCode(t, "--no-config", "--deadline=1ms") - assert.Equal(t, 4, exitCode) -} - -func runGolangciLintGetExitCode(t *testing.T, args ...string) int { - runArgs := append([]string{"run"}, args...) - cmd := exec.Command("golangci-lint", runArgs...) - err := cmd.Run() - if err != nil { - if exitError, ok := err.(*exec.ExitError); ok { - ws := exitError.Sys().(syscall.WaitStatus) - return ws.ExitStatus() - } - - t.Fatalf("can't get error code from %s", err) - return -1 - } - - // success, exitCode should be 0 if go is ok - ws := cmd.ProcessState.Sys().(syscall.WaitStatus) - return ws.ExitStatus() -} - -func testOneSource(t *testing.T, sourcePath string) { - goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk") - cmd := exec.Command(goErrchkBin, binName, "run", - "--enable-all", - "--dupl.threshold=20", - "--gocyclo.min-complexity=20", - "--print-issued-lines=false", - "--print-linter-name=false", - "--out-format=line-number", - "--print-welcome=false", - "--govet.check-shadowing=true", - "--depguard.include-go-root", - "--depguard.packages='log'", - sourcePath) - runGoErrchk(cmd, t) -} - -func chdir(b *testing.B, dir string) { - if err := os.Chdir(dir); err != nil { - b.Fatalf("can't chdir to %s: %s", dir, err) - } -} - -func prepareGoSource(b *testing.B) { - chdir(b, filepath.Join(build.Default.GOROOT, "src")) -} - -func prepareGithubProject(owner, name string) func(*testing.B) { - return func(b *testing.B) { - dir := filepath.Join(build.Default.GOPATH, "src", "github.com", owner, name) - _, err := os.Stat(dir) - if os.IsNotExist(err) { - err = exec.Command("git", "clone", fmt.Sprintf("https://github.com/%s/%s.git", owner, name)).Run() - if err != nil { - b.Fatalf("can't git clone %s/%s: %s", owner, name, err) - } - } - chdir(b, dir) - } -} - -func getBenchLintersArgsNoMegacheck() []string { - return []string{ - "--enable=deadcode", - "--enable=gocyclo", - "--enable=golint", - "--enable=varcheck", - "--enable=structcheck", - "--enable=maligned", - "--enable=errcheck", - "--enable=dupl", - "--enable=ineffassign", - "--enable=interfacer", - "--enable=unconvert", - "--enable=goconst", - "--enable=gas", - } -} - -func getBenchLintersArgs() []string { - return append([]string{ - "--enable=megacheck", - }, getBenchLintersArgsNoMegacheck()...) -} - -func getGometalinterCommonArgs() []string { - return []string{ - "--deadline=30m", - "--skip=testdata", - "--skip=builtin", - "--vendor", - "--cyclo-over=30", - "--dupl-threshold=150", - "--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")), - "--disable-all", - "--enable=vet", - "--enable=vetshadow", - } -} - -func printCommand(cmd string, args ...string) { - if os.Getenv("PRINT_CMD") != "1" { - return - } - quotedArgs := []string{} - for _, a := range args { - quotedArgs = append(quotedArgs, strconv.Quote(a)) - } - - logrus.Warnf("%s %s", cmd, strings.Join(quotedArgs, " ")) -} - -func runGometalinter(b *testing.B) { - args := []string{} - args = append(args, getGometalinterCommonArgs()...) - args = append(args, getBenchLintersArgs()...) - args = append(args, "./...") - - printCommand("gometalinter", args...) - _ = exec.Command("gometalinter", args...).Run() -} - -func getGolangciLintCommonArgs() []string { - return []string{"run", "--no-config", "--issues-exit-code=0", "--deadline=30m", "--disable-all", "--enable=govet"} -} - -func runGolangciLint(b *testing.B) { - args := getGolangciLintCommonArgs() - args = append(args, getBenchLintersArgs()...) - printCommand("golangci-lint", args...) - out, err := exec.Command("golangci-lint", args...).CombinedOutput() - if err != nil { - b.Fatalf("can't run golangci-lint: %s, %s", err, out) - } -} - -func getGoLinesTotalCount(b *testing.B) int { - cmd := exec.Command("bash", "-c", `find . -name "*.go" | fgrep -v vendor | xargs wc -l | tail -1`) - out, err := cmd.CombinedOutput() - if err != nil { - b.Fatalf("can't run go lines counter: %s", err) - } - - parts := bytes.Split(bytes.TrimSpace(out), []byte(" ")) - n, err := strconv.Atoi(string(parts[0])) - if err != nil { - b.Fatalf("can't parse go lines count: %s", err) - } - - return n -} - -func getLinterMemoryMB(b *testing.B, progName string) (int, error) { - processes, err := gops.Processes() - if err != nil { - b.Fatalf("Can't get processes: %s", err) - } - - var progPID int - for _, p := range processes { - if p.Executable() == progName { - progPID = p.Pid() - break - } - } - if progPID == 0 { - return 0, fmt.Errorf("no process") - } - - allProgPIDs := []int{progPID} - for _, p := range processes { - if p.PPid() == progPID { - allProgPIDs = append(allProgPIDs, p.Pid()) - } - } - - var totalProgMemBytes uint64 - for _, pid := range allProgPIDs { - p, err := process.NewProcess(int32(pid)) - if err != nil { - continue // subprocess could die - } - - mi, err := p.MemoryInfo() - if err != nil { - continue - } - - totalProgMemBytes += mi.RSS - } - - return int(totalProgMemBytes / 1024 / 1024), nil -} - -func trackPeakMemoryUsage(b *testing.B, doneCh <-chan struct{}, progName string) chan int { - resCh := make(chan int) - go func() { - var peakUsedMemMB int - t := time.NewTicker(time.Millisecond * 5) - defer t.Stop() - - for { - select { - case <-doneCh: - resCh <- peakUsedMemMB - close(resCh) - return - case <-t.C: - } - - m, err := getLinterMemoryMB(b, progName) - if err != nil { - continue - } - if m > peakUsedMemMB { - peakUsedMemMB = m - } - } - }() - return resCh -} - -type runResult struct { - peakMemMB int - duration time.Duration -} - -func compare(b *testing.B, gometalinterRun, golangciLintRun func(*testing.B), repoName, mode string, kLOC int) { // nolint - gometalinterRes := runOne(b, gometalinterRun, "gometalinter") - golangciLintRes := runOne(b, golangciLintRun, "golangci-lint") - - if mode != "" { - mode = " " + mode - } - logrus.Warnf("%s (%d kLoC): golangci-lint%s: time: %s, %.1f times faster; memory: %dMB, %.1f times less", - repoName, kLOC, mode, - golangciLintRes.duration, gometalinterRes.duration.Seconds()/golangciLintRes.duration.Seconds(), - golangciLintRes.peakMemMB, float64(gometalinterRes.peakMemMB)/float64(golangciLintRes.peakMemMB), - ) -} - -func runOne(b *testing.B, run func(*testing.B), progName string) *runResult { - doneCh := make(chan struct{}) - peakMemCh := trackPeakMemoryUsage(b, doneCh, progName) - startedAt := time.Now() - run(b) - duration := time.Since(startedAt) - close(doneCh) - - peakUsedMemMB := <-peakMemCh - return &runResult{ - peakMemMB: peakUsedMemMB, - duration: duration, - } -} - -func BenchmarkWithGometalinter(b *testing.B) { - installBinary(b) - - type bcase struct { - name string - prepare func(*testing.B) - } - bcases := []bcase{ - { - name: "self repo", - prepare: prepareGithubProject("golangci", "golangci-lint"), - }, - { - name: "gometalinter repo", - prepare: prepareGithubProject("alecthomas", "gometalinter"), - }, - { - name: "hugo", - prepare: prepareGithubProject("gohugoio", "hugo"), - }, - { - name: "go-ethereum", - prepare: prepareGithubProject("ethereum", "go-ethereum"), - }, - { - name: "beego", - prepare: prepareGithubProject("astaxie", "beego"), - }, - { - name: "terraform", - prepare: prepareGithubProject("hashicorp", "terraform"), - }, - { - name: "consul", - prepare: prepareGithubProject("hashicorp", "consul"), - }, - { - name: "go source code", - prepare: prepareGoSource, - }, - } - for _, bc := range bcases { - bc.prepare(b) - lc := getGoLinesTotalCount(b) - - compare(b, runGometalinter, runGolangciLint, bc.name, "", lc/1000) - } -} - func TestGetEnabledLintersSet(t *testing.T) { type cs struct { cfg config.Linters @@ -395,19 +29,30 @@ func TestGetEnabledLintersSet(t *testing.T) { }, name: "disable only staticcheck", def: getAllMegacheckSubLinterNames(), - exp: []string{"gosimple", "unused"}, + exp: []string{"megacheck.{unused,gosimple}"}, + }, + { + name: "merge into megacheck", + def: getAllMegacheckSubLinterNames(), + exp: []string{"megacheck"}, + }, + { + name: "don't disable anything", + def: []string{"gofmt", "govet"}, + exp: []string{"gofmt", "govet"}, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - defaultLinters := []Linter{} + defaultLinters := []LinterConfig{} for _, ln := range c.def { - defaultLinters = append(defaultLinters, getLinterByName(ln)) + defaultLinters = append(defaultLinters, *getLinterConfig(ln)) } els := getEnabledLintersSet(&c.cfg, defaultLinters) var enabledLinters []string - for ln := range els { + for ln, lc := range els { + assert.Equal(t, ln, lc.Linter.Name()) enabledLinters = append(enabledLinters, ln) } diff --git a/pkg/fsutils/path_resolver.go b/pkg/fsutils/path_resolver.go index c5a8669f..9bef6633 100644 --- a/pkg/fsutils/path_resolver.go +++ b/pkg/fsutils/path_resolver.go @@ -80,7 +80,7 @@ func (pr PathResolver) isIgnoredDir(dir string) bool { // https://github.com/golang/dep/issues/298 // https://github.com/tools/godep/issues/140 - if strings.HasPrefix(dirName, ".") && dirName != "." { + if strings.HasPrefix(dirName, ".") && dirName != "." && dirName != ".." { return true } if strings.HasPrefix(dirName, "_") { diff --git a/pkg/golinters/context.go b/pkg/golinters/context.go deleted file mode 100644 index fbe31b7e..00000000 --- a/pkg/golinters/context.go +++ /dev/null @@ -1,23 +0,0 @@ -package golinters - -import ( - "github.com/golangci/go-tools/ssa" - "github.com/golangci/golangci-lint/pkg/astcache" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" - "golang.org/x/tools/go/loader" -) - -type Context struct { - Paths *fsutils.ProjectPaths - Cfg *config.Config - Program *loader.Program - SSAProgram *ssa.Program - LoaderConfig *loader.Config - ASTCache *astcache.Cache - NotCompilingPackages []*loader.PackageInfo -} - -func (c *Context) Settings() *config.LintersSettings { - return &c.Cfg.LintersSettings -} diff --git a/pkg/golinters/deadcode.go b/pkg/golinters/deadcode.go index c7a16609..bb6e89c2 100644 --- a/pkg/golinters/deadcode.go +++ b/pkg/golinters/deadcode.go @@ -5,6 +5,7 @@ import ( "fmt" deadcodeAPI "github.com/golangci/go-misc/deadcode" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +19,7 @@ func (Deadcode) Desc() string { return "Finds unused code" } -func (d Deadcode) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (d Deadcode) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues, err := deadcodeAPI.Run(lintCtx.Program) if err != nil { return nil, err diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 8bef16a2..3c3987ea 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -6,6 +6,7 @@ import ( "strings" depguardAPI "github.com/OpenPeeDeeP/depguard" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -19,7 +20,7 @@ func (Depguard) Desc() string { return "Go linter that checks if package imports are in a list of acceptable packages" } -func (d Depguard) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (d Depguard) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { dg := &depguardAPI.Depguard{ Packages: lintCtx.Settings().Depguard.Packages, IncludeGoRoot: lintCtx.Settings().Depguard.IncludeGoRoot, diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index dafd4fdb..d9963620 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -5,6 +5,7 @@ import ( "fmt" "go/token" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" duplAPI "github.com/mibk/dupl" ) @@ -19,7 +20,7 @@ func (Dupl) Desc() string { return "Tool for code clone detection" } -func (d Dupl) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (d Dupl) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues, err := duplAPI.Run(lintCtx.Paths.Files, lintCtx.Settings().Dupl.Threshold) if err != nil { return nil, err diff --git a/pkg/golinters/errcheck.go b/pkg/golinters/errcheck.go index 5695ba22..fbd59afd 100644 --- a/pkg/golinters/errcheck.go +++ b/pkg/golinters/errcheck.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" errcheckAPI "github.com/kisielk/errcheck/golangci" ) @@ -18,7 +19,7 @@ func (Errcheck) Desc() string { return "Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases" } -func (e Errcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (e Errcheck) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { errCfg := &lintCtx.Settings().Errcheck issues, err := errcheckAPI.Run(lintCtx.Program, errCfg.CheckAssignToBlank, errCfg.CheckTypeAssertions) if err != nil { diff --git a/pkg/golinters/gas.go b/pkg/golinters/gas.go index ccf5edc4..58c5c2ee 100644 --- a/pkg/golinters/gas.go +++ b/pkg/golinters/gas.go @@ -10,6 +10,7 @@ import ( "github.com/GoASTScanner/gas" "github.com/GoASTScanner/gas/rules" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" "github.com/sirupsen/logrus" ) @@ -24,7 +25,7 @@ func (Gas) Desc() string { return "Inspects source code for security problems" } -func (lint Gas) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint Gas) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { gasConfig := gas.NewConfig() enabledRules := rules.Generate() logger := log.New(ioutil.Discard, "", 0) diff --git a/pkg/golinters/goconst.go b/pkg/golinters/goconst.go index 241709bd..09ee3628 100644 --- a/pkg/golinters/goconst.go +++ b/pkg/golinters/goconst.go @@ -5,6 +5,7 @@ import ( "fmt" goconstAPI "github.com/golangci/goconst" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +19,7 @@ func (Goconst) Desc() string { return "Finds repeated strings that could be replaced by a constant" } -func (lint Goconst) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint Goconst) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { var goconstIssues []goconstAPI.Issue // TODO: make it cross-package: pass package names inside goconst for _, files := range lintCtx.Paths.FilesGrouppedByDirs() { diff --git a/pkg/golinters/gocyclo.go b/pkg/golinters/gocyclo.go index e54fc0dc..8945abea 100644 --- a/pkg/golinters/gocyclo.go +++ b/pkg/golinters/gocyclo.go @@ -6,6 +6,7 @@ import ( "sort" gocycloAPI "github.com/golangci/gocyclo/pkg/gocyclo" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -19,7 +20,7 @@ func (Gocyclo) Desc() string { return "Computes and checks the cyclomatic complexity of functions" } -func (g Gocyclo) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (g Gocyclo) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { var stats []gocycloAPI.Stat for _, f := range lintCtx.ASTCache.GetAllValidFiles() { stats = gocycloAPI.BuildStats(f.F, f.Fset, stats) diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 62f8c60b..d4cc9cae 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -8,6 +8,7 @@ import ( gofmtAPI "github.com/golangci/gofmt/gofmt" goimportsAPI "github.com/golangci/gofmt/goimports" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" "github.com/sirupsen/logrus" "sourcegraph.com/sourcegraph/go-diff/diff" @@ -101,7 +102,7 @@ func (g Gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { return issues, nil } -func (g Gofmt) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (g Gofmt) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { var issues []result.Issue for _, f := range lintCtx.Paths.Files { diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 19048d58..c26f9fc4 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -5,7 +5,8 @@ import ( "fmt" "io/ioutil" - "github.com/golang/lint" + lintAPI "github.com/golang/lint" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" "github.com/sirupsen/logrus" ) @@ -20,7 +21,7 @@ func (Golint) Desc() string { return "Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes" } -func (g Golint) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (g Golint) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { var issues []result.Issue var lintErr error for _, pkgFiles := range lintCtx.Paths.FilesGrouppedByDirs() { @@ -48,7 +49,7 @@ func (g Golint) lintFiles(minConfidence float64, filenames ...string) ([]result. files[filename] = src } - l := new(lint.Linter) + l := new(lintAPI.Linter) ps, err := l.LintFiles(files) if err != nil { return nil, fmt.Errorf("can't lint files %s: %s", filenames, err) diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 229ea283..c50639c6 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -3,6 +3,7 @@ package golinters import ( "context" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" govetAPI "github.com/golangci/govet" ) @@ -17,7 +18,7 @@ func (Govet) Desc() string { return "Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string" } -func (g Govet) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (g Govet) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { // TODO: check .S asm files: govet can do it if pass dirs var govetIssues []govetAPI.Issue for _, files := range lintCtx.Paths.FilesGrouppedByDirs() { diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go index 51e666d2..cb7a75f1 100644 --- a/pkg/golinters/ineffassign.go +++ b/pkg/golinters/ineffassign.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ineffassignAPI "github.com/golangci/ineffassign" ) @@ -18,7 +19,7 @@ func (Ineffassign) Desc() string { return "Detects when assignments to existing variables are not used" } -func (lint Ineffassign) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint Ineffassign) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues := ineffassignAPI.Run(lintCtx.Paths.Files) if len(issues) == 0 { return nil, nil diff --git a/pkg/golinters/interfacer.go b/pkg/golinters/interfacer.go index e55942b1..becc50ce 100644 --- a/pkg/golinters/interfacer.go +++ b/pkg/golinters/interfacer.go @@ -5,6 +5,7 @@ import ( "mvdan.cc/interfacer/check" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +19,7 @@ func (Interfacer) Desc() string { return "Linter that suggests narrower interface types" } -func (lint Interfacer) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint Interfacer) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { c := new(check.Checker) c.Program(lintCtx.Program) c.ProgramSSA(lintCtx.SSAProgram) diff --git a/pkg/golinters/maligned.go b/pkg/golinters/maligned.go index 3e0f8198..a79d633e 100644 --- a/pkg/golinters/maligned.go +++ b/pkg/golinters/maligned.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" malignedAPI "github.com/golangci/maligned" ) @@ -18,7 +19,7 @@ func (Maligned) Desc() string { return "Tool to detect Go structs that would take less memory if their fields were sorted" } -func (m Maligned) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (m Maligned) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues := malignedAPI.Run(lintCtx.Program) if len(issues) == 0 { return nil, nil diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go index f9141d79..45f0392d 100644 --- a/pkg/golinters/megacheck.go +++ b/pkg/golinters/megacheck.go @@ -2,8 +2,11 @@ package golinters import ( "context" + "fmt" + "strings" megacheckAPI "github.com/golangci/go-tools/cmd/megacheck" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -14,17 +17,26 @@ type Megacheck struct { } func (m Megacheck) Name() string { - if m.UnusedEnabled && !m.GosimpleEnabled && !m.StaticcheckEnabled { - return "unused" + names := []string{} + if m.UnusedEnabled { + names = append(names, "unused") } - if m.GosimpleEnabled && !m.UnusedEnabled && !m.StaticcheckEnabled { - return "gosimple" + if m.GosimpleEnabled { + names = append(names, "gosimple") } - if m.StaticcheckEnabled && !m.UnusedEnabled && !m.GosimpleEnabled { - return "staticcheck" + if m.StaticcheckEnabled { + names = append(names, "staticcheck") } - return "megacheck" // all enabled + if len(names) == 1 { + return names[0] // only one sublinter is enabled + } + + if len(names) == 3 { + return "megacheck" // all enabled + } + + return fmt.Sprintf("megacheck.{%s}", strings.Join(names, ",")) } func (m Megacheck) Desc() string { @@ -38,7 +50,7 @@ func (m Megacheck) Desc() string { return descs[m.Name()] } -func (m Megacheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (m Megacheck) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues := megacheckAPI.Run(lintCtx.Program, lintCtx.LoaderConfig, lintCtx.SSAProgram, m.StaticcheckEnabled, m.GosimpleEnabled, m.UnusedEnabled) if len(issues) == 0 { diff --git a/pkg/golinters/structcheck.go b/pkg/golinters/structcheck.go index 3f015f8d..dd87a50c 100644 --- a/pkg/golinters/structcheck.go +++ b/pkg/golinters/structcheck.go @@ -5,6 +5,7 @@ import ( "fmt" structcheckAPI "github.com/golangci/check/cmd/structcheck" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +19,7 @@ func (Structcheck) Desc() string { return "Finds unused struct fields" } -func (s Structcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (s Structcheck) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues := structcheckAPI.Run(lintCtx.Program, lintCtx.Settings().Structcheck.CheckExportedFields) if len(issues) == 0 { return nil, nil diff --git a/pkg/golinters/typecheck.go b/pkg/golinters/typecheck.go index dcea36cd..01ad9862 100644 --- a/pkg/golinters/typecheck.go +++ b/pkg/golinters/typecheck.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -61,7 +62,7 @@ func (lint TypeCheck) parseError(err error) *result.Issue { } } -func (lint TypeCheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint TypeCheck) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { if lintCtx.NotCompilingPackages == nil { return nil, nil } diff --git a/pkg/golinters/unconvert.go b/pkg/golinters/unconvert.go index 768967fb..7f38f8c4 100644 --- a/pkg/golinters/unconvert.go +++ b/pkg/golinters/unconvert.go @@ -3,6 +3,7 @@ package golinters import ( "context" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" unconvertAPI "github.com/golangci/unconvert" ) @@ -17,7 +18,7 @@ func (Unconvert) Desc() string { return "Remove unnecessary type conversions" } -func (lint Unconvert) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (lint Unconvert) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { positions := unconvertAPI.Run(lintCtx.Program) if len(positions) == 0 { return nil, nil diff --git a/pkg/golinters/varcheck.go b/pkg/golinters/varcheck.go index 35d89999..05aa9c1d 100644 --- a/pkg/golinters/varcheck.go +++ b/pkg/golinters/varcheck.go @@ -5,6 +5,7 @@ import ( "fmt" varcheckAPI "github.com/golangci/check/cmd/varcheck" + "github.com/golangci/golangci-lint/pkg/lint" "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +19,7 @@ func (Varcheck) Desc() string { return "Finds unused global variables and constants" } -func (v Varcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { +func (v Varcheck) Run(ctx context.Context, lintCtx *lint.Context) ([]result.Issue, error) { issues := varcheckAPI.Run(lintCtx.Program, lintCtx.Settings().Varcheck.CheckExportedFields) if len(issues) == 0 { return nil, nil diff --git a/pkg/astcache/astcache.go b/pkg/lint/astcache/astcache.go similarity index 100% rename from pkg/astcache/astcache.go rename to pkg/lint/astcache/astcache.go diff --git a/pkg/lint/context.go b/pkg/lint/context.go new file mode 100644 index 00000000..b75369b0 --- /dev/null +++ b/pkg/lint/context.go @@ -0,0 +1,200 @@ +package lint + +import ( + "context" + "fmt" + "go/build" + "os" + "os/exec" + "strings" + "time" + + "github.com/golangci/go-tools/ssa" + "github.com/golangci/go-tools/ssa/ssautil" + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/lint/astcache" + "github.com/sirupsen/logrus" + "golang.org/x/tools/go/loader" +) + +type Context struct { + Paths *fsutils.ProjectPaths + Cfg *config.Config + Program *loader.Program + SSAProgram *ssa.Program + LoaderConfig *loader.Config + ASTCache *astcache.Cache + NotCompilingPackages []*loader.PackageInfo +} + +func (c *Context) Settings() *config.LintersSettings { + return &c.Cfg.LintersSettings +} + +type LinterConfig interface { + NeedsProgramLoading() bool + NeedsSSARepresentation() bool +} + +func isFullImportNeeded(linters []LinterConfig) bool { + for _, linter := range linters { + if linter.NeedsProgramLoading() { + return true + } + } + + return false +} + +func isSSAReprNeeded(linters []LinterConfig) bool { + for _, linter := range linters { + if linter.NeedsSSARepresentation() { + return true + } + } + + return false +} + +func loadWholeAppIfNeeded(ctx context.Context, linters []LinterConfig, cfg *config.Run, paths *fsutils.ProjectPaths) (*loader.Program, *loader.Config, error) { + if !isFullImportNeeded(linters) { + return nil, nil, nil + } + + startedAt := time.Now() + defer func() { + logrus.Infof("Program loading took %s", time.Since(startedAt)) + }() + + bctx := build.Default + bctx.BuildTags = append(bctx.BuildTags, cfg.BuildTags...) + loadcfg := &loader.Config{ + Build: &bctx, + AllowErrors: true, // Try to analyze event partially + } + rest, err := loadcfg.FromArgs(paths.MixedPaths(), cfg.AnalyzeTests) + if err != nil { + return nil, nil, fmt.Errorf("can't parepare load config with paths: %s", err) + } + if len(rest) > 0 { + return nil, nil, fmt.Errorf("unhandled loading paths: %v", rest) + } + + prog, err := loadcfg.Load() + if err != nil { + return nil, nil, fmt.Errorf("can't load program from paths %v: %s", paths.MixedPaths(), err) + } + + return prog, loadcfg, nil +} + +func buildSSAProgram(ctx context.Context, lprog *loader.Program) *ssa.Program { + startedAt := time.Now() + defer func() { + logrus.Infof("SSA repr building took %s", time.Since(startedAt)) + }() + + ssaProg := ssautil.CreateProgram(lprog, ssa.GlobalDebug) + ssaProg.Build() + return ssaProg +} + +func discoverGoRoot() (string, error) { + goroot := os.Getenv("GOROOT") + if goroot != "" { + return goroot, nil + } + + output, err := exec.Command("go", "env", "GOROOT").Output() + if err != nil { + return "", fmt.Errorf("can't execute go env GOROOT: %s", err) + } + + return strings.TrimSpace(string(output)), nil +} + +// separateNotCompilingPackages moves not compiling packages into separate slices: +// a lot of linters crash on such packages. Leave them only for those linters +// which can work with them. +func separateNotCompilingPackages(lintCtx *Context) { + prog := lintCtx.Program + + if prog.Created != nil { + compilingCreated := make([]*loader.PackageInfo, 0, len(prog.Created)) + for _, info := range prog.Created { + if len(info.Errors) != 0 { + lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info) + } else { + compilingCreated = append(compilingCreated, info) + } + } + prog.Created = compilingCreated + } + + if prog.Imported != nil { + for k, info := range prog.Imported { + if len(info.Errors) != 0 { + lintCtx.NotCompilingPackages = append(lintCtx.NotCompilingPackages, info) + delete(prog.Imported, k) + } + } + } +} + +func BuildContext(ctx context.Context, linters []LinterConfig, cfg *config.Config) (*Context, error) { + // Set GOROOT to have working cross-compilation: cross-compiled binaries + // have invalid GOROOT. XXX: can't use runtime.GOROOT(). + goroot, err := discoverGoRoot() + if err != nil { + return nil, fmt.Errorf("can't discover GOROOT: %s", err) + } + os.Setenv("GOROOT", goroot) + build.Default.GOROOT = goroot + logrus.Infof("set GOROOT=%q", goroot) + + args := cfg.Run.Args + if len(args) == 0 { + args = []string{"./..."} + } + + paths, err := fsutils.GetPathsForAnalysis(ctx, args, cfg.Run.AnalyzeTests) + if err != nil { + return nil, err + } + + prog, loaderConfig, err := loadWholeAppIfNeeded(ctx, linters, &cfg.Run, paths) + if err != nil { + return nil, err + } + + var ssaProg *ssa.Program + if prog != nil && isSSAReprNeeded(linters) { + ssaProg = buildSSAProgram(ctx, prog) + } + + var astCache *astcache.Cache + if prog != nil { + astCache, err = astcache.LoadFromProgram(prog) + if err != nil { + return nil, err + } + } else { + astCache = astcache.LoadFromFiles(paths.Files) + } + + ret := &Context{ + Paths: paths, + Cfg: cfg, + Program: prog, + SSAProgram: ssaProg, + LoaderConfig: loaderConfig, + ASTCache: astCache, + } + + if prog != nil { + separateNotCompilingPackages(ret) + } + + return ret, nil +} diff --git a/pkg/commands/run_test.go b/pkg/lint/context_test.go similarity index 76% rename from pkg/commands/run_test.go rename to pkg/lint/context_test.go index 53024ca0..a236c744 100644 --- a/pkg/commands/run_test.go +++ b/pkg/lint/context_test.go @@ -1,22 +1,30 @@ -package commands +package lint 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/golangci/golangci-lint/pkg/lint/astcache" "github.com/stretchr/testify/assert" ) +type testLinterConfig struct{} + +func (t testLinterConfig) NeedsProgramLoading() bool { + return true +} + +func (t testLinterConfig) NeedsSSARepresentation() bool { + return true +} + func TestASTCacheLoading(t *testing.T) { ctx := context.Background() - linters := []pkg.Linter{golinters.Errcheck{}} + linters := []LinterConfig{testLinterConfig{}} - inputPaths := []string{"./...", "./", "./run.go", "run.go"} + inputPaths := []string{"./...", "./", "./context.go", "context.go"} for _, inputPath := range inputPaths { paths, err := fsutils.GetPathsForAnalysis(ctx, []string{inputPath}, true) assert.NoError(t, err) diff --git a/pkg/lint/linter.go b/pkg/lint/linter.go new file mode 100644 index 00000000..1dd58017 --- /dev/null +++ b/pkg/lint/linter.go @@ -0,0 +1,13 @@ +package lint + +import ( + "context" + + "github.com/golangci/golangci-lint/pkg/result" +) + +type Linter interface { + Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) + Name() string + Desc() string +} diff --git a/pkg/runner.go b/pkg/lint/runner.go similarity index 85% rename from pkg/runner.go rename to pkg/lint/runner.go index 952c1ea5..09038027 100644 --- a/pkg/runner.go +++ b/pkg/lint/runner.go @@ -1,4 +1,4 @@ -package pkg +package lint import ( "context" @@ -9,7 +9,6 @@ import ( "sync" "time" - "github.com/golangci/golangci-lint/pkg/golinters" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result/processors" "github.com/golangci/golangci-lint/pkg/timeutils" @@ -26,7 +25,7 @@ type lintRes struct { issues []result.Issue } -func runLinterSafe(ctx context.Context, lintCtx *golinters.Context, linter Linter) (ret []result.Issue, err error) { +func runLinterSafe(ctx context.Context, lintCtx *Context, linter Linter) (ret []result.Issue, err error) { defer func() { if panicData := recover(); panicData != nil { err = fmt.Errorf("panic occured: %s", panicData) @@ -37,7 +36,7 @@ func runLinterSafe(ctx context.Context, lintCtx *golinters.Context, linter Linte return linter.Run(ctx, lintCtx) } -func runWorker(ctx context.Context, lintCtx *golinters.Context, tasksCh <-chan Linter, lintResultsCh chan<- lintRes, name string) { +func runWorker(ctx context.Context, lintCtx *Context, tasksCh <-chan Linter, lintResultsCh chan<- lintRes, name string) { sw := timeutils.NewStopwatch(name) defer sw.Print() @@ -88,20 +87,23 @@ func logWorkersStat(workersFinishTimes []time.Time) { logrus.Infof("Workers idle times: %s", strings.Join(logStrings, ", ")) } -func getSortedLintersConfigs(linters []Linter) []LinterConfig { - ret := make([]LinterConfig, 0, len(linters)) - for _, linter := range linters { - ret = append(ret, *GetLinterConfig(linter.Name())) - } +type RunnerLinterConfig interface { + GetSpeed() int + GetLinter() Linter +} + +func getSortedLintersConfigs(linters []RunnerLinterConfig) []RunnerLinterConfig { + ret := make([]RunnerLinterConfig, len(linters)) + copy(ret, linters) sort.Slice(ret, func(i, j int) bool { - return ret[i].Speed < ret[j].Speed + return ret[i].GetSpeed() < ret[j].GetSpeed() }) return ret } -func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *golinters.Context, linters []Linter) <-chan lintRes { +func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *Context, linters []RunnerLinterConfig) <-chan lintRes { tasksCh := make(chan Linter, len(linters)) lintResultsCh := make(chan lintRes, len(linters)) var wg sync.WaitGroup @@ -120,7 +122,7 @@ func (r *SimpleRunner) runWorkers(ctx context.Context, lintCtx *golinters.Contex lcs := getSortedLintersConfigs(linters) for _, lc := range lcs { - tasksCh <- lc.Linter + tasksCh <- lc.GetLinter() } close(tasksCh) @@ -187,7 +189,7 @@ func collectIssues(ctx context.Context, resCh <-chan lintRes) <-chan result.Issu return retIssues } -func (r SimpleRunner) Run(ctx context.Context, linters []Linter, lintCtx *golinters.Context) <-chan result.Issue { +func (r SimpleRunner) Run(ctx context.Context, linters []RunnerLinterConfig, lintCtx *Context) <-chan result.Issue { lintResultsCh := r.runWorkers(ctx, lintCtx, linters) processedLintResultsCh := r.processLintResults(ctx, lintResultsCh) if ctx.Err() != nil { diff --git a/pkg/linter.go b/pkg/linter.go deleted file mode 100644 index 7405ef2f..00000000 --- a/pkg/linter.go +++ /dev/null @@ -1,14 +0,0 @@ -package pkg - -import ( - "context" - - "github.com/golangci/golangci-lint/pkg/golinters" - "github.com/golangci/golangci-lint/pkg/result" -) - -type Linter interface { - Run(ctx context.Context, lintCtx *golinters.Context) ([]result.Issue, error) - Name() string - Desc() string -} diff --git a/pkg/printers/json.go b/pkg/printers/json.go index 38987344..45ab98a9 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -1,6 +1,7 @@ package printers import ( + "context" "encoding/json" "fmt" @@ -13,7 +14,7 @@ func NewJSON() *JSON { return &JSON{} } -func (JSON) Print(issues <-chan result.Issue) (bool, error) { +func (JSON) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { var allIssues []result.Issue for i := range issues { allIssues = append(allIssues, i) diff --git a/pkg/printers/printer.go b/pkg/printers/printer.go index 0c1275cd..667e6408 100644 --- a/pkg/printers/printer.go +++ b/pkg/printers/printer.go @@ -1,7 +1,11 @@ package printers -import "github.com/golangci/golangci-lint/pkg/result" +import ( + "context" + + "github.com/golangci/golangci-lint/pkg/result" +) type Printer interface { - Print(issues <-chan result.Issue) (bool, error) + Print(ctx context.Context, issues <-chan result.Issue) (bool, error) } diff --git a/pkg/printers/text.go b/pkg/printers/text.go index fe94bfe2..6d4c3329 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -2,6 +2,7 @@ package printers import ( "bytes" + "context" "fmt" "io/ioutil" "strings" @@ -58,7 +59,7 @@ func (p *Text) getFileLinesForIssue(i *result.Issue) (linesCache, error) { return fc, nil } -func (p *Text) Print(issues <-chan result.Issue) (bool, error) { +func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { var issuedLineExtractingDuration time.Duration defer func() { logrus.Infof("Extracting issued lines took %s", issuedLineExtractingDuration) @@ -86,11 +87,11 @@ func (p *Text) Print(issues <-chan result.Issue) (bool, error) { } } - if issuesN == 0 { + if issuesN != 0 { + logrus.Infof("Found %d issues", issuesN) + } else if ctx.Err() == nil { // don't print "congrats" if timeouted outStr := p.SprintfColored(color.FgGreen, "Congrats! No issues were found.") fmt.Fprintln(StdOut, outStr) - } else { - logrus.Infof("Found %d issues", issuesN) } return issuesN != 0, nil diff --git a/pkg/result/processors/max_per_file_from_linter.go b/pkg/result/processors/max_per_file_from_linter.go index ae8319a5..29d535b1 100644 --- a/pkg/result/processors/max_per_file_from_linter.go +++ b/pkg/result/processors/max_per_file_from_linter.go @@ -1,7 +1,6 @@ package processors import ( - "github.com/golangci/golangci-lint/pkg/golinters" "github.com/golangci/golangci-lint/pkg/result" ) @@ -25,9 +24,9 @@ func (p MaxPerFileFromLinter) Name() string { } var maxPerFileFromLinterConfig = map[string]int{ - golinters.Gofmt{}.Name(): 1, - golinters.Gofmt{UseGoimports: true}.Name(): 1, - golinters.TypeCheck{}.Name(): 3, + "gofmt": 1, + "goimports": 1, + "typecheck": 3, } func (p *MaxPerFileFromLinter) Process(issues []result.Issue) ([]result.Issue, error) { diff --git a/test/bench_test.go b/test/bench_test.go new file mode 100644 index 00000000..f919f7e9 --- /dev/null +++ b/test/bench_test.go @@ -0,0 +1,287 @@ +package test + +import ( + "bytes" + "fmt" + "go/build" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/golangci/golangci-lint/pkg/config" + gops "github.com/mitchellh/go-ps" + "github.com/shirou/gopsutil/process" + "github.com/sirupsen/logrus" +) + +func chdir(b *testing.B, dir string) { + if err := os.Chdir(dir); err != nil { + b.Fatalf("can't chdir to %s: %s", dir, err) + } +} + +func prepareGoSource(b *testing.B) { + chdir(b, filepath.Join(build.Default.GOROOT, "src")) +} + +func prepareGithubProject(owner, name string) func(*testing.B) { + return func(b *testing.B) { + dir := filepath.Join(build.Default.GOPATH, "src", "github.com", owner, name) + _, err := os.Stat(dir) + if os.IsNotExist(err) { + err = exec.Command("git", "clone", fmt.Sprintf("https://github.com/%s/%s.git", owner, name)).Run() + if err != nil { + b.Fatalf("can't git clone %s/%s: %s", owner, name, err) + } + } + chdir(b, dir) + } +} + +func getBenchLintersArgsNoMegacheck() []string { + return []string{ + "--enable=deadcode", + "--enable=gocyclo", + "--enable=golint", + "--enable=varcheck", + "--enable=structcheck", + "--enable=maligned", + "--enable=errcheck", + "--enable=dupl", + "--enable=ineffassign", + "--enable=interfacer", + "--enable=unconvert", + "--enable=goconst", + "--enable=gas", + } +} + +func getBenchLintersArgs() []string { + return append([]string{ + "--enable=megacheck", + }, getBenchLintersArgsNoMegacheck()...) +} + +func getGometalinterCommonArgs() []string { + return []string{ + "--deadline=30m", + "--skip=testdata", + "--skip=builtin", + "--vendor", + "--cyclo-over=30", + "--dupl-threshold=150", + "--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")), + "--disable-all", + "--enable=vet", + "--enable=vetshadow", + } +} + +func printCommand(cmd string, args ...string) { + if os.Getenv("PRINT_CMD") != "1" { + return + } + quotedArgs := []string{} + for _, a := range args { + quotedArgs = append(quotedArgs, strconv.Quote(a)) + } + + logrus.Warnf("%s %s", cmd, strings.Join(quotedArgs, " ")) +} + +func runGometalinter(b *testing.B) { + args := []string{} + args = append(args, getGometalinterCommonArgs()...) + args = append(args, getBenchLintersArgs()...) + args = append(args, "./...") + + printCommand("gometalinter", args...) + _ = exec.Command("gometalinter", args...).Run() +} + +func getGolangciLintCommonArgs() []string { + return []string{"run", "--no-config", "--issues-exit-code=0", "--deadline=30m", "--disable-all", "--enable=govet"} +} + +func runGolangciLintForBench(b *testing.B) { + args := getGolangciLintCommonArgs() + args = append(args, getBenchLintersArgs()...) + printCommand("golangci-lint", args...) + out, err := exec.Command("golangci-lint", args...).CombinedOutput() + if err != nil { + b.Fatalf("can't run golangci-lint: %s, %s", err, out) + } +} + +func getGoLinesTotalCount(b *testing.B) int { + cmd := exec.Command("bash", "-c", `find . -name "*.go" | fgrep -v vendor | xargs wc -l | tail -1`) + out, err := cmd.CombinedOutput() + if err != nil { + b.Fatalf("can't run go lines counter: %s", err) + } + + parts := bytes.Split(bytes.TrimSpace(out), []byte(" ")) + n, err := strconv.Atoi(string(parts[0])) + if err != nil { + b.Fatalf("can't parse go lines count: %s", err) + } + + return n +} + +func getLinterMemoryMB(b *testing.B, progName string) (int, error) { + processes, err := gops.Processes() + if err != nil { + b.Fatalf("Can't get processes: %s", err) + } + + var progPID int + for _, p := range processes { + if p.Executable() == progName { + progPID = p.Pid() + break + } + } + if progPID == 0 { + return 0, fmt.Errorf("no process") + } + + allProgPIDs := []int{progPID} + for _, p := range processes { + if p.PPid() == progPID { + allProgPIDs = append(allProgPIDs, p.Pid()) + } + } + + var totalProgMemBytes uint64 + for _, pid := range allProgPIDs { + p, err := process.NewProcess(int32(pid)) + if err != nil { + continue // subprocess could die + } + + mi, err := p.MemoryInfo() + if err != nil { + continue + } + + totalProgMemBytes += mi.RSS + } + + return int(totalProgMemBytes / 1024 / 1024), nil +} + +func trackPeakMemoryUsage(b *testing.B, doneCh <-chan struct{}, progName string) chan int { + resCh := make(chan int) + go func() { + var peakUsedMemMB int + t := time.NewTicker(time.Millisecond * 5) + defer t.Stop() + + for { + select { + case <-doneCh: + resCh <- peakUsedMemMB + close(resCh) + return + case <-t.C: + } + + m, err := getLinterMemoryMB(b, progName) + if err != nil { + continue + } + if m > peakUsedMemMB { + peakUsedMemMB = m + } + } + }() + return resCh +} + +type runResult struct { + peakMemMB int + duration time.Duration +} + +func compare(b *testing.B, gometalinterRun, golangciLintRun func(*testing.B), repoName, mode string, kLOC int) { // nolint + gometalinterRes := runOne(b, gometalinterRun, "gometalinter") + golangciLintRes := runOne(b, golangciLintRun, "golangci-lint") + + if mode != "" { + mode = " " + mode + } + logrus.Warnf("%s (%d kLoC): golangci-lint%s: time: %s, %.1f times faster; memory: %dMB, %.1f times less", + repoName, kLOC, mode, + golangciLintRes.duration, gometalinterRes.duration.Seconds()/golangciLintRes.duration.Seconds(), + golangciLintRes.peakMemMB, float64(gometalinterRes.peakMemMB)/float64(golangciLintRes.peakMemMB), + ) +} + +func runOne(b *testing.B, run func(*testing.B), progName string) *runResult { + doneCh := make(chan struct{}) + peakMemCh := trackPeakMemoryUsage(b, doneCh, progName) + startedAt := time.Now() + run(b) + duration := time.Since(startedAt) + close(doneCh) + + peakUsedMemMB := <-peakMemCh + return &runResult{ + peakMemMB: peakUsedMemMB, + duration: duration, + } +} + +func BenchmarkWithGometalinter(b *testing.B) { + installBinary(b) + + type bcase struct { + name string + prepare func(*testing.B) + } + bcases := []bcase{ + { + name: "self repo", + prepare: prepareGithubProject("golangci", "golangci-lint"), + }, + { + name: "gometalinter repo", + prepare: prepareGithubProject("alecthomas", "gometalinter"), + }, + { + name: "hugo", + prepare: prepareGithubProject("gohugoio", "hugo"), + }, + { + name: "go-ethereum", + prepare: prepareGithubProject("ethereum", "go-ethereum"), + }, + { + name: "beego", + prepare: prepareGithubProject("astaxie", "beego"), + }, + { + name: "terraform", + prepare: prepareGithubProject("hashicorp", "terraform"), + }, + { + name: "consul", + prepare: prepareGithubProject("hashicorp", "consul"), + }, + { + name: "go source code", + prepare: prepareGoSource, + }, + } + for _, bc := range bcases { + bc.prepare(b) + lc := getGoLinesTotalCount(b) + + compare(b, runGometalinter, runGolangciLintForBench, bc.name, "", lc/1000) + } +} diff --git a/test/linters_test.go b/test/linters_test.go new file mode 100644 index 00000000..9a9ce1ce --- /dev/null +++ b/test/linters_test.go @@ -0,0 +1,56 @@ +package test + +import ( + "bytes" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" +) + +func runGoErrchk(c *exec.Cmd, t *testing.T) { + output, err := c.CombinedOutput() + assert.NoError(t, err, "Output:\n%s", output) + + // Can't check exit code: tool only prints to output + assert.False(t, bytes.Contains(output, []byte("BUG")), "Output:\n%s", output) +} + +const testdataDir = "testdata" +const binName = "golangci-lint" + +func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { + t.Log(filepath.Join(testdataDir, "*.go")) + sources, err := filepath.Glob(filepath.Join(testdataDir, "*.go")) + assert.NoError(t, err) + assert.NotEmpty(t, sources) + + installBinary(t) + + for _, s := range sources { + s := s + t.Run(s, func(t *testing.T) { + t.Parallel() + testOneSource(t, s) + }) + } +} + +func testOneSource(t *testing.T, sourcePath string) { + goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk") + cmd := exec.Command(goErrchkBin, binName, "run", + "--enable-all", + "--dupl.threshold=20", + "--gocyclo.min-complexity=20", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number", + "--print-welcome=false", + "--govet.check-shadowing=true", + "--depguard.include-go-root", + "--depguard.packages='log'", + sourcePath) + runGoErrchk(cmd, t) +} diff --git a/test/run_test.go b/test/run_test.go new file mode 100644 index 00000000..e73a2708 --- /dev/null +++ b/test/run_test.go @@ -0,0 +1,56 @@ +package test + +import ( + "os/exec" + "path/filepath" + "sync" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +var installOnce sync.Once + +func installBinary(t assert.TestingT) { + installOnce.Do(func() { + cmd := exec.Command("go", "install", filepath.Join("..", "cmd", binName)) + assert.NoError(t, cmd.Run(), "Can't go install %s", binName) + }) +} + +func TestCongratsMessageIfNoIssues(t *testing.T) { + installBinary(t) + + out, exitCode := runGolangciLint(t, "../...") + assert.Equal(t, 0, exitCode) + assert.Equal(t, "Congrats! No issues were found.\n", out) +} + +func TestDeadline(t *testing.T) { + installBinary(t) + + out, exitCode := runGolangciLint(t, "--no-config", "--deadline=1ms", "../...") + assert.Equal(t, 4, exitCode) + assert.Equal(t, "", out) // no 'Congrats! No issues were found.' +} + +func runGolangciLint(t *testing.T, args ...string) (string, int) { + runArgs := append([]string{"run"}, args...) + cmd := exec.Command("golangci-lint", runArgs...) + out, err := cmd.Output() + if err != nil { + if exitError, ok := err.(*exec.ExitError); ok { + t.Logf("stderr: %s", exitError.Stderr) + ws := exitError.Sys().(syscall.WaitStatus) + return string(out), ws.ExitStatus() + } + + t.Fatalf("can't get error code from %s", err) + return "", -1 + } + + // success, exitCode should be 0 if go is ok + ws := cmd.ProcessState.Sys().(syscall.WaitStatus) + return string(out), ws.ExitStatus() +} diff --git a/pkg/testdata/with_issues/deadcode.go b/test/testdata/deadcode.go similarity index 100% rename from pkg/testdata/with_issues/deadcode.go rename to test/testdata/deadcode.go diff --git a/pkg/testdata/with_issues/depguard.go b/test/testdata/depguard.go similarity index 100% rename from pkg/testdata/with_issues/depguard.go rename to test/testdata/depguard.go diff --git a/pkg/testdata/with_issues/dupl.go b/test/testdata/dupl.go similarity index 82% rename from pkg/testdata/with_issues/dupl.go rename to test/testdata/dupl.go index 919b04ef..9fe413ee 100644 --- a/pkg/testdata/with_issues/dupl.go +++ b/test/testdata/dupl.go @@ -9,7 +9,7 @@ func (DuplLogger) level() int { func (DuplLogger) Debug(args ...interface{}) {} func (DuplLogger) Info(args ...interface{}) {} -func (logger *DuplLogger) First(args ...interface{}) { // ERROR "12-21 lines are duplicate of `testdata/with_issues/dupl.go:23-32`" +func (logger *DuplLogger) First(args ...interface{}) { // ERROR "12-21 lines are duplicate of `testdata/dupl.go:23-32`" if logger.level() >= 0 { logger.Debug(args...) logger.Debug(args...) @@ -20,7 +20,7 @@ func (logger *DuplLogger) First(args ...interface{}) { // ERROR "12-21 lines are } } -func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "23-32 lines are duplicate of `testdata/with_issues/dupl.go:12-21`" +func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "23-32 lines are duplicate of `testdata/dupl.go:12-21`" if logger.level() >= 1 { logger.Info(args...) logger.Info(args...) diff --git a/pkg/testdata/with_issues/errcheck.go b/test/testdata/errcheck.go similarity index 100% rename from pkg/testdata/with_issues/errcheck.go rename to test/testdata/errcheck.go diff --git a/pkg/testdata/with_issues/gas.go b/test/testdata/gas.go similarity index 100% rename from pkg/testdata/with_issues/gas.go rename to test/testdata/gas.go diff --git a/pkg/testdata/with_issues/goconst.go b/test/testdata/goconst.go similarity index 100% rename from pkg/testdata/with_issues/goconst.go rename to test/testdata/goconst.go diff --git a/pkg/testdata/with_issues/gocyclo.go b/test/testdata/gocyclo.go similarity index 100% rename from pkg/testdata/with_issues/gocyclo.go rename to test/testdata/gocyclo.go diff --git a/pkg/testdata/with_issues/gofmt.go b/test/testdata/gofmt.go similarity index 100% rename from pkg/testdata/with_issues/gofmt.go rename to test/testdata/gofmt.go diff --git a/pkg/testdata/with_issues/goimports.go b/test/testdata/goimports.go similarity index 100% rename from pkg/testdata/with_issues/goimports.go rename to test/testdata/goimports.go diff --git a/pkg/testdata/with_issues/golint.go b/test/testdata/golint.go similarity index 100% rename from pkg/testdata/with_issues/golint.go rename to test/testdata/golint.go diff --git a/pkg/testdata/with_issues/govet.go b/test/testdata/govet.go similarity index 89% rename from pkg/testdata/with_issues/govet.go rename to test/testdata/govet.go index b30155d9..1474f9d2 100644 --- a/pkg/testdata/with_issues/govet.go +++ b/test/testdata/govet.go @@ -11,7 +11,7 @@ func Govet() error { func GovetShadow(f io.Reader, buf []byte) (err error) { if f != nil { - _, err := f.Read(buf) // ERROR "declaration of .err. shadows declaration at testdata/with_issues/govet.go:\d+" + _, err := f.Read(buf) // ERROR "declaration of .err. shadows declaration at testdata/govet.go:\d+" if err != nil { return err } diff --git a/pkg/testdata/with_issues/ineffassign.go b/test/testdata/ineffassign.go similarity index 100% rename from pkg/testdata/with_issues/ineffassign.go rename to test/testdata/ineffassign.go diff --git a/pkg/testdata/with_issues/interfacer.go b/test/testdata/interfacer.go similarity index 100% rename from pkg/testdata/with_issues/interfacer.go rename to test/testdata/interfacer.go diff --git a/pkg/testdata/with_issues/maligned.go b/test/testdata/maligned.go similarity index 100% rename from pkg/testdata/with_issues/maligned.go rename to test/testdata/maligned.go diff --git a/pkg/testdata/with_issues/megacheck.go b/test/testdata/megacheck.go similarity index 100% rename from pkg/testdata/with_issues/megacheck.go rename to test/testdata/megacheck.go diff --git a/pkg/testdata/with_issues/structcheck.go b/test/testdata/structcheck.go similarity index 100% rename from pkg/testdata/with_issues/structcheck.go rename to test/testdata/structcheck.go diff --git a/pkg/testdata/with_issues/typecheck.go b/test/testdata/typecheck.go similarity index 100% rename from pkg/testdata/with_issues/typecheck.go rename to test/testdata/typecheck.go diff --git a/pkg/testdata/with_issues/typecheck_many_issues.go b/test/testdata/typecheck_many_issues.go similarity index 100% rename from pkg/testdata/with_issues/typecheck_many_issues.go rename to test/testdata/typecheck_many_issues.go diff --git a/pkg/testdata/with_issues/unconvert.go b/test/testdata/unconvert.go similarity index 100% rename from pkg/testdata/with_issues/unconvert.go rename to test/testdata/unconvert.go diff --git a/pkg/testdata/with_issues/varcheck.go b/test/testdata/varcheck.go similarity index 100% rename from pkg/testdata/with_issues/varcheck.go rename to test/testdata/varcheck.go