From 289392ed3dae3d53fa13d193ba300c65cebb7ad8 Mon Sep 17 00:00:00 2001 From: Anton Telyshev Date: Sun, 6 Oct 2024 15:04:20 +0300 Subject: [PATCH] "Detect opposite" feature (#42) * Detect opposite feature * self-review fixes * self-review fixes --- README.md | 141 +++++++++++------- Taskfile.yml | 6 +- pkg/analyzer/analyzer.go | 27 ++-- pkg/analyzer/analyzer_test.go | 25 +++- pkg/analyzer/config.go | 4 +- pkg/analyzer/testdata/src/examples/issue29.go | 2 +- .../src/examples/negative_opposite.go | 62 ++++++++ .../testdata/src/examples/positive.go | 58 +++---- .../src/examples/positive_external_types.go | 12 +- .../src/examples/positive_own_types.go | 14 +- .../src/opposite-chan-map-only/positive.go | 33 ++++ .../testdata/src/opposite/opposite.go | 73 +++++++++ .../testdata/src/pointers-only/positive.go | 4 +- 13 files changed, 346 insertions(+), 115 deletions(-) create mode 100644 pkg/analyzer/testdata/src/examples/negative_opposite.go create mode 100644 pkg/analyzer/testdata/src/opposite-chan-map-only/positive.go create mode 100644 pkg/analyzer/testdata/src/opposite/opposite.go diff --git a/README.md b/README.md index 707f56d..cd9b0dd 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,34 @@ if err != nil { // Use user. ``` +### Opposite situation + +Sometimes people consider the opposite situation (returning a non-nil error and a valid value at the same time) to be +an anti-pattern too, since it can lead to hard-to-find bugs. + +For example, this **kubernetes** code + +```go +func (fh *fakeHistory) UpdateControllerRevision(revision *apps.ControllerRevision, newRevision int64) (*apps.ControllerRevision, error) { + clone := revision.DeepCopy() + clone.Revision = newRevision + return clone, fh.indexer.Update(clone) +} +``` + +can be rewritten as follows + +```go +func (fh *fakeHistory) UpdateControllerRevision(revision *apps.ControllerRevision, newRevision int64) (*apps.ControllerRevision, error) { + clone := revision.DeepCopy() + clone.Revision = newRevision + if err := fh.indexer.Update(clone); err != nil { + return nil, fmt.Errorf("update index: %w", err) + } + return clone, nil +} +``` + ### What if I think it's bullshit? I understand that each case needs to be analyzed separately, @@ -67,8 +95,9 @@ In any case, you can just not enable the linter. ### CLI ```shell -# See help for full list of types. -$ nilnil --checked-types ptr,func ./... +$ nilnil --checked-types chan,func,iface,map,ptr,uintptr,unsafeptr ./... +$ nilnil --detect-opposite ./... +$ nilnil --detect-opposite --checked-types ptr ./.. ``` ### golangci-lint @@ -76,15 +105,21 @@ $ nilnil --checked-types ptr,func ./... https://golangci-lint.run/usage/linters/#nilnil ```yaml -nilnil: - checked-types: - - ptr - - func - - iface - - map - - chan - - uintptr - - unsafeptr +linters-settings: + 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: ["chan", "func", "iface", "map", "ptr", "uintptr", "unsafeptr"] + checked-types: + - chan + - func + - iface + - map + - ptr + - uintptr + - unsafeptr ``` ## Examples @@ -250,48 +285,48 @@ func (r *RateLimiter) Allow() bool { ```shell $ cd $GOROOT/src $ nilnil ./... 2>&1 | grep -v "_test.go" -/usr/local/go/src/internal/bisect/bisect.go:196:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/fd_unix.go:71:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/fd_unix.go:79:4: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/fd_unix.go:156:4: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/iprawsock_posix.go:36:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/tcpsock_posix.go:38:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/udpsock_posix.go:37:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/unixsock_posix.go:92:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/crypto/tls/key_agreement.go:46:2: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/crypto/tls/ticket.go:355:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/crypto/tls/ticket.go:359:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/driver/types.go:157:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/driver/types.go:232:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/driver/types.go:263:4: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/convert.go:548:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:205:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:231:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:257:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:284:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:311:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:337:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:363:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:389:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/database/sql/sql.go:422:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/debug/dwarf/entry.go:884:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/debug/dwarf/line.go:146:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/debug/dwarf/line.go:153:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/debug/dwarf/typeunit.go:138:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/debug/pe/file.go:470:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/http/h2_bundle.go:9530:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/http/transfer.go:765:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/http/transfer.go:775:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/http/transfer.go:798:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/go/build/build.go:1442:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/go/build/build.go:1453:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/go/build/build.go:1457:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/go/build/build.go:1491:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/go/internal/gccgoimporter/ar.go:125:3: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/image/jpeg/reader.go:622:5: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/image/png/reader.go:434:4: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/internal/profile/legacy_profile.go:1089:4: return both the `nil` error and invalid value: use a sentinel error instead -/usr/local/go/src/net/internal/socktest/switch.go:142:3: return both the `nil` error and invalid value: use a sentinel error instead +/usr/local/go/src/internal/bisect/bisect.go:196:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/fd_unix.go:71:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/fd_unix.go:79:4: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/fd_unix.go:156:4: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/iprawsock_posix.go:36:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/tcpsock_posix.go:38:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/udpsock_posix.go:37:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/unixsock_posix.go:92:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/crypto/tls/key_agreement.go:46:2: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/crypto/tls/ticket.go:355:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/crypto/tls/ticket.go:359:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/driver/types.go:157:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/driver/types.go:232:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/driver/types.go:263:4: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/convert.go:548:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:205:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:231:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:257:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:284:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:311:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:337:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:363:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:389:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/database/sql/sql.go:422:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/debug/dwarf/entry.go:884:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/debug/dwarf/line.go:146:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/debug/dwarf/line.go:153:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/debug/dwarf/typeunit.go:138:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/debug/pe/file.go:470:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/http/h2_bundle.go:9530:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/http/transfer.go:765:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/http/transfer.go:775:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/http/transfer.go:798:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/go/build/build.go:1442:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/go/build/build.go:1453:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/go/build/build.go:1457:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/go/build/build.go:1491:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/go/internal/gccgoimporter/ar.go:125:3: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/image/jpeg/reader.go:622:5: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/image/png/reader.go:434:4: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/internal/profile/legacy_profile.go:1089:4: return both a `nil` error and an invalid value: use a sentinel error instead +/usr/local/go/src/net/internal/socktest/switch.go:142:3: return both a `nil` error and an invalid value: use a sentinel error instead ``` diff --git a/Taskfile.yml b/Taskfile.yml index a6a633a..b6fb4b5 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -10,7 +10,7 @@ tasks: - task: tidy - task: fmt - task: lint - - task: tests + - task: test - task: install tools:install: @@ -34,12 +34,12 @@ tasks: - echo "Lint..." - golangci-lint run --fix ./... - tests: + test: cmds: - echo "Tests..." - go test ./... - tests:coverage: + test:coverage: cmds: - echo "Tests with coverage..." - go test -coverprofile=coverage.out ./... diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index f0bee07..5507d95 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -15,7 +15,8 @@ const ( name = "nilnil" doc = "Checks that there is no simultaneous return of `nil` error and an invalid value." - reportMsg = "return both the `nil` error and invalid value: use a sentinel error instead" + nilNilReportMsg = "return both a `nil` error and an invalid value: use a sentinel error instead" + notNilNotNilReportMsg = "return both a non-nil error and a valid value: use separate returns instead" ) // New returns new nilnil analyzer. @@ -29,17 +30,21 @@ func New() *analysis.Analyzer { Requires: []*analysis.Analyzer{inspect.Analyzer}, } a.Flags.Var(&n.checkedTypes, "checked-types", "comma separated list of return types to check") + a.Flags.BoolVar(&n.detectOpposite, "detect-opposite", false, + "in addition, detect opposite situation (simultaneous return of non-nil error and valid value)") return a } type nilNil struct { - checkedTypes checkedTypes + checkedTypes checkedTypes + detectOpposite bool } func newNilNil() *nilNil { return &nilNil{ - checkedTypes: newDefaultCheckedTypes(), + checkedTypes: newDefaultCheckedTypes(), + detectOpposite: false, } } @@ -93,16 +98,16 @@ func (n *nilNil) run(pass *analysis.Pass) (interface{}, error) { retVal, retErr := v.Results[0], v.Results[1] - var needWarn bool - switch zv { - case zeroValueNil: - needWarn = isNil(pass, retVal) && isNil(pass, retErr) - case zeroValueZero: - needWarn = isZero(retVal) && isNil(pass, retErr) + if ((zv == zeroValueNil) && isNil(pass, retVal) && isNil(pass, retErr)) || + ((zv == zeroValueZero) && isZero(retVal) && isNil(pass, retErr)) { + pass.Reportf(v.Pos(), nilNilReportMsg) + return false } - if needWarn { - pass.Reportf(v.Pos(), reportMsg) + if n.detectOpposite && (((zv == zeroValueNil) && !isNil(pass, retVal) && !isNil(pass, retErr)) || + ((zv == zeroValueZero) && !isZero(retVal) && !isNil(pass, retErr))) { + pass.Reportf(v.Pos(), notNilNotNilReportMsg) + return false } } diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 148a8f6..108d21f 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -18,7 +18,7 @@ func TestNilNil(t *testing.T) { analysistest.Run(t, analysistest.TestData(), analyzer.New(), pkgs...) } -func TestNilNil_Flags(t *testing.T) { +func TestNilNil_Flags_CheckedTypes(t *testing.T) { t.Parallel() anlzr := analyzer.New() @@ -27,3 +27,26 @@ func TestNilNil_Flags(t *testing.T) { } analysistest.Run(t, analysistest.TestData(), anlzr, "pointers-only") } + +func TestNilNil_Flags_DetectOpposite(t *testing.T) { + t.Parallel() + + anlzr := analyzer.New() + if err := anlzr.Flags.Set("detect-opposite", "true"); err != nil { + t.Fatal(err) + } + analysistest.Run(t, analysistest.TestData(), anlzr, "opposite") +} + +func TestNilNil_Flags_DetectOppositeAndCheckedTypes(t *testing.T) { + t.Parallel() + + anlzr := analyzer.New() + if err := anlzr.Flags.Set("detect-opposite", "true"); err != nil { + t.Fatal(err) + } + if err := anlzr.Flags.Set("checked-types", "chan,map"); err != nil { + t.Fatal(err) + } + analysistest.Run(t, analysistest.TestData(), anlzr, "opposite-chan-map-only") +} diff --git a/pkg/analyzer/config.go b/pkg/analyzer/config.go index c9b8e3e..90ae548 100644 --- a/pkg/analyzer/config.go +++ b/pkg/analyzer/config.go @@ -8,11 +8,11 @@ import ( func newDefaultCheckedTypes() checkedTypes { return checkedTypes{ - ptrType: {}, + chanType: {}, funcType: {}, ifaceType: {}, mapType: {}, - chanType: {}, + ptrType: {}, uintptrType: {}, unsafeptrType: {}, } diff --git a/pkg/analyzer/testdata/src/examples/issue29.go b/pkg/analyzer/testdata/src/examples/issue29.go index f9c169f..55b417d 100644 --- a/pkg/analyzer/testdata/src/examples/issue29.go +++ b/pkg/analyzer/testdata/src/examples/issue29.go @@ -7,7 +7,7 @@ type data struct { func nilnil(a, b int) (*data, error) { if a == 0 && b == 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 &data{a: a, b: b}, nil } diff --git a/pkg/analyzer/testdata/src/examples/negative_opposite.go b/pkg/analyzer/testdata/src/examples/negative_opposite.go new file mode 100644 index 0000000..2fe77b1 --- /dev/null +++ b/pkg/analyzer/testdata/src/examples/negative_opposite.go @@ -0,0 +1,62 @@ +package examples + +import ( + "bytes" + "errors" + "fmt" + "io" + "net" + "unsafe" +) + +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/analyzer/testdata/src/examples/positive.go b/pkg/analyzer/testdata/src/examples/positive.go index 0ce263d..d072099 100644 --- a/pkg/analyzer/testdata/src/examples/positive.go +++ b/pkg/analyzer/testdata/src/examples/positive.go @@ -5,100 +5,100 @@ import "unsafe" 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" } } @@ -106,15 +106,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 @@ -122,11 +122,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" }() } @@ -137,7 +137,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 @@ -151,5 +151,5 @@ 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" } diff --git a/pkg/analyzer/testdata/src/examples/positive_external_types.go b/pkg/analyzer/testdata/src/examples/positive_external_types.go index 25c6dad..bddde0f 100644 --- a/pkg/analyzer/testdata/src/examples/positive_external_types.go +++ b/pkg/analyzer/testdata/src/examples/positive_external_types.go @@ -10,27 +10,27 @@ import ( ) 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 channelTypeExtPkg() (somepkg.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 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" } diff --git a/pkg/analyzer/testdata/src/examples/positive_own_types.go b/pkg/analyzer/testdata/src/examples/positive_own_types.go index 0d94968..7cc74a5 100644 --- a/pkg/analyzer/testdata/src/examples/positive_own_types.go +++ b/pkg/analyzer/testdata/src/examples/positive_own_types.go @@ -9,29 +9,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 ( @@ -40,5 +40,5 @@ 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" } diff --git a/pkg/analyzer/testdata/src/opposite-chan-map-only/positive.go b/pkg/analyzer/testdata/src/opposite-chan-map-only/positive.go new file mode 100644 index 0000000..52684eb --- /dev/null +++ b/pkg/analyzer/testdata/src/opposite-chan-map-only/positive.go @@ -0,0 +1,33 @@ +package examples + +import "io" + +type User struct{} + +func primitivePtr() (*int, error) { + return new(int), io.EOF +} + +func structPtr() (*User, error) { + return new(User), io.EOF +} + +func uintPtr0o() (uintptr, error) { + return 0o000, io.EOF +} + +func chBi() (chan int, error) { + return make(chan int), io.EOF // want "return both a non-nil error and a valid value: use separate returns instead" +} + +func fun() (func(), error) { + return func() {}, io.EOF +} + +func anyType() (any, error) { + return io.EOF, io.EOF +} + +func m1() (map[int]int, error) { + return make(map[int]int), io.EOF // want "return both a non-nil error and a valid value: use separate returns instead" +} diff --git a/pkg/analyzer/testdata/src/opposite/opposite.go b/pkg/analyzer/testdata/src/opposite/opposite.go new file mode 100644 index 0000000..e9ea86f --- /dev/null +++ b/pkg/analyzer/testdata/src/opposite/opposite.go @@ -0,0 +1,73 @@ +package examples + +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 } diff --git a/pkg/analyzer/testdata/src/pointers-only/positive.go b/pkg/analyzer/testdata/src/pointers-only/positive.go index 7be3ddc..696ed7b 100644 --- a/pkg/analyzer/testdata/src/pointers-only/positive.go +++ b/pkg/analyzer/testdata/src/pointers-only/positive.go @@ -3,11 +3,11 @@ package examples 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 uintPtr0o() (uintptr, error) {