Skip to content

Commit

Permalink
Fix #17, #87: govet becomes SLOW linter by default
Browse files Browse the repository at this point in the history
1. Allow govet to work in 2 modes: fast and slow. Default is slow.
In fast mode golangci-lint runs `go install -i` and `go test -i`
for analyzed packages. But it's fast only when:
  - go >= 1.10
  - it's repeated run or $GOPATH/pkg or `go env GOCACHE` is cached
  between CI builds
In slow mode we load program from source code like for another linters
and do it only once for all linters.

3. Patch govet code to warn about any troubles with the type
information. Default behaviour of govet was to hide such warnings.
Fail analysis if there are any troubles with type loading: it will
prevent false-positives and false-negatives from govet.

4. Describe almost all options in .golangci.example.yml and
include it into README. Describe when to use slow or fast mode of govet.

5. Speed up govet: reuse AST parsing: it's already parsed once by
golangci-lint.
For "slow" runs (when we run at least one slow linter) speedup by
not loading type information second time.

6. Improve logging, debug logging

7. Fix crash in logging of AST cache warnings (#118)
  • Loading branch information
jirfag committed Jun 18, 2018
1 parent f239b80 commit 5514c43
Show file tree
Hide file tree
Showing 22 changed files with 721 additions and 159 deletions.
94 changes: 88 additions & 6 deletions .golangci.example.yml
Original file line number Diff line number Diff line change
@@ -1,40 +1,98 @@
# This file contains all available configuration options
# with their default values.

# options for analysis running
run:
# default concurrency is a available CPU number
concurrency: 4

# timeout for analysis, e.g. 30s, 5m, default is 1m
deadline: 1m

# exit code when at least one issue was found, default is 1
issues-exit-code: 1

# include test files or not, default is true
tests: true

# list of build tags, all linters use it. Default is empty list.
build-tags:
- mytag

# which dirs to skip: they won't be analyzed;
# can use regexp here: generated.*, regexp is applied on full path;
# default value is empty list, but next dirs are always skipped independently
# from this option's value:
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
skip-dirs:
- src/external_libs
- autogenerated_by_my_lib

# which files to skip: they will be analyzed, but issues from them
# won't be reported. Default value is empty list, but there is
# no need to include all autogenerated files, we confidently recognize
# autogenerated files. If it's not please let us know.
skip-files:
- ".*\\.pb\\.go$"
- ".*\\.my\\.go$"
- lib/bad.go

# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
format: colored-line-number

# print lines of code with issue, default is true
print-issued-lines: true

# print linter name in the end of issue text, default is true
print-linter-name: true

# all available settings of specific linters
linters-settings:
errcheck:
# report about not checking of errors in type assetions: `a := b.(MyStruct)`;
# default is false: such cases aren't reported by default.
check-type-assertions: false

# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
# default is false: such cases aren't reported by default.
check-blank: false
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
gofmt:
# simplify code: gofmt with `-s` option, true by default
simplify: true
gocyclo:
# minimal code complexity to report, 30 by default (but we recommend 10-20)
min-complexity: 10
maligned:
# print struct with more effective memory layout or not, false by default
suggest-new: true
dupl:
threshold: 50
# tokens count to trigger issue, 150 by default
threshold: 100
goconst:
# minimal length of string constant, 3 by default
min-len: 3
# minimal occurrences count to trigger, 3 by default
min-occurrences: 3
depguard:
list-type: blacklist
Expand All @@ -45,8 +103,8 @@ linters-settings:
linters:
enable:
- megacheck
- vet
enable-all: true
- govet
enable-all: false
disable:
maligned
disable-all: false
Expand All @@ -56,11 +114,35 @@ linters:
fast: false

issues:
# List of regexps of issue texts to exclude, empty list by default.
# But independently from this option we use default exclude patterns,
# it can be disabled by `exclude-use-default: false`. To list all
# excluded by default patterns execute `golangci-lint run --help`
exclude:
- abcdef

# Independently from option `exclude` we use default exclude patterns,
# it can be disabled by this option. To list all
# excluded by default patterns execute `golangci-lint run --help`.
# Default value for this option is false.
exclude-use-default: true

# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
max-per-linter: 0

# Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
max-same: 0

# Show only new issues: if there are unstaged changes or untracked files,
# only those changes are analyzed, else only changes in HEAD~ are analyzed.
# It's a super-useful option for integration of golangci-lint into existing
# large codebase. It's not practical to fix all existing issues at the moment
# of integration: much better don't allow issues in new code.
# Default is false.
new: false
new-from-rev: ""
new-from-patch: ""

# Show only new issues created after git revision `REV`
new-from-rev: REV

# Show only new issues created in git patch with set file path.
new-from-patch: path/to/patch/file
2 changes: 1 addition & 1 deletion Gopkg.lock

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

1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
test:
go install ./cmd/... # needed for govet and golint if go < 1.10
GL_TEST_RUN=1 golangci-lint run -v
GL_TEST_RUN=1 golangci-lint run --fast --no-config -v
GL_TEST_RUN=1 golangci-lint run --no-config -v
Expand Down
165 changes: 158 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ GolangCI-Lint can be used with zero configuration. By default the following lint
```
$ golangci-lint 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: true]
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]
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]
Expand Down Expand Up @@ -311,10 +311,161 @@ To see which config file is being used and where it was sourced from run golangc
Config options inside the file are identical to command-line options.
You can configure specific linters' options only within the config file (not the command-line).

There is a [`.golangci.yml`](https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml) example config file with all supported options.
There is a [`.golangci.yml`](https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml) example
config file with all supported options, their description and default value:
```yaml
# This file contains all available configuration options
# with their default values.

# options for analysis running
run:
# default concurrency is a available CPU number
concurrency: 4

# timeout for analysis, e.g. 30s, 5m, default is 1m
deadline: 1m

# exit code when at least one issue was found, default is 1
issues-exit-code: 1

# include test files or not, default is true
tests: true

# list of build tags, all linters use it. Default is empty list.
build-tags:
- mytag

# which dirs to skip: they won't be analyzed;
# can use regexp here: generated.*, regexp is applied on full path;
# default value is empty list, but next dirs are always skipped independently
# from this option's value:
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
skip-dirs:
- src/external_libs
- autogenerated_by_my_lib

# which files to skip: they will be analyzed, but issues from them
# won't be reported. Default value is empty list, but there is
# no need to include all autogenerated files, we confidently recognize
# autogenerated files. If it's not please let us know.
skip-files:
- ".*\\.my\\.go$"
- lib/bad.go

# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
format: colored-line-number

# print lines of code with issue, default is true
print-issued-lines: true

# print linter name in the end of issue text, default is true
print-linter-name: true

# all available settings of specific linters
linters-settings:
errcheck:
# report about not checking of errors in type assetions: `a := b.(MyStruct)`;
# default is false: such cases aren't reported by default.
check-type-assertions: false

# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
# default is false: such cases aren't reported by default.
check-blank: false
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
gofmt:
# simplify code: gofmt with `-s` option, true by default
simplify: true
gocyclo:
# minimal code complexity to report, 30 by default (but we recommend 10-20)
min-complexity: 10
maligned:
# print struct with more effective memory layout or not, false by default
suggest-new: true
dupl:
# tokens count to trigger issue, 150 by default
threshold: 100
goconst:
# minimal length of string constant, 3 by default
min-len: 3
# minimal occurrences count to trigger, 3 by default
min-occurrences: 3
depguard:
list-type: blacklist
include-go-root: false
packages:
- github.com/davecgh/go-spew/spew

linters:
enable:
- megacheck
- govet
enable-all: false
disable:
maligned
disable-all: false
presets:
- bugs
- unused
fast: false

issues:
# List of regexps of issue texts to exclude, empty list by default.
# But independently from this option we use default exclude patterns,
# it can be disabled by `exclude-use-default: false`. To list all
# excluded by default patterns execute `golangci-lint run --help`
exclude:
- abcdef

# Independently from option `exclude` we use default exclude patterns,
# it can be disabled by this option. To list all
# excluded by default patterns execute `golangci-lint run --help`.
# Default value for this option is false.
exclude-use-default: true

# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
max-per-linter: 0

# Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
max-same: 0

# Show only new issues: if there are unstaged changes or untracked files,
# only those changes are analyzed, else only changes in HEAD~ are analyzed.
# It's a super-useful option for integration of golangci-lint into existing
# large codebase. It's not practical to fix all existing issues at the moment
# of integration: much better don't allow issues in new code.
# Default is false.
new: false

# Show only new issues created after git revision `REV`
new-from-rev: REV

# Show only new issues created in git patch with set file path.
new-from-patch: path/to/patch/file
```
It's a [.golangci.yml](https://github.com/golangci/golangci-lint/blob/master/.golangci.yml) config file of this repo: we enable more linters
than the default and more strict settings:
than the default and have more strict settings:
```yaml
linters-settings:
govet:
Expand Down Expand Up @@ -410,11 +561,11 @@ go install ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint/
```
Vendoring `golangci-lint` saves a network request, potentially making your CI system a little more reliable.

**`govet` or `golint` reports false-positives or false-negatives**
**Does I need to run `go install`?**

These linters obtain type information from installed package files.
The same issue you will encounter if will run `govet` or `golint` without `golangci-lint`.
Try `go install ./...` (or `go install ./cmd/...`: depends on the project) first.
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`.

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

Expand Down
16 changes: 10 additions & 6 deletions README.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,14 @@ To see which config file is being used and where it was sourced from run golangc
Config options inside the file are identical to command-line options.
You can configure specific linters' options only within the config file (not the command-line).

There is a [`.golangci.yml`](https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml) example config file with all supported options.
There is a [`.golangci.yml`](https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml) example
config file with all supported options, their description and default value:
```yaml
{{.GolangciYamlExample}}
```

It's a [.golangci.yml](https://github.com/golangci/golangci-lint/blob/master/.golangci.yml) config file of this repo: we enable more linters
than the default and more strict settings:
than the default and have more strict settings:
```yaml
{{.GolangciYaml}}
```
Expand Down Expand Up @@ -275,11 +279,11 @@ go install ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint/
```
Vendoring `golangci-lint` saves a network request, potentially making your CI system a little more reliable.

**`govet` or `golint` reports false-positives or false-negatives**
**Does I need to run `go install`?**

These linters obtain type information from installed package files.
The same issue you will encounter if will run `govet` or `golint` without `golangci-lint`.
Try `go install ./...` (or `go install ./cmd/...`: depends on the project) first.
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`.

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

Expand Down
3 changes: 2 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ type LintersSettings struct {
CheckAssignToBlank bool `mapstructure:"check-blank"`
}
Govet struct {
CheckShadowing bool `mapstructure:"check-shadowing"`
CheckShadowing bool `mapstructure:"check-shadowing"`
UseInstalledPackages bool `mapstructure:"use-installed-packages"`
}
Golint struct {
MinConfidence float64 `mapstructure:"min-confidence"`
Expand Down
Loading

0 comments on commit 5514c43

Please sign in to comment.