Skip to content

Commit

Permalink
Support types from external packages (#33)
Browse files Browse the repository at this point in the history
  • Loading branch information
Antonboom committed May 9, 2024
1 parent 2cc96a1 commit 43a41a7
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 73 deletions.
78 changes: 54 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,53 +224,83 @@ func (r *RateLimiter) Allow() bool {

<br>

- Linter only checks funcs with two return arguments, the last of which has `error` type.
- Linter only checks functions with two return arguments, the last of which has `error` type.
- Next types are checked:
* pointers, functions & interfaces (`panic: invalid memory address or nil pointer dereference`);
* maps (`panic: assignment to entry in nil map`);
* channels (`fatal error: all goroutines are asleep - deadlock!`)
- `uinptr` & `unsafe.Pointer` are not checked as a special case.
- Supported only explicit `return nil, nil`.
- Types from external packages are not supported.

</details>

## Check Golang source code
## Check Go 1.22.2 source code

<details>
<summary>Click to expand</summary>

```shell
$ cd $GOROOT/src
$ nilnil ./...
/usr/local/go/src/net/sockopt_posix.go:48:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/x509/parser.go:321:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/key_agreement.go:45:2: 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 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:231:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/database/sql/driver/types.go:262:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/debug/dwarf/entry.go:882: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:450:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/http/h2_bundle.go:8644:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/http/transfer.go:768:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/http/transfer.go:778:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/http/transfer.go:801:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/go/build/build.go:1404:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/go/build/build.go:1414:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/go/build/build.go:1419: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/internal/profile/legacy_profile.go:1087:4: 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/crypto/tls/handshake_server_test.go:411:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_server_test.go:1012:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_server_test.go:1470:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:747:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:751:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:755:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_client_test.go:2712:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_server_test.go:427:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_server_test.go:1029:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/handshake_server_test.go:1490:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/quic_test.go:390:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:777:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:781:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:785:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/crypto/tls/tls_test.go:797:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/database/sql/fakedb_test.go:1200:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/database/sql/sql_test.go:938:2: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/database/sql/sql_test.go:942:2: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/encoding/xml/xml_test.go:92:4: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/main_posix_test.go:48:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/net_test.go:338:3: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/http/transport_test.go:6234:85: return both the `nil` error and invalid value: use a sentinel error instead
/usr/local/go/src/net/internal/socktest/main_test.go:48:61: return both the `nil` error and invalid value: use a sentinel error instead
```

</details>
79 changes: 31 additions & 48 deletions pkg/analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package analyzer

import (
"go/ast"
"go/types"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
Expand Down Expand Up @@ -40,29 +41,15 @@ func newNilNil() *nilNil {
}
}

var (
types = []ast.Node{(*ast.TypeSpec)(nil)}

funcAndReturns = []ast.Node{
(*ast.FuncDecl)(nil),
(*ast.FuncLit)(nil),
(*ast.ReturnStmt)(nil),
}
)

type typeSpecByName map[string]typer
var funcAndReturns = []ast.Node{
(*ast.FuncDecl)(nil),
(*ast.FuncLit)(nil),
(*ast.ReturnStmt)(nil),
}

func (n *nilNil) run(pass *analysis.Pass) (interface{}, error) {
insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

typeSpecs := typeSpecByName{
"any": newTyper(new(ast.InterfaceType)),
}
insp.Preorder(types, func(node ast.Node) {
t := node.(*ast.TypeSpec)
typeSpecs[t.Name.Name] = newTyper(t.Type)
})

var fs funcTypeStack
insp.Nodes(funcAndReturns, func(node ast.Node, push bool) (proceed bool) {
switch v := node.(type) {
Expand All @@ -88,12 +75,12 @@ func (n *nilNil) run(pass *analysis.Pass) (interface{}, error) {
}

fRes1, fRes2 := ft.Results.List[0], ft.Results.List[1]
if !(n.isDangerNilField(fRes1, typeSpecs) && n.isErrorField(fRes2)) {
if !(n.isDangerNilField(pass, fRes1) && n.isErrorField(pass, fRes2)) {
return false
}

rRes1, rRes2 := v.Results[0], v.Results[1]
if isNil(rRes1) && isNil(rRes2) {
if isNil(pass, rRes1) && isNil(pass, rRes2) {
pass.Reportf(v.Pos(), reportMsg)
}
}
Expand All @@ -104,55 +91,51 @@ func (n *nilNil) run(pass *analysis.Pass) (interface{}, error) {
return nil, nil //nolint:nilnil
}

func (n *nilNil) isDangerNilField(f *ast.Field, typeSpecs typeSpecByName) bool {
return n.isDangerNilType(f.Type, typeSpecs)
func (n *nilNil) isDangerNilField(pass *analysis.Pass, f *ast.Field) bool {
return n.isDangerNilType(pass.TypesInfo.TypeOf(f.Type))
}

func (n *nilNil) isDangerNilType(t ast.Expr, typeSpecs typeSpecByName) bool {
func (n *nilNil) isDangerNilType(t types.Type) bool {
switch v := t.(type) {
case *ast.StarExpr:
case *types.Pointer:
return n.checkedTypes.Contains(ptrType)

case *ast.FuncType:
case *types.Signature:
return n.checkedTypes.Contains(funcType)

case *ast.InterfaceType:
case *types.Interface:
return n.checkedTypes.Contains(ifaceType)

case *ast.MapType:
case *types.Map:
return n.checkedTypes.Contains(mapType)

case *ast.ChanType:
case *types.Chan:
return n.checkedTypes.Contains(chanType)

case *ast.Ident:
if t, ok := typeSpecs[v.Name]; ok {
return n.isDangerNilType(t.Type(), typeSpecs)
}
case *types.Named:
return n.isDangerNilType(v.Underlying())
}
return false
}

func (n *nilNil) isErrorField(f *ast.Field) bool {
return isIdent(f.Type, "error")
}
var errorIface = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)

func isNil(e ast.Expr) bool {
return isIdent(e, "nil")
func (n *nilNil) isErrorField(pass *analysis.Pass, f *ast.Field) bool {
t := pass.TypesInfo.TypeOf(f.Type)
if t == nil {
return false
}

_, ok := t.Underlying().(*types.Interface)
return ok && types.Implements(t, errorIface)
}

func isIdent(n ast.Node, name string) bool {
i, ok := n.(*ast.Ident)
func isNil(pass *analysis.Pass, e ast.Expr) bool {
i, ok := e.(*ast.Ident)
if !ok {
return false
}
return i.Name == name
}

type typer interface {
Type() ast.Expr
_, ok = pass.TypesInfo.ObjectOf(i).(*types.Nil)
return ok
}

func newTyper(t ast.Expr) typer { return typerImpl{t: t} } //
type typerImpl struct{ t ast.Expr } //
func (ti typerImpl) Type() ast.Expr { return ti.t }
1 change: 1 addition & 0 deletions pkg/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
func TestNilNil(t *testing.T) {
pkgs := []string{
"examples",
"strange",
}
analysistest.Run(t, analysistest.TestData(), analyzer.New(), pkgs...)
}
43 changes: 42 additions & 1 deletion pkg/analyzer/testdata/src/examples/negative.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,58 @@
package examples

import (
"bytes"
"io"
"unsafe"
)

// Not checked at all.

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 }

// Valid.

func structPtrTypeValid() (*User, error) {
if false {
return nil, io.EOF
}
return new(User), nil
}

func primitivePtrTypeValid() (*int, error) {
if false {
return nil, io.EOF
}
return new(int), nil
}

func channelTypeValid() (ChannelType, error) {
if false {
return nil, io.EOF
}
return make(ChannelType), nil
}

func funcTypeValid() (FuncType, error) {
if false {
return nil, io.EOF
}
return func(i int) int {
return 0
}, nil
}

func ifaceTypeValid() (io.Reader, error) {
if false {
return nil, io.EOF
}
return new(bytes.Buffer), nil
}

// Unsupported.

func invalidOrder() (error, *User) { return nil, nil }
Expand All @@ -19,7 +61,6 @@ 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)
Expand Down
9 changes: 9 additions & 0 deletions pkg/analyzer/testdata/src/examples/positive.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,12 @@ func deeplyNested() {
}
}
}

type MyError interface {
error
Code() string
}

func myError() (*User, MyError) {
return nil, nil // want "return both the `nil` error and invalid value: use a sentinel error instead"
}
36 changes: 36 additions & 0 deletions pkg/analyzer/testdata/src/examples/positive_external_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package examples

import (
"go/token"
"io"
"net/http"
"os"

"somepkg"
)

func structPtrTypeExtPkg() (*os.File, error) {
return nil, nil // want "return both the `nil` error and 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"
}

func channelTypeExtPkg() (somepkg.ChannelType, error) {
return nil, nil // want "return both the `nil` error and 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"
}

func ifaceTypeExtPkg() (io.Closer, error) {
return nil, nil // want "return both the `nil` error and 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"
}
3 changes: 3 additions & 0 deletions pkg/analyzer/testdata/src/somepkg/channel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package somepkg

type ChannelType chan int
12 changes: 12 additions & 0 deletions pkg/analyzer/testdata/src/strange/aliases.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package strange

type error = *int

func myOwnError() (*int, error) {
return nil, nil
}

func myOwnNil() (*int, error) {
nil := new(int)
return nil, nil
}

0 comments on commit 43a41a7

Please sign in to comment.