Skip to content

Commit

Permalink
Fix #237, fix #178: use go/packages
Browse files Browse the repository at this point in the history
Use go/packages instead of x/tools/loader: it allows to work
with go modules and speedups loading of packages with the help
of build cache.

A lot of linters became "fast": they are enabled by --fast now and
work in 1-2 seconds. Only unparam, interfacer and megacheck
are "slow" linters now.

Average project is analyzed 20-40% faster than before if all linters are
enabled! If we enable all linters except unparam, interfacer and
megacheck analysis is 10-20x faster!
  • Loading branch information
jirfag committed Oct 25, 2018
1 parent 6ecea1c commit 4858a4a
Show file tree
Hide file tree
Showing 87 changed files with 4,228 additions and 1,687 deletions.
14 changes: 0 additions & 14 deletions .golangci.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,6 @@ linters-settings:
govet:
# report about shadowed variables
check-shadowing: true

# Obtain type information from installed (to $GOPATH/pkg) package files:
# golangci-lint will execute `go install -i` and `go test -i` for analyzed packages
# before analyzing them.
# By default this option is disabled and govet gets type information by loader from source code.
# Loading from source code is slow, but it's done only once for all linters.
# Go-installing of packages first time is much slower than loading them from source code,
# therefore this option is disabled by default.
# But repeated installation is fast in go >= 1.10 because of build caching.
# Enable this option only if all conditions are met:
# 1. you use only "fast" linters (--fast e.g.): no program loading occurs
# 2. you use go >= 1.10
# 3. you do repeated runs (false for CI) or cache $GOPATH/pkg or `go env GOCACHE` dir in CI.
use-installed-packages: false
golint:
# minimal confidence for issues, default is 0.8
min-confidence: 0.8
Expand Down
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ linters-settings:
- github.com/sirupsen/logrus
misspell:
locale: US
lll:
line-length: 140

linters:
enable-all: true
Expand Down
11 changes: 7 additions & 4 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 19 additions & 28 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,16 @@ GolangCI-Lint can be used with zero configuration. By default the following lint
```
$ golangci-lint help linters
Enabled by default linters:
govet: Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false]
govet: Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true]
staticcheck: Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false]
unused: Checks Go code for unused constants, variables, functions and types [fast: false]
gosimple: Linter for Go source code that specializes in simplifying a code [fast: false]
structcheck: Finds an unused struct fields [fast: false]
varcheck: Finds unused global variables and constants [fast: false]
structcheck: Finds an unused struct fields [fast: true]
varcheck: Finds unused global variables and constants [fast: true]
ineffassign: Detects when assignments to existing variables are not used [fast: true]
deadcode: Finds unused code [fast: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: false]
deadcode: Finds unused code [fast: true]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true]
```

and the following linters are disabled by default:
Expand All @@ -128,17 +128,17 @@ $ golangci-lint help linters
...
Disabled by default linters:
golint: Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: true]
gosec (gas): Inspects source code for security problems [fast: false]
gosec (gas): Inspects source code for security problems [fast: true]
interfacer: Linter that suggests narrower interface types [fast: false]
unconvert: Remove unnecessary type conversions [fast: false]
unconvert: Remove unnecessary type conversions [fast: true]
dupl: Tool for code clone detection [fast: true]
goconst: Finds repeated strings that could be replaced by a constant [fast: true]
gocyclo: Computes and checks the cyclomatic complexity of functions [fast: true]
gofmt: Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true]
goimports: Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true]
maligned: Tool to detect Go structs that would take less memory if their fields were sorted [fast: false]
maligned: Tool to detect Go structs that would take less memory if their fields were sorted [fast: true]
megacheck: 3 sub-linters in one: unused, gosimple and staticcheck [fast: false]
depguard: Go linter that checks if package imports are in a list of acceptable packages [fast: false]
depguard: Go linter that checks if package imports are in a list of acceptable packages [fast: true]
misspell: Finds commonly misspelled English words in comments [fast: true]
lll: Reports long lines [fast: true]
unparam: Reports unused function parameters [fast: false]
Expand Down Expand Up @@ -369,7 +369,7 @@ Flags:
--enable-all Enable all linters
--disable-all Disable all linters
-p, --presets strings Enable presets (bugs|unused|format|style|complexity|performance) of linters. Run 'golangci-lint linters' to see them. This option implies option --disable-all
--fast Run only fast linters from enabled linters set
--fast Run only fast linters from enabled linters set (first run won't be fast)
-e, --exclude strings Exclude issue by regexp
--exclude-use-default Use or not use default excludes:
# errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
Expand Down Expand Up @@ -499,20 +499,6 @@ linters-settings:
govet:
# report about shadowed variables
check-shadowing: true

# Obtain type information from installed (to $GOPATH/pkg) package files:
# golangci-lint will execute `go install -i` and `go test -i` for analyzed packages
# before analyzing them.
# By default this option is disabled and govet gets type information by loader from source code.
# Loading from source code is slow, but it's done only once for all linters.
# Go-installing of packages first time is much slower than loading them from source code,
# therefore this option is disabled by default.
# But repeated installation is fast in go >= 1.10 because of build caching.
# Enable this option only if all conditions are met:
# 1. you use only "fast" linters (--fast e.g.): no program loading occurs
# 2. you use go >= 1.10
# 3. you do repeated runs (false for CI) or cache $GOPATH/pkg or `go env GOCACHE` dir in CI.
use-installed-packages: false
golint:
# minimal confidence for issues, default is 0.8
min-confidence: 0.8
Expand Down Expand Up @@ -654,6 +640,8 @@ linters-settings:
- github.com/sirupsen/logrus
misspell:
locale: US
lll:
line-length: 140

linters:
enable-all: true
Expand Down Expand Up @@ -698,17 +686,20 @@ We don't recommend vendoring `golangci-lint` in your repo: you will get troubles

**Does I need to run `go install`?**

No, you don't need to do it anymore. We will run `go install -i` and `go test -i`
for analyzed packages ourselves. We will run them only
if option `govet.use-installed-packages` is `true`.
No, you don't need to do it anymore.

**`golangci-lint` doesn't work**

1. Update it: `go get -u github.com/golangci/golangci-lint/cmd/golangci-lint`
2. Run it with `-v` option and check the output.
3. If it doesn't help create a [GitHub issue](https://github.com/golangci/golangci-lint/issues/new) with the output from the error and #2 above.

**Why running with `--fast` is slow on the first run?**
Because the first run caches type information. All subsequent runs will be fast.
Usually this options is used during development on local machine and compilation was already performed.

# Thanks

Thanks to [alecthomas/gometalinter](https://github.com/alecthomas/gometalinter) for inspiration and amazing work.
Thanks to [bradleyfalzon/revgrep](https://github.com/bradleyfalzon/revgrep) for cool diff tool.

Expand Down
9 changes: 6 additions & 3 deletions README.tmpl.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,17 +358,20 @@ We don't recommend vendoring `golangci-lint` in your repo: you will get troubles

**Does I need to run `go install`?**

No, you don't need to do it anymore. We will run `go install -i` and `go test -i`
for analyzed packages ourselves. We will run them only
if option `govet.use-installed-packages` is `true`.
No, you don't need to do it anymore.

**`golangci-lint` doesn't work**

1. Update it: `go get -u github.com/golangci/golangci-lint/cmd/golangci-lint`
2. Run it with `-v` option and check the output.
3. If it doesn't help create a [GitHub issue](https://github.com/golangci/golangci-lint/issues/new) with the output from the error and #2 above.

**Why running with `--fast` is slow on the first run?**
Because the first run caches type information. All subsequent runs will be fast.
Usually this options is used during development on local machine and compilation was already performed.

# Thanks

Thanks to [alecthomas/gometalinter](https://github.com/alecthomas/gometalinter) for inspiration and amazing work.
Thanks to [bradleyfalzon/revgrep](https://github.com/bradleyfalzon/revgrep) for cool diff tool.

Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ require (
github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770
github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21
github.com/golangci/revgrep v0.0.0-20180526074752-d9c87f5ffaf0
github.com/golangci/tools v0.0.0-20180902102414-98e75f53b4b9
github.com/golangci/tools v0.0.0-20180902102414-01dd7756e01d
github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4
github.com/golangci/unparam v0.0.0-20180902112548-7ad9dbcccc16
github.com/hashicorp/hcl v0.0.0-20180404174102-ef8a98b0bbce // indirect
Expand Down Expand Up @@ -56,7 +56,7 @@ require (
github.com/spf13/viper v1.0.2
github.com/stretchr/testify v1.2.1
golang.org/x/crypto v0.0.0-20180505025534-4ec37c66abab // indirect
golang.org/x/tools v0.0.0-20180831211245-5d4988d199e2
golang.org/x/tools v0.0.0-20180831211245-3e7aa9e59977
gopkg.in/airbrake/gobrake.v2 v2.0.9 // indirect
gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2 // indirect
sourcegraph.com/sourcegraph/go-diff v0.0.0-20171119081133-3f415a150aec
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21 h1:leSNB7iYzLYSS
github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21/go.mod h1:tf5+bzsHdTM0bsB7+8mt0GUMvjCgwLpTapNZHU8AajI=
github.com/golangci/revgrep v0.0.0-20180526074752-d9c87f5ffaf0 h1:HVfrLniijszjS1aiNg8JbBMO2+E1WIQ+j/gL4SQqGPg=
github.com/golangci/revgrep v0.0.0-20180526074752-d9c87f5ffaf0/go.mod h1:qOQCunEYvmd/TLamH+7LlVccLvUH5kZNhbCgTHoBbp4=
github.com/golangci/tools v0.0.0-20180902102414-01dd7756e01d/go.mod h1:zgj6NOYXOC1cexsdtDceI4/mj3aXK4JOVg9AV3C5LWI=
github.com/golangci/tools v0.0.0-20180902102414-98e75f53b4b9 h1:JGHGJqnbD9OMyjgQqyja7DZd0/to1LKFpN31Fq8byxc=
github.com/golangci/tools v0.0.0-20180902102414-98e75f53b4b9/go.mod h1:zgj6NOYXOC1cexsdtDceI4/mj3aXK4JOVg9AV3C5LWI=
github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 h1:zwtduBRr5SSWhqsYNgcuWO2kFlpdOZbP0+yRjmvPGys=
Expand Down Expand Up @@ -131,6 +132,7 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180826000951-f6ba57429505/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180831211245-3e7aa9e59977/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180831211245-5d4988d199e2/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180831211245-7ca132754999 h1:mf2VYfMpSMTlp0I/UXrX13w5LejDx34QeUUHH4TrUA8=
golang.org/x/tools v0.0.0-20180831211245-7ca132754999/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
Expand Down
6 changes: 6 additions & 0 deletions pkg/commands/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package commands

import (
"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/goutil"
"github.com/golangci/golangci-lint/pkg/lint"
"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/report"
Expand All @@ -21,6 +23,8 @@ type Executor struct {
reportData report.Data
DBManager *lintersdb.Manager
EnabledLintersSet *lintersdb.EnabledSet
contextLoader *lint.ContextLoader
goenv *goutil.Env
}

func NewExecutor(version, commit, date string) *Executor {
Expand Down Expand Up @@ -65,6 +69,8 @@ func NewExecutor(version, commit, date string) *Executor {

e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager,
lintersdb.NewValidator(e.DBManager), e.log.Child("lintersdb"), e.cfg)
e.goenv = goutil.NewEnv(e.log.Child("goenv"))
e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child("loader"), e.goenv)

return e
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func printLinterConfigs(lcs []linter.Config) {
altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", "))
}
fmt.Fprintf(logutils.StdOut, "%s%s: %s [fast: %t]\n", color.YellowString(lc.Name()),
altNamesStr, lc.Linter.Desc(), !lc.DoesFullImport)
altNamesStr, lc.Linter.Desc(), !lc.NeedsSSARepr)
}
}

Expand Down
25 changes: 21 additions & 4 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager) {
fs.StringSliceVarP(&lc.Presets, "presets", "p", nil,
wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint linters' to see "+
"them. This option implies option --disable-all", strings.Join(m.AllPresets(), "|"))))
fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set"))
fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set (first run won't be fast)"))

// Issues config
ic := &cfg.Issues
Expand Down Expand Up @@ -257,12 +257,13 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
e.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault)
}

lintCtx, err := lint.LoadContext(enabledLinters, e.cfg, e.log.Child("load"))
lintCtx, err := e.contextLoader.Load(ctx, enabledLinters)
if err != nil {
return nil, errors.Wrap(err, "context loading failed")
}
lintCtx.Log = e.log.Child("linters context")

runner, err := lint.NewRunner(lintCtx.ASTCache, e.cfg, e.log.Child("runner"))
runner, err := lint.NewRunner(lintCtx.ASTCache, e.cfg, e.log.Child("runner"), e.goenv)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -303,6 +304,10 @@ func (e *Executor) setExitCodeIfIssuesFound(issues <-chan result.Issue) <-chan r
}

func (e *Executor) runAndPrint(ctx context.Context, args []string) error {
if err := e.goenv.Discover(ctx); err != nil {
e.log.Warnf("Failed to discover go env: %s", err)
}

if !logutils.HaveDebugTag("linters_output") {
// Don't allow linters and loader to print anything
log.SetOutput(ioutil.Discard)
Expand Down Expand Up @@ -379,8 +384,20 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) {
}
}

if e.exitCode == exitcodes.Success && ctx.Err() != nil {
e.setupExitCode(ctx)
}

func (e *Executor) setupExitCode(ctx context.Context) {
if ctx.Err() != nil {
e.exitCode = exitcodes.Timeout
e.log.Errorf("Deadline exceeded: try increase it by passing --deadline option")
}

if e.exitCode == exitcodes.Success &&
os.Getenv("GL_TEST_RUN") == "1" &&
len(e.reportData.Warnings) != 0 {

e.exitCode = exitcodes.WarningInTest
}
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ type Run struct {

type LintersSettings struct {
Govet struct {
CheckShadowing bool `mapstructure:"check-shadowing"`
UseInstalledPackages bool `mapstructure:"use-installed-packages"`
CheckShadowing bool `mapstructure:"check-shadowing"`
}
Golint struct {
MinConfidence float64 `mapstructure:"min-confidence"`
Expand Down
10 changes: 8 additions & 2 deletions pkg/golinters/dupl.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import (
"go/token"

duplAPI "github.com/golangci/dupl"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/result"
"github.com/pkg/errors"
)

type Dupl struct{}
Expand All @@ -21,7 +23,7 @@ func (Dupl) Desc() string {
}

func (d Dupl) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) {
issues, err := duplAPI.Run(lintCtx.PkgProgram.Files(lintCtx.Cfg.Run.AnalyzeTests), lintCtx.Settings().Dupl.Threshold)
issues, err := duplAPI.Run(getAllFileNames(lintCtx), lintCtx.Settings().Dupl.Threshold)
if err != nil {
return nil, err
}
Expand All @@ -32,7 +34,11 @@ func (d Dupl) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue,

res := make([]result.Issue, 0, len(issues))
for _, i := range issues {
dupl := fmt.Sprintf("%s:%d-%d", i.To.Filename(), i.To.LineStart(), i.To.LineEnd())
toFilename, err := fsutils.ShortestRelPath(i.To.Filename(), "")
if err != nil {
return nil, errors.Wrapf(err, "failed to get shortest rel path for %q", i.To.Filename())
}
dupl := fmt.Sprintf("%s:%d-%d", toFilename, i.To.LineStart(), i.To.LineEnd())
text := fmt.Sprintf("%d-%d lines are duplicate of %s",
i.From.LineStart(), i.From.LineEnd(),
formatCode(dupl, lintCtx.Cfg))
Expand Down
4 changes: 2 additions & 2 deletions pkg/golinters/goconst.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ func (lint Goconst) Run(ctx context.Context, lintCtx *linter.Context) ([]result.
MinStringLength: lintCtx.Settings().Goconst.MinStringLen,
MinOccurrences: lintCtx.Settings().Goconst.MinOccurrencesCount,
}
for _, pkg := range lintCtx.PkgProgram.Packages() {
files, fset, err := getASTFilesForPkg(lintCtx, &pkg)
for _, pkg := range lintCtx.Packages {
files, fset, err := getASTFilesForGoPkg(lintCtx, pkg)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 4858a4a

Please sign in to comment.