Skip to content

Commit

Permalink
Update gocritic
Browse files Browse the repository at this point in the history 
Fix #324, relates #314

1. Update gocritic to the latest version
2. Use proper gocritic checkers repo, old repo was archived
3. Get enabled by default gocritic checks in sync with go-critic: don't
enable performance, experimental and opinionated checks by default
4. Support of `enabled-tags` options for gocritic
5. Enable almost all gocritic checks for the project
6. Make rich debugging for gocritic
7. Meticulously validate gocritic checks config
  • Loading branch information
@jirfag
jirfag committed Jan 9, 2019
1 parent 7705f82 commit 87aae77
Show file tree
Hide file tree
Showing 86 changed files with 1,155 additions and 467 deletions.
19 changes: 12 additions & 7 deletions .golangci.example.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,22 @@ linters-settings:
range-loops: true # Report preallocation suggestions on range loops, true by default
for-loops: false # Report preallocation suggestions on for loops, false by default
gocritic:
# which checks should be enabled; can't be combined with 'disabled-checks';
# default are: [appendAssign appendCombine assignOp builtinShadow captLocal caseOrder defaultCaseOrder
# dupArg dupBranchBody dupCase elseif flagDeref ifElseChain importShadow indexAlloc paramTypeCombine
# rangeExprCopy rangeValCopy regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar typeUnparen
# underef unlambda unslice dupSubExpr hugeParam];
# all checks list: https://github.com/go-critic/checkers
# Which checks should be enabled; can't be combined with 'disabled-checks';
# See https://go-critic.github.io/overview#checks-overview
# To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
# By default list of stable checks is used.
enabled-checks:
- rangeValCopy
# which checks should be disabled; can't be combined with 'enabled-checks'; default is empty

# Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
disabled-checks:
- regexpMust

# Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks.
# Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
enabled-tags:
- performance

settings: # settings passed to gocritic
captLocal: # must be valid enabled check name
checkLocals: true
Expand Down
10 changes: 9 additions & 1 deletion .golangci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ linters-settings:
line-length: 140
goimports:
local-prefixes: github.com/golangci/golangci-lint
gocritic:
enabled-tags:
- performance
- style
- experimental
disabled-checks:
- wrapperFunc
- commentFormatting # https://github.com/go-critic/go-critic/issues/755

linters:
enable-all: true
Expand All @@ -35,4 +43,4 @@ linters:

run:
skip-dirs:
- test/testdata_etc
- test/testdata_etc
6 changes: 6 additions & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,10 @@ release:
rm -rf dist
curl -sL https://git.io/goreleaser | bash

update_deps:
GO111MODULE=on go mod verify
GO111MODULE=on go mod tidy
rm -rf vendor
GO111MODULE=on go mod vendor

.PHONY: test
27 changes: 20 additions & 7 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,17 +667,22 @@ linters-settings:
range-loops: true # Report preallocation suggestions on range loops, true by default
for-loops: false # Report preallocation suggestions on for loops, false by default
gocritic:
# which checks should be enabled; can't be combined with 'disabled-checks';
# default are: [appendAssign appendCombine assignOp builtinShadow captLocal caseOrder defaultCaseOrder
# dupArg dupBranchBody dupCase elseif flagDeref ifElseChain importShadow indexAlloc paramTypeCombine
# rangeExprCopy rangeValCopy regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar typeUnparen
# underef unlambda unslice dupSubExpr hugeParam];
# all checks list: https://github.com/go-critic/checkers
# Which checks should be enabled; can't be combined with 'disabled-checks';
# See https://go-critic.github.io/overview#checks-overview
# To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
# By default list of stable checks is used.
enabled-checks:
- rangeValCopy
# which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
# Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
disabled-checks:
- regexpMust
# Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks.
# Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
enabled-tags:
- performance
settings: # settings passed to gocritic
captLocal: # must be valid enabled check name
checkLocals: true
Expand Down Expand Up @@ -764,6 +769,14 @@ linters-settings:
line-length: 140
goimports:
local-prefixes: github.com/golangci/golangci-lint
gocritic:
enabled-tags:
- performance
- style
- experimental
disabled-checks:
- wrapperFunc
- commentFormatting # https://github.com/go-critic/go-critic/issues/755
linters:
enable-all: true
Expand Down
8 changes: 2 additions & 6 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ require (
github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6 // indirect
github.com/davecgh/go-spew v1.1.0 // indirect
github.com/fatih/color v1.6.0
github.com/go-critic/checkers v0.0.0-20181204210945-97246d3b3c67
github.com/go-lintpack/lintpack v0.5.1
github.com/go-critic/go-critic v0.0.0-20181204210945-0af0999fabfb
github.com/go-lintpack/lintpack v0.5.2
github.com/go-ole/go-ole v1.2.1 // indirect
github.com/go-toolsmith/astcast v0.0.0-20181028201508-b7a89ed70af1 // indirect
github.com/go-toolsmith/astcopy v0.0.0-20180903214859-79b422d080c4 // indirect
github.com/go-toolsmith/pkgload v0.0.0-20181120203407-5122569a890b // indirect
github.com/go-toolsmith/strparse v0.0.0-20180903215201-830b6daa1241 // indirect
github.com/go-toolsmith/typep v0.0.0-20181030061450-d63dc7650676 // indirect
github.com/gobwas/glob v0.2.3 // indirect
github.com/gogo/protobuf v1.0.0 // indirect
github.com/golang/mock v1.1.1
Expand Down
9 changes: 5 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ github.com/fatih/color v1.6.0 h1:66qjqZk8kalYAvDRtM1AdAJQI0tj4Wrue3Eq3B3pmFU=
github.com/fatih/color v1.6.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
github.com/go-critic/checkers v0.0.0-20181204210945-97246d3b3c67 h1:AhL5n4pH/qzefJ64+0RbymXZSBsvgbBaVJQCcjFaJPw=
github.com/go-critic/checkers v0.0.0-20181204210945-97246d3b3c67/go.mod h1:Cg5JCP9M6m93z6fecpRcVgD2lZf2RvPtb85ldjiShZc=
github.com/go-lintpack/lintpack v0.5.1 h1:v5D/csM90cu5PANqkj1JcNZGX/mrr3Z2Wu7Q8KuFd9M=
github.com/go-lintpack/lintpack v0.5.1/go.mod h1:NwZuYi2nUHho8XEIZ6SIxihrnPoqBTDqfpXvXAN0sXM=
github.com/go-critic/go-critic v0.0.0-20181204210945-0af0999fabfb h1:faOtDYqSVJsFEJAW+SwEMvh7alhYsb42fER6tt8yXfA=
github.com/go-critic/go-critic v0.0.0-20181204210945-0af0999fabfb/go.mod h1:PSww+HOJZQ3TN2hi6sphNiW1PhwELxbsK8+Jy1sjML8=
github.com/go-lintpack/lintpack v0.5.2 h1:DI5mA3+eKdWeJ40nU4d6Wc26qmdG8RCi/btYq0TuRN0=
github.com/go-lintpack/lintpack v0.5.2/go.mod h1:NwZuYi2nUHho8XEIZ6SIxihrnPoqBTDqfpXvXAN0sXM=
github.com/go-ole/go-ole v1.2.1 h1:2lOsA72HgjxAuMlKpFiCbHTvu44PIVkZ5hqm3RSdI/E=
github.com/go-ole/go-ole v1.2.1/go.mod h1:7FAglXiTm7HKlQRDeOQ6ZNUHidzCWXuZWq/1dTyBNF8=
github.com/go-toolsmith/astcast v0.0.0-20181028201508-b7a89ed70af1 h1:h+1eMw+tZAlgTVclcVN0/rdPaBI/RUzG0peblT6df+Q=
Expand Down Expand Up @@ -147,6 +147,7 @@ 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-20181117154741-2ddaf7f79a09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20181201035826-d0ca3933b724/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20181205014116-22934f0fdb62/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20181220024903-92cdcd90bf52 h1:oOIe9Zzq27JsS/3ACpGF1HwWnWNflZWT/3EvM7mtcEk=
golang.org/x/tools v0.0.0-20181220024903-92cdcd90bf52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
gopkg.in/airbrake/gobrake.v2 v2.0.9 h1:7z2uVWwn7oVeeugY1DtlPAy5H+KYgB1KeKTnqjNatLo=
Expand Down
97 changes: 0 additions & 97 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package config

import (
"errors"
"strings"
"time"

"github.com/golangci/golangci-lint/pkg/logutils"
)

const (
Expand Down Expand Up @@ -198,99 +194,6 @@ type PreallocSettings struct {
ForLoops bool `mapstructure:"for-loops"`
}

type GocriticCheckSettings map[string]interface{}

type GocriticSettings struct {
EnabledChecks []string `mapstructure:"enabled-checks"`
DisabledChecks []string `mapstructure:"disabled-checks"`
SettingsPerCheck map[string]GocriticCheckSettings `mapstructure:"settings"`

inferredEnabledChecks map[string]bool
}

func (s *GocriticSettings) InferEnabledChecks(log logutils.Log) {
enabledChecks := s.EnabledChecks
if len(enabledChecks) == 0 {
if len(s.DisabledChecks) != 0 {
for _, defaultCheck := range defaultGocriticEnabledChecks {
if !s.isCheckDisabled(defaultCheck) {
enabledChecks = append(enabledChecks, defaultCheck)
}
}
} else {
enabledChecks = defaultGocriticEnabledChecks
}
}

s.inferredEnabledChecks = map[string]bool{}
for _, check := range enabledChecks {
s.inferredEnabledChecks[strings.ToLower(check)] = true
}
log.Infof("Gocritic enabled checks: %s", enabledChecks)
}

func (s GocriticSettings) isCheckDisabled(name string) bool {
for _, disabledCheck := range s.DisabledChecks {
if disabledCheck == name {
return true
}
}

return false
}

func (s GocriticSettings) Validate(log logutils.Log) error {
if len(s.EnabledChecks) != 0 && len(s.DisabledChecks) != 0 {
return errors.New("both enabled and disabled check aren't allowed for gocritic")
}

for checkName := range s.SettingsPerCheck {
if !s.IsCheckEnabled(checkName) {
log.Warnf("Gocritic settings were provided for not enabled check %q", checkName)
}
}

return nil
}

func (s GocriticSettings) IsCheckEnabled(name string) bool {
return s.inferredEnabledChecks[strings.ToLower(name)]
}

// Its a good idea to keep this list in sync with the gocritic stable checks list in:
// https://github.com/go-critic/go-critic/blob/master/checkers/checkers_test.go#L63
var defaultGocriticEnabledChecks = []string{
"appendAssign",
"appendCombine",
"assignOp",
"builtinShadow",
"captLocal",
"caseOrder",
"defaultCaseOrder",
"dupArg",
"dupBranchBody",
"dupCase",
"elseif",
"flagDeref",
"ifElseChain",
"importShadow",
"indexAlloc",
"paramTypeCombine",
"rangeExprCopy",
"rangeValCopy",
"regexpMust",
"singleCaseSwitch",
"sloppyLen",
"switchTrue",
"typeSwitchVar",
"typeUnparen",
"underef",
"unlambda",
"unslice",
"dupSubExpr",
"hugeParam",
}

var defaultLintersSettings = LintersSettings{
Lll: LllSettings{
LineLength: 120,
Expand Down
Loading

0 comments on commit 87aae77

Please sign in to comment.