Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nilnil linter #2236

Merged
merged 1 commit into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 25 additions & 16 deletions .golangci.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go.sum

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

5 changes: 5 additions & 0 deletions pkg/config/linters_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ type LintersSettings struct {
Misspell MisspellSettings
Nakedret NakedretSettings
Nestif NestifSettings
NilNil NilNilSettings
NoLintLint NoLintLintSettings
Prealloc PreallocSettings
Predeclared PredeclaredSettings
Expand Down Expand Up @@ -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"`
Expand Down
30 changes: 30 additions & 0 deletions pkg/golinters/nilnil.go
Original file line number Diff line number Diff line change
@@ -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)
}
51 changes: 29 additions & 22 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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()).
Expand Down
180 changes: 180 additions & 0 deletions test/testdata/nilnil.go
Original file line number Diff line number Diff line change
@@ -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 }