diff --git a/.golangci.example.yml b/.golangci.example.yml index 8c4c43ca..ce12ab0b 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -172,10 +172,6 @@ linters-settings: # minimal code complexity to report, 30 by default (but we recommend 10-20) min-complexity: 10 - nestif: - # minimal complexity of if statements to report, 5 by default - min-complexity: 4 - goconst: # minimal length of string constant, 3 by default min-len: 3 @@ -497,6 +493,31 @@ linters-settings: # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 max-func-lines: 30 + nestif: + # minimal complexity of if statements to report, 5 by default + min-complexity: 4 + + nilnil: + # By default, nilnil checks all returned types below. + checked-types: + - ptr + - func + - iface + - map + - chan + + nolintlint: + # Enable to ensure that nolint directives are all used. Default is true. + allow-unused: false + # Disable to ensure that nolint directives don't have a leading space. Default is true. + allow-leading-space: true + # Exclude following linters from requiring an explanation. Default is []. + allow-no-explanation: [ ] + # Enable to require an explanation of nonzero length after each nolint directive. Default is false. + require-explanation: true + # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. + require-specific: true + prealloc: # XXX: we don't recommend using this linter before doing performance profiling. # For most programs usage of prealloc will be a premature optimization. @@ -528,18 +549,6 @@ linters-settings: # include method names and field names (i.e., qualified names) in checks q: false - nolintlint: - # Enable to ensure that nolint directives are all used. Default is true. - allow-unused: false - # Disable to ensure that nolint directives don't have a leading space. Default is true. - allow-leading-space: true - # Exclude following linters from requiring an explanation. Default is []. - allow-no-explanation: [] - # Enable to require an explanation of nonzero length after each nolint directive. Default is false. - require-explanation: true - # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. - require-specific: true - rowserrcheck: packages: - github.com/jmoiron/sqlx diff --git a/go.mod b/go.mod index a80493dc..db6d32e1 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.15 require ( 4d63.com/gochecknoglobals v0.0.0-20201008074935-acfc0b28355a github.com/Antonboom/errname v0.1.4 + github.com/Antonboom/nilnil v0.1.0 github.com/BurntSushi/toml v0.4.1 github.com/Djarvur/go-err113 v0.0.0-20210108212216-aea10b59be24 github.com/OpenPeeDeeP/depguard v1.0.1 diff --git a/go.sum b/go.sum index 9be25685..2682f888 100644 --- a/go.sum +++ b/go.sum @@ -46,6 +46,10 @@ contrib.go.opencensus.io/exporter/stackdriver v0.13.4/go.mod h1:aXENhDJ1Y4lIg4EU dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/Antonboom/errname v0.1.4 h1:lGSlI42Gm4bI1e+IITtXJXvxFM8N7naWimVFKcb0McY= github.com/Antonboom/errname v0.1.4/go.mod h1:jRXo3m0E0EuCnK3wbsSVH3X55Z4iTDLl6ZfCxwFj4TM= +github.com/Antonboom/nilnil v0.0.0-20210914182304-d33f75a9101c h1:GZ/bAqfmHdiHy8O0jwbGwLsK0qyw+8iuCaqNbCcV/to= +github.com/Antonboom/nilnil v0.0.0-20210914182304-d33f75a9101c/go.mod h1:PhHLvRPSghY5Y7mX4TW+BHZQYo1A8flE5H20D3IPZBo= +github.com/Antonboom/nilnil v0.1.0 h1:DLDavmg0a6G/F4Lt9t7Enrbgb3Oph6LnDE6YVsmTt74= +github.com/Antonboom/nilnil v0.1.0/go.mod h1:PhHLvRPSghY5Y7mX4TW+BHZQYo1A8flE5H20D3IPZBo= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw= github.com/BurntSushi/toml v0.4.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 201a57e2..b0d1d7cd 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -118,6 +118,7 @@ type LintersSettings struct { Misspell MisspellSettings Nakedret NakedretSettings Nestif NestifSettings + NilNil NilNilSettings NoLintLint NoLintLintSettings Prealloc PreallocSettings Predeclared PredeclaredSettings @@ -360,6 +361,10 @@ type NestifSettings struct { MinComplexity int `mapstructure:"min-complexity"` } +type NilNilSettings struct { + CheckedTypes []string `mapstructure:"checked-types"` +} + type NoLintLintSettings struct { RequireExplanation bool `mapstructure:"require-explanation"` AllowLeadingSpace bool `mapstructure:"allow-leading-space"` diff --git a/pkg/golinters/nilnil.go b/pkg/golinters/nilnil.go new file mode 100644 index 00000000..739b4d4f --- /dev/null +++ b/pkg/golinters/nilnil.go @@ -0,0 +1,30 @@ +package golinters + +import ( + "strings" + + "github.com/Antonboom/nilnil/pkg/analyzer" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewNilNil(cfg *config.NilNilSettings) *goanalysis.Linter { + a := analyzer.New() + + cfgMap := make(map[string]map[string]interface{}) + if cfg != nil && len(cfg.CheckedTypes) != 0 { + cfgMap[a.Name] = map[string]interface{}{ + "checked-types": strings.Join(cfg.CheckedTypes, ","), + } + } + + return goanalysis.NewLinter( + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + cfgMap, + ). + WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 93abb3bb..5e7fcd38 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -99,46 +99,48 @@ func enableLinterConfigs(lcs []*linter.Config, isEnabled func(lc *linter.Config) //nolint:funlen func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { - var govetCfg *config.GovetSettings - var testpackageCfg *config.TestpackageSettings + var cyclopCfg *config.Cyclop + var errorlintCfg *config.ErrorLintSettings var exhaustiveCfg *config.ExhaustiveSettings var exhaustiveStructCfg *config.ExhaustiveStructSettings - var errorlintCfg *config.ErrorLintSettings - var thelperCfg *config.ThelperSettings - var predeclaredCfg *config.PredeclaredSettings - var ifshortCfg *config.IfshortSettings - var reviveCfg *config.ReviveSettings - var cyclopCfg *config.Cyclop - var importAsCfg *config.ImportAsSettings - var ireturnCfg *config.IreturnSettings var goModDirectivesCfg *config.GoModDirectivesSettings - var tagliatelleCfg *config.TagliatelleSettings var gosecCfg *config.GoSecSettings var gosimpleCfg *config.StaticCheckSettings + var govetCfg *config.GovetSettings + var ifshortCfg *config.IfshortSettings + var importAsCfg *config.ImportAsSettings + var ireturnCfg *config.IreturnSettings + var nilNilCfg *config.NilNilSettings + var predeclaredCfg *config.PredeclaredSettings + var reviveCfg *config.ReviveSettings var staticcheckCfg *config.StaticCheckSettings var stylecheckCfg *config.StaticCheckSettings + var tagliatelleCfg *config.TagliatelleSettings + var testpackageCfg *config.TestpackageSettings + var thelperCfg *config.ThelperSettings var unusedCfg *config.StaticCheckSettings var wrapcheckCfg *config.WrapcheckSettings if m.cfg != nil { - govetCfg = &m.cfg.LintersSettings.Govet - testpackageCfg = &m.cfg.LintersSettings.Testpackage + cyclopCfg = &m.cfg.LintersSettings.Cyclop + errorlintCfg = &m.cfg.LintersSettings.ErrorLint exhaustiveCfg = &m.cfg.LintersSettings.Exhaustive exhaustiveStructCfg = &m.cfg.LintersSettings.ExhaustiveStruct - errorlintCfg = &m.cfg.LintersSettings.ErrorLint - thelperCfg = &m.cfg.LintersSettings.Thelper - predeclaredCfg = &m.cfg.LintersSettings.Predeclared - ifshortCfg = &m.cfg.LintersSettings.Ifshort - reviveCfg = &m.cfg.LintersSettings.Revive - cyclopCfg = &m.cfg.LintersSettings.Cyclop - importAsCfg = &m.cfg.LintersSettings.ImportAs - ireturnCfg = &m.cfg.LintersSettings.Ireturn goModDirectivesCfg = &m.cfg.LintersSettings.GoModDirectives - tagliatelleCfg = &m.cfg.LintersSettings.Tagliatelle gosecCfg = &m.cfg.LintersSettings.Gosec gosimpleCfg = &m.cfg.LintersSettings.Gosimple + govetCfg = &m.cfg.LintersSettings.Govet + ifshortCfg = &m.cfg.LintersSettings.Ifshort + importAsCfg = &m.cfg.LintersSettings.ImportAs + ireturnCfg = &m.cfg.LintersSettings.Ireturn + nilNilCfg = &m.cfg.LintersSettings.NilNil + predeclaredCfg = &m.cfg.LintersSettings.Predeclared + reviveCfg = &m.cfg.LintersSettings.Revive staticcheckCfg = &m.cfg.LintersSettings.Staticcheck stylecheckCfg = &m.cfg.LintersSettings.Stylecheck + tagliatelleCfg = &m.cfg.LintersSettings.Tagliatelle + testpackageCfg = &m.cfg.LintersSettings.Testpackage + thelperCfg = &m.cfg.LintersSettings.Thelper unusedCfg = &m.cfg.LintersSettings.Unused wrapcheckCfg = &m.cfg.LintersSettings.Wrapcheck } @@ -513,6 +515,11 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithPresets(linter.PresetStyle). WithLoadForGoAnalysis(). WithURL("https://github.com/butuzov/ireturn"), + linter.NewConfig(golinters.NewNilNil(nilNilCfg)). + WithPresets(linter.PresetStyle). + WithLoadForGoAnalysis(). + WithURL("https://github.com/Antonboom/nilnil"). + WithSince("v1.43.0"), // nolintlint must be last because it looks at the results of all the previous linters for unused nolint directives linter.NewConfig(golinters.NewNoLintLint()). diff --git a/test/testdata/nilnil.go b/test/testdata/nilnil.go new file mode 100644 index 00000000..34c4c8fe --- /dev/null +++ b/test/testdata/nilnil.go @@ -0,0 +1,180 @@ +//args: -Enilnil +package testdata + +import ( + "io" + "unsafe" +) + +type User struct{} + +func primitivePtr() (*int, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func structPtr() (*User, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func emptyStructPtr() (*struct{}, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func anonymousStructPtr() (*struct{ ID string }, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func chBi() (chan int, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func chIn() (chan<- int, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func chOut() (<-chan int, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func fun() (func(), error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func funWithArgsAndResults() (func(a, b, c int) (int, int), error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func iface() (interface{}, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func m1() (map[int]int, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func m2() (map[int]*User, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +type Storage struct{} + +func (s *Storage) GetUser() (*User, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func ifReturn() (*User, error) { + var s Storage + if _, err := s.GetUser(); err != nil { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + return new(User), nil +} + +func forReturn() (*User, error) { + for { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } +} + +func multipleReturn() (*User, error) { + var s Storage + + if _, err := s.GetUser(); err != nil { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + + if _, err := s.GetUser(); err != nil { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + + if _, err := s.GetUser(); err != nil { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + + return new(User), nil +} + +func nested() { + _ = func() (*User, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + + _, _ = func() (*User, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + }() +} + +func deeplyNested() { + _ = func() { + _ = func() int { + _ = func() { + _ = func() (*User, error) { + _ = func() {} + _ = func() int { return 0 } + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" + } + } + return 0 + } + } +} + +type ( + StructPtrType *User + PrimitivePtrType *int + ChannelType chan int + FuncType func(int) int + Checker interface{ Check() } +) + +func structPtrType() (StructPtrType, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func primitivePtrType() (PrimitivePtrType, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func channelType() (ChannelType, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func funcType() (FuncType, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func ifaceType() (Checker, error) { + return nil, nil // ERROR "return both the `nil` error and invalid value: use a sentinel error instead" +} + +func withoutArgs() {} +func withoutError1() *User { return nil } +func withoutError2() (*User, *User) { return nil, nil } +func withoutError3() (*User, *User, *User) { return nil, nil, nil } +func withoutError4() (*User, *User, *User, *User) { return nil, nil, nil, nil } + +// Unsupported. + +func invalidOrder() (error, *User) { return nil, nil } +func withError3rd() (*User, bool, error) { return nil, false, nil } +func withError4th() (*User, *User, *User, error) { return nil, nil, nil, nil } +func unsafePtr() (unsafe.Pointer, error) { return nil, nil } +func uintPtr() (uintptr, error) { return 0, nil } +func slice() ([]int, error) { return nil, nil } +func ifaceExtPkg() (io.Closer, error) { return nil, nil } + +func implicitNil1() (*User, error) { + err := (error)(nil) + return nil, err +} + +func implicitNil2() (*User, error) { + err := io.EOF + err = nil + return nil, err +} + +func implicitNil3() (*User, error) { + return nil, wrap(nil) +} +func wrap(err error) error { return err }