Add nilnil linter (#2236)

.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

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

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=

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"`

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)
+}

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 cyclopCfg *config.Cyclop
-	var testpackageCfg *config.TestpackageSettings
+	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
+		cyclopCfg = &m.cfg.LintersSettings.Cyclop
-		testpackageCfg = &m.cfg.LintersSettings.Testpackage
+		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()).

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 }