diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index bfc20df37c51..a3de5ad5086f 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2115,14 +2115,17 @@ linters-settings: min-complexity: 4 nilnil: + # In addition, detect opposite situation (simultaneous return of non-nil error and valid value). + # Default: false + detect-opposite: true # List of return types to check. - # Default: ["ptr", "func", "iface", "map", "chan", "uintptr", "unsafeptr"] + # Default: ["chan", "func", "iface", "map", "ptr", "uintptr", "unsafeptr"] checked-types: - - ptr + - chan - func - iface - map - - chan + - ptr - uintptr - unsafeptr diff --git a/go.mod b/go.mod index cdfc1b309b2a..1e273fce06a2 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/4meepo/tagalign v1.3.4 github.com/Abirdcfly/dupword v0.1.1 github.com/Antonboom/errname v1.0.0 - github.com/Antonboom/nilnil v0.1.9 + github.com/Antonboom/nilnil v1.0.0 github.com/Antonboom/testifylint v1.5.0 github.com/BurntSushi/toml v1.4.1-0.20240526193622-a339e1f7089c github.com/Crocmagnon/fatcontext v0.5.2 diff --git a/go.sum b/go.sum index 53887a10fec6..bcd9f1ca7345 100644 --- a/go.sum +++ b/go.sum @@ -41,8 +41,8 @@ github.com/Abirdcfly/dupword v0.1.1 h1:Bsxe0fIw6OwBtXMIncaTxCLHYO5BB+3mcsR5E8VXl github.com/Abirdcfly/dupword v0.1.1/go.mod h1:B49AcJdTYYkpd4HjgAcutNGG9HZ2JWwKunH9Y2BA6sM= github.com/Antonboom/errname v1.0.0 h1:oJOOWR07vS1kRusl6YRSlat7HFnb3mSfMl6sDMRoTBA= github.com/Antonboom/errname v1.0.0/go.mod h1:gMOBFzK/vrTiXN9Oh+HFs+e6Ndl0eTFbtsRTSRdXyGI= -github.com/Antonboom/nilnil v0.1.9 h1:eKFMejSxPSA9eLSensFmjW2XTgTwJMjZ8hUHtV4s/SQ= -github.com/Antonboom/nilnil v0.1.9/go.mod h1:iGe2rYwCq5/Me1khrysB4nwI7swQvjclR8/YRPl5ihQ= +github.com/Antonboom/nilnil v1.0.0 h1:n+v+B12dsE5tbAqRODXmEKfZv9j2KcTBrp+LkoM4HZk= +github.com/Antonboom/nilnil v1.0.0/go.mod h1:fDJ1FSFoLN6yoG65ANb1WihItf6qt9PJVTn/s2IrcII= github.com/Antonboom/testifylint v1.5.0 h1:dlUIsDMtCrZWUnvkaCz3quJCoIjaGi41GzjPBGkkJ8A= github.com/Antonboom/testifylint v1.5.0/go.mod h1:wqaJbu0Blb5Wag2wv7Z5xt+CIV+eVLxtGZrlK13z3AE= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 24f26ba4cd68..a1fc391253d2 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -2188,13 +2188,18 @@ "type": "object", "additionalProperties": false, "properties": { + "detect-opposite": { + "type": "boolean", + "description": "In addition, detect opposite situation (simultaneous return of non-nil error and valid value).", + "default": false + }, "checked-types": { "type": "array", "description": "List of return types to check.", "items": { - "enum": ["ptr", "func", "iface", "map", "chan", "uintptr", "unsafeptr"] + "enum": ["chan", "func", "iface", "map", "ptr", "uintptr", "unsafeptr"] }, - "default": ["ptr", "func", "iface", "map", "chan", "uintptr", "unsafeptr"] + "default": ["chan", "func", "iface", "map", "ptr", "uintptr", "unsafeptr"] } } }, diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index e1a777ea8127..12f9725d6bdc 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -733,7 +733,8 @@ type NestifSettings struct { } type NilNilSettings struct { - CheckedTypes []string `mapstructure:"checked-types"` + DetectOpposite bool `mapstructure:"detect-opposite"` + CheckedTypes []string `mapstructure:"checked-types"` } type NlreturnSettings struct { diff --git a/pkg/golinters/nilnil/nilnil.go b/pkg/golinters/nilnil/nilnil.go index c9237035d331..d8d677d9997e 100644 --- a/pkg/golinters/nilnil/nilnil.go +++ b/pkg/golinters/nilnil/nilnil.go @@ -1,8 +1,6 @@ package nilnil import ( - "strings" - "github.com/Antonboom/nilnil/pkg/analyzer" "golang.org/x/tools/go/analysis" @@ -10,13 +8,16 @@ import ( "github.com/golangci/golangci-lint/pkg/goanalysis" ) -func New(cfg *config.NilNilSettings) *goanalysis.Linter { +func New(settings *config.NilNilSettings) *goanalysis.Linter { a := analyzer.New() cfgMap := make(map[string]map[string]any) - if cfg != nil && len(cfg.CheckedTypes) != 0 { + if settings != nil { cfgMap[a.Name] = map[string]any{ - "checked-types": strings.Join(cfg.CheckedTypes, ","), + "detect-opposite": settings.DetectOpposite, + } + if len(settings.CheckedTypes) != 0 { + cfgMap[a.Name]["checked-types"] = settings.CheckedTypes } } diff --git a/pkg/golinters/nilnil/testdata/nilnil.go b/pkg/golinters/nilnil/testdata/nilnil.go index 970ef24ddbb4..2446df779ee7 100644 --- a/pkg/golinters/nilnil/testdata/nilnil.go +++ b/pkg/golinters/nilnil/testdata/nilnil.go @@ -3,8 +3,11 @@ package testdata import ( "bytes" + "errors" + "fmt" "go/token" "io" + "net" "net/http" "os" "unsafe" @@ -13,100 +16,100 @@ import ( type User struct{} func primitivePtr() (*int, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func structPtr() (*User, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func emptyStructPtr() (*struct{}, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func anonymousStructPtr() (*struct{ ID string }, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func unsafePtr() (unsafe.Pointer, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func uintPtr() (uintptr, error) { - return 0, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return 0, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func uintPtr0b() (uintptr, error) { - return 0b0, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return 0b0, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func uintPtr0x() (uintptr, error) { - return 0x00, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return 0x00, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func uintPtr0o() (uintptr, error) { - return 0o000, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return 0o000, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func chBi() (chan int, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func chIn() (chan<- int, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func chOut() (<-chan int, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func fun() (func(), error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func funWithArgsAndResults() (func(a, b, c int) (int, int), error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func iface() (interface{}, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func anyType() (any, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func m1() (map[int]int, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func m2() (map[int]*User, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type mapAlias = map[int]*User func m3() (mapAlias, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type Storage struct{} func (s *Storage) GetUser() (*User, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func ifReturn() (*User, error) { var s Storage if _, err := s.GetUser(); err != nil { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } return new(User), nil } func forReturn() (*User, error) { for { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } } @@ -114,15 +117,15 @@ func multipleReturn() (*User, error) { var s Storage if _, err := s.GetUser(); err != nil { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } if _, err := s.GetUser(); err != nil { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } if _, err := s.GetUser(); err != nil { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } return new(User), nil @@ -130,11 +133,11 @@ func multipleReturn() (*User, error) { func nested() { _ = func() (*User, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } _, _ = func() (*User, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" }() } @@ -145,7 +148,7 @@ func deeplyNested() { _ = func() (*User, error) { _ = func() {} _ = func() int { return 0 } - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } } return 0 @@ -159,31 +162,31 @@ type MyError interface { } func myError() (*User, MyError) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } // Types. func structPtrTypeExtPkg() (*os.File, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func primitivePtrTypeExtPkg() (*token.Token, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func funcTypeExtPkg() (http.HandlerFunc, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func ifaceTypeExtPkg() (io.Closer, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type closerAlias = io.Closer func ifaceTypeAliasedExtPkg() (closerAlias, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type ( @@ -195,29 +198,29 @@ type ( ) func structPtrType() (StructPtrType, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func primitivePtrType() (PrimitivePtrType, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func channelType() (ChannelType, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func funcType() (FuncType, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } func ifaceType() (Checker, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type checkerAlias = Checker func ifaceTypeAliased() (checkerAlias, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } type ( @@ -226,7 +229,7 @@ type ( ) func ptrIntegerType() (PtrIntegerType, error) { - return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead" + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" } // Not checked at all. @@ -317,3 +320,57 @@ func implicitNil3() (*User, error) { return nil, wrap(nil) } func wrap(err error) error { return err } + +// Opposite. + +func primitivePtrTypeOpposite() (*int, error) { + if false { + return nil, io.EOF + } + return new(int), errors.New("validation failed") +} + +func structPtrTypeOpposite() (*User, error) { + if false { + return nil, io.EOF + } + return new(User), fmt.Errorf("invalid %v", 42) +} + +func unsafePtrOpposite() (unsafe.Pointer, error) { + if false { + return nil, io.EOF + } + var i int + return unsafe.Pointer(&i), io.EOF +} + +func uintPtrOpposite() (uintptr, error) { + if false { + return 0, io.EOF + } + return 0xc82000c290, wrap(io.EOF) +} + +func channelTypeOpposite() (ChannelType, error) { + if false { + return nil, io.EOF + } + return make(ChannelType), fmt.Errorf("wrapped: %w", io.EOF) +} + +func funcTypeOpposite() (FuncType, error) { + if false { + return nil, io.EOF + } + return func(i int) int { + return 0 + }, errors.New("no func type, please") +} + +func ifaceTypeOpposite() (io.Reader, error) { + if false { + return nil, io.EOF + } + return new(bytes.Buffer), new(net.AddrError) +} diff --git a/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.go b/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.go new file mode 100644 index 000000000000..799d86de6796 --- /dev/null +++ b/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.go @@ -0,0 +1,83 @@ +//golangcitest:args -Enilnil +//golangcitest:config_path testdata/nilnil_detect_opposite.yml +package testdata + +import ( + "bytes" + "errors" + "fmt" + "io" + "net" + "unsafe" +) + +func primitivePtrTypeOpposite() (*int, error) { + if false { + return nil, io.EOF + } + return new(int), errors.New("validation failed") // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func structPtrTypeOpposite() (*User, error) { + if false { + return nil, io.EOF + } + return new(User), fmt.Errorf("invalid %v", 42) // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func unsafePtrOpposite() (unsafe.Pointer, error) { + if false { + return nil, io.EOF + } + var i int + return unsafe.Pointer(&i), io.EOF // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func uintPtrOpposite() (uintptr, error) { + if false { + return 0, io.EOF + } + return 0xc82000c290, wrap(io.EOF) // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func channelTypeOpposite() (ChannelType, error) { + if false { + return nil, io.EOF + } + return make(ChannelType), fmt.Errorf("wrapped: %w", io.EOF) // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func funcTypeOpposite() (FuncType, error) { + if false { + return nil, io.EOF + } + return func(i int) int { // want "return both a non-nil error and a valid value: use separate returns instead" + return 0 + }, errors.New("no func type, please") +} + +func ifaceTypeOpposite() (io.Reader, error) { + if false { + return nil, io.EOF + } + return new(bytes.Buffer), new(net.AddrError) // want "return both a non-nil error and a valid value: use separate returns instead" +} + +type ( + User struct{} + StructPtrType *User + PrimitivePtrType *int + ChannelType chan int + FuncType func(int) int + Checker interface{ Check() } +) + +func wrap(err error) error { return err } + +func structPtr() (*int, error) { + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" +} + +func structPtrValid() (*int, error) { + return new(int), nil +} diff --git a/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.yml b/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.yml new file mode 100644 index 000000000000..1c2d5ef724fb --- /dev/null +++ b/pkg/golinters/nilnil/testdata/nilnil_detect_opposite.yml @@ -0,0 +1,3 @@ +linters-settings: + nilnil: + detect-opposite: true diff --git a/pkg/golinters/nilnil/testdata/nilnil_pointers_only.go b/pkg/golinters/nilnil/testdata/nilnil_pointers_only.go new file mode 100644 index 000000000000..f8fb1838e1a7 --- /dev/null +++ b/pkg/golinters/nilnil/testdata/nilnil_pointers_only.go @@ -0,0 +1,39 @@ +//golangcitest:args -Enilnil +//golangcitest:config_path testdata/nilnil_pointers_only.yml +package testdata + +import "unsafe" + +type User struct{} + +func primitivePtr() (*int, error) { + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" +} + +func structPtr() (*User, error) { + return nil, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" +} + +func unsafePtr() (unsafe.Pointer, error) { + return nil, nil +} + +func uintPtr0o() (uintptr, error) { + return 0o000, nil // want "return both a `nil` error and an invalid value: use a sentinel error instead" +} + +func chBi() (chan int, error) { + return nil, nil +} + +func fun() (func(), error) { + return nil, nil +} + +func anyType() (any, error) { + return nil, nil +} + +func m1() (map[int]int, error) { + return nil, nil +} diff --git a/pkg/golinters/nilnil/testdata/nilnil_pointers_only.yml b/pkg/golinters/nilnil/testdata/nilnil_pointers_only.yml new file mode 100644 index 000000000000..42d89b34b120 --- /dev/null +++ b/pkg/golinters/nilnil/testdata/nilnil_pointers_only.yml @@ -0,0 +1,5 @@ +linters-settings: + nilnil: + checked-types: + - ptr + - uintptr