diff --git a/README.md b/README.md index 1dcfcb85..1e79dfd1 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,29 @@ Sponsored by [GolangCI.com](https://golangci.com): SaaS service for running lint -// TOC Here at the end +* [GolangCI-Lint](#golangci-lint) +* [Install](#install) +* [Quick Start](#quick-start) +* [Comparison](#comparison) + * [gometalinter](#gometalinter) + * [Run Needed Linters Manually](#run-needed-linters-manually) +* [Performance](#performance) + * [Default Mode](#default-mode) + * [Fast Mode](#fast-mode) +* [Supported Linters](#supported-linters) + * [Enabled By Default Linters](#enabled-by-default-linters) + * [Disabled By Default Linters (-E/--enable)](#disabled-by-default-linters--e--enable) +* [Configuration](#configuration) + * [Command-Line Options](#command-line-options) + * [Run Options](#run-options) + * [Linters](#linters) + * [Linters Options](#linters-options) + * [Issues Options](#issues-options) + * [Output Options](#output-options) + * [Configuration File](#configuration-file) +* [False Positives](#false-positives) +* [FAQ](#faq) +* [Internals](#internals) # Install ```bash @@ -73,7 +95,7 @@ $ golangci-lint run --disable-all -E errcheck # Comparison ## `gometalinter` GolangCI-Lint was created to fix next issues with `gometalinter`: -1. Slow work: `gometalinter` usually works for minutes in average projects. GolangCI-Lint works [2-10x times faster](#benchmarks) by [reusing work](#internals). +1. Slow work: `gometalinter` usually works for minutes in average projects. GolangCI-Lint works [2-6x times faster](#benchmarks) by [reusing work](#internals). 2. Huge memory consumption: parallel linters don't share the same program representation and can eat `n` times more memory (`n` - concurrency). GolangCI-Lint fixes it by sharing representation. 3. Can't set honest concurrency: if you set it to `n` it can take `n+x` threads because of forced threads in specific linters. `gometalinter` can't do anything about it, because it runs linters as black-boxes in forked processes. In GolangCI-Lint we run all linters in one process and fully control them. Configured concurrency will be honest. This issue is important because often you'd like to set concurrency to CPUs count minus one to save one CPU for example for IDE. It concurrency isn't correct you will have troubles using IDE while analyzing code. @@ -89,14 +111,53 @@ 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 version of `n` linters. # Performance -## Benchmarks +## Default Mode +We compare golangci-lint and gometalinter in default mode, but explicitly specify all linters to enable because of small differences in default configuration. +```bash +$ golangci-lint run --no-config --issues-exit-code=0 --deadline=30m \ + --disable-all --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 --enable=megacheck +$ gometalinter --deadline=30m --vendor --cyclo-over=30 --dupl-threshold=150 \ + --exclude= --skip=testdata --skip=builtin \ + --disable-all --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 --enable=megacheck + ./... ``` -BenchmarkWithGometalinter/self_repo/gometalinter_--fast_(4098_lines_of_code)-4 30 1482617961 ns/op -BenchmarkWithGometalinter/self_repo/golangci-lint_fast_(4098_lines_of_code)-4 100 414381899 ns/op -BenchmarkWithGometalinter/self_repo/gometalinter_(4098_lines_of_code)-4 1 39304954722 ns/op -BenchmarkWithGometalinter/self_repo/golangci-lint_(4098_lines_of_code)-4 5 8290405036 ns/op + +| Repository | GolangCI Lint Time | GolangCI Is Faster In | +| ---------- | ------------------ | --------------------- | +| self repo, 4.4 kLoC | 9.1s | 6.6x | +| gometalinter repo, 3.8 kLoC | 5.1s | 4.9x | +| hugo, 69 kLoC | 12.4s | 5.8x | +| go source, 1300 kLoC | 3m15s | 1.8x | + +On average golangci-lint is 4.8 times faster than gometalinter. Maximum difference is in +self repo: 6.6 times faster, minimum difference is in go source code repo: 1.8 faster. + +## Fast Mode +We compare golangci-lint and gometalinter in fast mode (`--fast`), but don't use option `--fast` because it differs a little. +Instead we configure common linters from this option. +```bash +$ golangci-lint run --no-config --issues-exit-code=0 --deadline=30m \ + --disable-all --enable=govet --enable=dupl --enable=goconst --enable=gocyclo --enable=golint --enable=ineffassign +$ gometalinter --deadline=30m --vendor --cyclo-over=30 --dupl-threshold=150 \ + --exclude= --skip=testdata --skip=builtin \ + --disable-all --enable=vet --enable=vetshadow -enable=dupl --enable=goconst --enable=gocyclo --enable=golint --enable=ineffassign \ + ./... ``` -## Internals + +| Repository | GolangCI Lint Time | GolangCI Is Faster In | +| ---------- | ------------------ | --------------------- | +| self repo, 4.4 kLoC | 0.4s | 3.1x | +| gometalinter repo, 3.8 kLoC | 0.2s | 1.9x | +| hugo, 69 kLoC | 1.6s | 4x | +| go source, 1300 kLoC | 35.4s | 1.17x | + +On average golangci-lint is 2.5 times faster than gometalinter. Maximum difference is in +self repo: 3.1 times faster, minimum difference is in go source code repo: 17% faster. + # Supported Linters To see a list of supported linters and which linters are enabled/disabled by default execute command @@ -261,3 +322,21 @@ A: You have 2 choices: 1. Update it: `go get -u gopkg.in/golangci/golangci-lint.v1/cmd/golangci-lint` 2. Run it with `-v` option and check output. 3. If it doesn't help create [GitHub issue](https://github.com/golangci/golangci-lint/issues/new). + +# Internals +Key difference with gometalinter is that golangci-lint shares work between specific linters (golint, govet, ...). +For small and medium projects 50-80% of work between linters can be reused. +Now we share `loader.Program` and `SSA` representation building. `SSA` representation is used from +a [fork of go-tools](https://github.com/dominikh/go-tools), not the official one. Also we are going to +reuse `AST` parsing and traversal. + +We don't fork to call specific linter but use it's API. We forked github repos of almost all linters +to make API. It also allows us to be more performant and control actual count of used threads. + +All linters are vendored in `/vendor` folder: their version is fixed, they are builtin +and you don't need to install them separately. + +We use chains for issues and independent processors to post-process them: exclude issues by limits, +nolint comment, diff, regexps; prettify paths etc. + +We use `cobra` for command-line action. diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 4090bbb9..0a7b900c 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -41,11 +41,23 @@ func (e *Executor) initRoot() { if e.cfg.Run.CPUProfilePath != "" { pprof.StopCPUProfile() } + if e.cfg.Run.MemProfilePath != "" { + f, err := os.Create(e.cfg.Run.MemProfilePath) + if err != nil { + log.Fatal(err) + } + runtime.GC() // get up-to-date statistics + if err := pprof.WriteHeapProfile(f); err != nil { + log.Fatal("could not write memory profile: ", err) + } + } + os.Exit(e.exitCode) }, } rootCmd.PersistentFlags().BoolVarP(&e.cfg.Run.IsVerbose, "verbose", "v", false, "verbose output") rootCmd.PersistentFlags().StringVar(&e.cfg.Run.CPUProfilePath, "cpu-profile-path", "", "Path to CPU profile output file") + rootCmd.PersistentFlags().StringVar(&e.cfg.Run.MemProfilePath, "mem-profile-path", "", "Path to memory profile output file") rootCmd.PersistentFlags().IntVarP(&e.cfg.Run.Concurrency, "concurrency", "j", runtime.NumCPU(), "Concurrency") e.rootCmd = rootCmd diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 6cc8a1a0..46971b2f 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -56,6 +56,7 @@ func (e *Executor) initRun() { runCmd.Flags().BoolVar(&rc.AnalyzeTests, "tests", false, "Analyze tests (*_test.go)") runCmd.Flags().BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, "Print avg and max memory usage of golangci-lint and total time") runCmd.Flags().StringVarP(&rc.Config, "config", "c", "", "Read config from file path `PATH`") + runCmd.Flags().BoolVar(&rc.NoConfig, "no-config", false, "Don't read config") // Linters settings config lsc := &e.cfg.LintersSettings @@ -315,7 +316,15 @@ func (e *Executor) parseConfig(cmd *cobra.Command) { viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) viper.AutomaticEnv() - configFile := viper.GetString("config") + configFile := e.cfg.Run.Config + if e.cfg.Run.NoConfig && configFile != "" { + log.Fatal("can't combine option --config and --no-config") + } + + if e.cfg.Run.NoConfig { + return + } + if configFile == "" { viper.SetConfigName(".golangci") viper.AddConfigPath("./") @@ -323,6 +332,10 @@ func (e *Executor) parseConfig(cmd *cobra.Command) { viper.SetConfigFile(configFile) } + e.parseConfigImpl() +} + +func (e *Executor) parseConfigImpl() { commandLineConfig := *e.cfg // make copy if err := viper.ReadInConfig(); err != nil { @@ -351,6 +364,10 @@ func (e *Executor) validateConfig(commandLineConfig *config.Config) error { return errors.New("option run.cpuprofilepath in config isn't allowed") } + if commandLineConfig.Run.MemProfilePath == "" && c.Run.MemProfilePath != "" { + return errors.New("option run.memprofilepath in config isn't allowed") + } + if !commandLineConfig.Run.IsVerbose && c.Run.IsVerbose { return errors.New("can't set run.verbose option with config: only on command-line") } diff --git a/pkg/config/config.go b/pkg/config/config.go index ab7057cf..7ac9034c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -41,10 +41,12 @@ var DefaultExcludePatterns = []string{ type Run struct { IsVerbose bool `mapstructure:"verbose"` CPUProfilePath string + MemProfilePath string Concurrency int PrintResourcesUsage bool `mapstructure:"print-resources-usage"` - Config string + Config string + NoConfig bool Args []string diff --git a/pkg/enabled_linters_test.go b/pkg/enabled_linters_test.go index a333976d..c711b6f2 100644 --- a/pkg/enabled_linters_test.go +++ b/pkg/enabled_linters_test.go @@ -15,6 +15,7 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/shirou/gopsutil/mem" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -136,55 +137,44 @@ func getBenchFastLintersArgs() []string { } } -func runGometalinter(b *testing.B) { - args := []string{"--disable-all", "--deadline=30m"} - args = append(args, getBenchLintersArgs()...) - args = append(args, - "--enable=vet", - "--enable=vetshadow", +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 runGometalinter(b *testing.B) { + args := []string{} + args = append(args, getGometalinterCommonArgs()...) + args = append(args, getBenchLintersArgs()...) + args = append(args, "./...") _ = exec.Command("gometalinter", args...).Run() } func runGometalinterFast(b *testing.B) { - args := []string{"--disable-all", "--deadline=30m"} + args := []string{} + args = append(args, getGometalinterCommonArgs()...) args = append(args, getBenchFastLintersArgs()...) - args = append(args, - "--enable=vet", - "--enable=vetshadow", - "--vendor", - "--cyclo-over=30", - "--dupl-threshold=150", - "--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")), - "./...", - ) + args = append(args, "./...") _ = exec.Command("gometalinter", args...).Run() } -func runGometalinterNoMegacheck(b *testing.B) { - args := []string{"--disable-all", "--deadline=30m"} - args = append(args, getBenchLintersArgsNoMegacheck()...) - args = append(args, - "--enable=vet", - "--enable=vetshadow", - "--vendor", - "--cyclo-over=30", - "--dupl-threshold=150", - "--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")), - "./...", - ) - _ = 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 := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"} + args := getGolangciLintCommonArgs() args = append(args, getBenchLintersArgs()...) - b.Logf("golangci-lint %s", strings.Join(args, " ")) out, err := exec.Command("golangci-lint", args...).CombinedOutput() if err != nil { b.Fatalf("can't run golangci-lint: %s, %s", err, out) @@ -192,7 +182,7 @@ func runGolangciLint(b *testing.B) { } func runGolangciLintFast(b *testing.B) { - args := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"} + args := getGolangciLintCommonArgs() args = append(args, getBenchFastLintersArgs()...) out, err := exec.Command("golangci-lint", args...).CombinedOutput() if err != nil { @@ -200,16 +190,6 @@ func runGolangciLintFast(b *testing.B) { } } -func runGolangciLintNoMegacheck(b *testing.B) { - args := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"} - args = append(args, getBenchLintersArgsNoMegacheck()...) - b.Logf("golangci-lint %s", strings.Join(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() @@ -276,7 +256,7 @@ func runBench(b *testing.B, run func(*testing.B), format string, args ...interfa if peakUsedMemMB > startUsedMemMB { linterPeakMemUsage = peakUsedMemMB - startUsedMemMB } - b.Logf("%s: start used mem is %dMB, peak used mem is %dMB, linter peak mem usage is %dMB", + logrus.Warnf("%s: start used mem is %dMB, peak used mem is %dMB, linter peak mem usage is %dMB", name, startUsedMemMB, peakUsedMemMB, linterPeakMemUsage) } @@ -314,8 +294,5 @@ func BenchmarkWithGometalinter(b *testing.B) { runBench(b, runGometalinter, "%s/gometalinter (%d lines of code)", bc.name, lc) runBench(b, runGolangciLint, "%s/golangci-lint (%d lines of code)", bc.name, lc) - - runBench(b, runGometalinterNoMegacheck, "%s/gometalinter wo megacheck (%d lines of code)", bc.name, lc) - runBench(b, runGolangciLintNoMegacheck, "%s/golangci-lint wo megacheck (%d lines of code)", bc.name, lc) } } diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 55b80c1b..6b8c758e 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -7,6 +7,7 @@ import ( "github.com/golang/lint" "github.com/golangci/golangci-lint/pkg/result" + "github.com/sirupsen/logrus" ) type Golint struct{} @@ -21,14 +22,18 @@ func (Golint) Desc() string { func (g Golint) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) { var issues []result.Issue + var lintErr error for _, pkgFiles := range lintCtx.Paths.FilesGrouppedByDirs() { i, err := g.lintFiles(lintCtx.Settings().Golint.MinConfidence, pkgFiles...) if err != nil { - // TODO: skip and warn - return nil, fmt.Errorf("can't lint files %s: %s", lintCtx.Paths.Files, err) + lintErr = err + continue } issues = append(issues, i...) } + if lintErr != nil { + logrus.Warnf("golint: %s", lintErr) + } return issues, nil } diff --git a/pkg/runner.go b/pkg/runner.go index 59cd4871..8a184a4e 100644 --- a/pkg/runner.go +++ b/pkg/runner.go @@ -203,8 +203,6 @@ func setOutputToDevNull() (savedStdout, savedStderr *os.File) { } func (r SimpleRunner) Run(ctx context.Context, linters []Linter, lintCtx *golinters.Context) <-chan result.Issue { - defer timeutils.NewStopwatch("runner").Print() - lintResultsCh := r.runWorkers(ctx, lintCtx, linters) processedLintResultsCh := r.processLintResults(ctx, lintResultsCh) if ctx.Err() != nil { diff --git a/pkg/timeutils/stopwatch.go b/pkg/timeutils/stopwatch.go index 7527c9c9..e17deaf6 100644 --- a/pkg/timeutils/stopwatch.go +++ b/pkg/timeutils/stopwatch.go @@ -61,7 +61,11 @@ func (s *Stopwatch) Print() { } func (s *Stopwatch) PrintStages() { - logrus.Infof("%s %s", s.name, s.sprintStages()) + var stagesDuration time.Duration + for _, s := range s.stages { + stagesDuration += s + } + logrus.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintStages()) } func (s *Stopwatch) TrackStage(name string, f func()) {