diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5f9443e..f73be17 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: - name: 🧸 golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.60.3 + version: v1.61.0 - name: 🔨 Test run: | go get -C ./pkg/analyzer/testdata golang.org/x/exp/errors diff --git a/.golangci.yaml b/.golangci.yaml index 452ee08..3f68612 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -48,3 +48,7 @@ issues: linters: - govet text: "lostcancel" + - path: ^main\.go$ + linters: + - gocheckcompilerdirectives + text: "go:debug" diff --git a/go.mod b/go.mod index 0c1a2c9..d99e41e 100644 --- a/go.mod +++ b/go.mod @@ -1,12 +1,12 @@ module fillmore-labs.com/zerolint -go 1.22 +go 1.22.7 -toolchain go1.23.0 +toolchain go1.23.1 -require golang.org/x/tools v0.24.0 +require golang.org/x/tools v0.25.0 require ( - golang.org/x/mod v0.20.0 // indirect + golang.org/x/mod v0.21.0 // indirect golang.org/x/sync v0.8.0 // indirect ) diff --git a/go.sum b/go.sum index 5b7f1f0..6f1f712 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -golang.org/x/mod v0.20.0 h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0= -golang.org/x/mod v0.20.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0= +golang.org/x/mod v0.21.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/tools v0.24.0 h1:J1shsA93PJUEVaUSaay7UXAyE8aimq3GW0pjlolpa24= -golang.org/x/tools v0.24.0/go.mod h1:YhNqVBIfWHdzvTLs0d8LCuMhkKUgSUKldakyV7W/WDQ= +golang.org/x/tools v0.25.0 h1:oFU9pkj/iJgs+0DT+VMHrx+oBKs/LJMV+Uvg78sl+fE= +golang.org/x/tools v0.25.0/go.mod h1:/vtpO8WL1N9cQC3FN5zPqb//fRXskFHbLKk4OW1Q7rg= diff --git a/main.go b/main.go index e271878..77e34d9 100644 --- a/main.go +++ b/main.go @@ -14,6 +14,8 @@ // // SPDX-License-Identifier: Apache-2.0 +//go:debug gotypesalias=1 + // This is the main program for the zerolint linter. package main diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index bcb763f..481fc8d 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -41,6 +41,7 @@ func init() { //nolint:gochecknoinits Analyzer.Flags.StringVar(&Excludes, "excluded", "", "read excluded types from this file") Analyzer.Flags.BoolVar(&ZeroTrace, "zerotrace", false, "trace found zero-sized types") Analyzer.Flags.BoolVar(&Basic, "basic", false, "basic analysis only") + Analyzer.Flags.BoolVar(&Generated, "generated", false, "check generated files") } var ( @@ -52,6 +53,9 @@ var ( // Basic enables basic analysis only. Basic bool //nolint:gochecknoglobals + + // Generated enables checking generated files. + Generated bool //nolint:gochecknoglobals ) // Run applies the analyzer to a package. @@ -69,6 +73,7 @@ func run(pass *analysis.Pass) (any, error) { }, ZeroTrace: ZeroTrace, Basic: Basic, + Generated: Generated, } v.Run() diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 59bd5c5..7d005ca 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -28,7 +28,7 @@ func TestAnalyzer(t *testing.T) { //nolint:paralleltest a := analyzer.Analyzer analyzer.Basic = true - analysistest.Run(t, dir, a, "go.test/basic") + analysistest.RunWithSuggestedFixes(t, dir, a, "go.test/basic") analyzer.Basic = false analyzer.Excludes = dir + "/excluded.txt" diff --git a/pkg/analyzer/testdata/a/testdata.go b/pkg/analyzer/testdata/a/testdata.go index 717e7ef..60a6f99 100644 --- a/pkg/analyzer/testdata/a/testdata.go +++ b/pkg/analyzer/testdata/a/testdata.go @@ -17,6 +17,7 @@ package a import ( + "encoding/json" "errors" "fmt" @@ -73,6 +74,10 @@ func Exported() { fmt.Println("equal") } + _ = json.Unmarshal(nil, &myErrs) + _ = (*json.Decoder)(nil).Decode(&myErrs) + _ = json.NewDecoder(nil).Decode(&myErrs) + var err *typedError[int] // want "pointer to zero-sized type" _ = errors.As(ErrOne, &err) diff --git a/pkg/analyzer/testdata/a/testdata.go.golden b/pkg/analyzer/testdata/a/testdata.go.golden index b4dcedf..255c3bc 100644 --- a/pkg/analyzer/testdata/a/testdata.go.golden +++ b/pkg/analyzer/testdata/a/testdata.go.golden @@ -17,6 +17,7 @@ package a import ( + "encoding/json" "errors" "fmt" @@ -73,6 +74,10 @@ func Exported() { fmt.Println("equal") } + _ = json.Unmarshal(nil, &myErrs) + _ = (*json.Decoder)(nil).Decode(&myErrs) + _ = json.NewDecoder(nil).Decode(&myErrs) + var err typedError[int] // want "pointer to zero-sized type" _ = errors.As(ErrOne, &err) diff --git a/pkg/analyzer/testdata/basic/testdata.go b/pkg/analyzer/testdata/basic/testdata.go index 9a182c7..7bf4be4 100644 --- a/pkg/analyzer/testdata/basic/testdata.go +++ b/pkg/analyzer/testdata/basic/testdata.go @@ -17,6 +17,7 @@ package basic import ( + "encoding/json" "errors" "fmt" ) @@ -46,6 +47,12 @@ func Exported() { var err *myError _ = errors.As(ErrOne, &err) + var u myError + _ = json.Unmarshal(nil, &u) + d := json.NewDecoder(nil) + _ = d.Decode(&u) + _ = json.NewDecoder(nil).Decode(&u) + if ErrOne == ErrTwo { // want "comparison of pointers to zero-size variables" fmt.Println("equal") } diff --git a/pkg/analyzer/testdata/basic/testdata.go.golden b/pkg/analyzer/testdata/basic/testdata.go.golden new file mode 100644 index 0000000..b06626e --- /dev/null +++ b/pkg/analyzer/testdata/basic/testdata.go.golden @@ -0,0 +1,71 @@ +// Copyright 2024 Oliver Eikemeier. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package basic + +import ( + "encoding/json" + "errors" + "fmt" +) + +type myError struct{} + +func (myError) Error() string { // want "method receiver is pointer to zero-size variable" + return "my error" +} + +var ( + ErrOne = &myError{} + ErrTwo = new(myError) +) + +func Exported() { + if errors.Is(nil, ErrOne) { + fmt.Println("nil") + } + + if errors.Is(func() error { // want "comparison of pointer to zero-size variable" + return ErrOne + }(), ErrTwo) { + fmt.Println("equal") + } + + var err *myError + _ = errors.As(ErrOne, &err) + + var u myError + _ = json.Unmarshal(nil, &u) + d := json.NewDecoder(nil) + _ = d.Decode(&u) + _ = json.NewDecoder(nil).Decode(&u) + + if ErrOne == ErrTwo { // want "comparison of pointers to zero-size variables" + fmt.Println("equal") + } + + if ErrOne != ErrTwo { // want "comparison of pointers to zero-size variables" + fmt.Println("not equal") + } +} + +type D struct{ _ int } + +func (*D) String() string { + return "hello, world" +} + +var _ fmt.Stringer = (*D)(nil) diff --git a/pkg/visitor/call.go b/pkg/visitor/call.go index 6240dd1..d5f4229 100644 --- a/pkg/visitor/call.go +++ b/pkg/visitor/call.go @@ -21,32 +21,43 @@ import ( "fmt" "go/ast" "go/format" - "go/token" "go/types" "golang.org/x/tools/go/analysis" ) -// visitCallBasic checks for errors.Is(x, y) and errors.As(x, y). -func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { - if len(x.Args) != 2 { //nolint:mnd - return true - } +// visitCallValue checks for encoding/json#Decoder.Decode, json.Unmarshal, errors.Is and errors.As. +func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { //nolint:cyclop fun, ok := unwrap(x.Fun).(*ast.SelectorExpr) - if !ok || !v.isErrors(fun.X) { + if !ok { return true } - if fun.Sel.Name == "As" { // Do not report pointers in errors.As(..., ...). - return false + funType := v.TypesInfo.Types[fun.X] + if funType.IsValue() && funType.Type.String() == "*encoding/json.Decoder" && fun.Sel.Name == "Decode" { + return false // Do not report pointers in json.Decoder#Decode } - if fun.Sel.Name != "Is" { + switch path, ok2 := v.pathOf(fun.X); { + case !ok2: + return true + + case path == "encoding/json" && fun.Sel.Name == "Unmarshal": + return false // Do not report pointers in json.Unmarshal(..., ...). + + case len(x.Args) != 2 || path != "errors" && path != "golang.org/x/exp/errors": return true - } - // Delegate errors.Is(..., ...) to visitCmp for further analysis of the comparison. - return v.visitCmp(x, x.Args[0], x.Args[1]) + case fun.Sel.Name == "As": + return false // Do not report pointers in errors.As(..., ...) + + case fun.Sel.Name == "Is": + // Delegate errors.Is(..., ...) to visitCmp for further analysis of the comparison. + return v.visitCmp(x, x.Args[0], x.Args[1]) + + default: + return true + } } // visitCall checks for type casts T(x), errors.Is(x, y), errors.As(x, y) and new(T). @@ -67,20 +78,18 @@ func (v Visitor) visitCall(x *ast.CallExpr) bool { } // isErrors checks whether the expression is a package specifier for errors or golang.org/x/exp/errors. -func (v Visitor) isErrors(x ast.Expr) bool { +func (v Visitor) pathOf(x ast.Expr) (string, bool) { id, ok := x.(*ast.Ident) if !ok { - return false + return "", false } pkg, ok := v.TypesInfo.Uses[id].(*types.PkgName) if !ok { - return false + return "", false } - path := pkg.Imported().Path() - - return path == "errors" || path == "golang.org/x/exp/errors" + return pkg.Imported().Path(), true } // visitCast is called for type casts T(nil). @@ -97,7 +106,7 @@ func (v Visitor) visitCast(t types.Type, x *ast.CallExpr) bool { message := fmt.Sprintf("cast of nil to pointer to zero-size variable of type %q", elem) var fixes []analysis.SuggestedFix if s, ok2 := unwrap(x.Fun).(*ast.StarExpr); ok2 { - fixes = makePure(x, s.X) + fixes = v.makePure(x, s.X) } v.report(x, message, fixes) @@ -117,12 +126,12 @@ func (v Visitor) visitNew(x *ast.CallExpr) bool { arg := x.Args[0] // new(arg). argType := v.TypesInfo.Types[arg].Type - if !v.isZeroSizedType(argType) { + if !v.zeroSizedType(argType) { return true } message := fmt.Sprintf("new called on zero-sized type %q", argType) - fixes := makePure(x, arg) + fixes := v.makePure(x, arg) v.report(x, message, fixes) return false @@ -143,9 +152,9 @@ func unwrap(e ast.Expr) ast.Expr { } // makePure adds a suggested fix from (*T)(nil) or new(T) to T{}. -func makePure(n ast.Node, x ast.Expr) []analysis.SuggestedFix { +func (v Visitor) makePure(n ast.Node, x ast.Expr) []analysis.SuggestedFix { var buf bytes.Buffer - if err := format.Node(&buf, token.NewFileSet(), x); err != nil { + if err := format.Node(&buf, v.Fset, x); err != nil { return nil } buf.WriteString("{}") diff --git a/pkg/visitor/cmp.go b/pkg/visitor/cmp.go index 8285b72..f0bd058 100644 --- a/pkg/visitor/cmp.go +++ b/pkg/visitor/cmp.go @@ -24,9 +24,9 @@ import ( // comparisonInfo holds type information for comparison operands. type comparisonInfo struct { - elem types.Type - isZeroPointer bool - isInterface bool + elem types.Type + zeroPointer bool + iface bool } // visitCmp checks comparisons like x == y, x != y and errors.Is(x, y). @@ -42,14 +42,14 @@ func (v Visitor) visitCmp(n ast.Node, x, y ast.Expr) bool { var message string switch { - case p[0].isZeroPointer && p[1].isZeroPointer: + case p[0].zeroPointer && p[1].zeroPointer: message = comparisonMessage(p[0].elem, p[1].elem) - case p[0].isZeroPointer: - message = comparisonIMessage(p[0].elem, p[1].elem, p[1].isInterface) + case p[0].zeroPointer: + message = comparisonIMessage(p[0].elem, p[1].elem, p[1].iface) - case p[1].isZeroPointer: - message = comparisonIMessage(p[1].elem, p[0].elem, p[0].isInterface) + case p[1].zeroPointer: + message = comparisonIMessage(p[1].elem, p[0].elem, p[0].iface) default: return true @@ -68,11 +68,11 @@ func (v Visitor) getComparisonInfo(t types.Type) comparisonInfo { switch underlying := t.Underlying().(type) { case *types.Pointer: info.elem = underlying.Elem() - info.isZeroPointer = v.isZeroSizedType(info.elem) + info.zeroPointer = v.zeroSizedType(info.elem) case *types.Interface: info.elem = t - info.isInterface = true + info.iface = true default: info.elem = t diff --git a/pkg/visitor/func.go b/pkg/visitor/func.go index 14a14c0..a5d1339 100644 --- a/pkg/visitor/func.go +++ b/pkg/visitor/func.go @@ -19,6 +19,8 @@ package visitor import ( "fmt" "go/ast" + + "golang.org/x/tools/go/analysis" ) // visitFunc checks if the function declaration has a receiver of a pointer to a zero-sized type. @@ -35,8 +37,13 @@ func (v Visitor) visitFunc(x *ast.FuncDecl) bool { return true } + var fixes []analysis.SuggestedFix + if s, ok2 := recv.Type.(*ast.StarExpr); ok2 { + fixes = v.removeOp(s, s.X) + } + message := fmt.Sprintf("method receiver is pointer to zero-size variable of type %q", elem) - v.report(recv, message, nil) + v.report(recv, message, fixes) return true } diff --git a/pkg/visitor/report.go b/pkg/visitor/report.go index 1c61840..0bb6ff7 100644 --- a/pkg/visitor/report.go +++ b/pkg/visitor/report.go @@ -20,15 +20,14 @@ import ( "bytes" "go/ast" "go/format" - "go/token" "golang.org/x/tools/go/analysis" ) // removeOp suggests a fix that removes the operator from the given expression. -func removeOp(n ast.Node, x ast.Expr) []analysis.SuggestedFix { +func (v Visitor) removeOp(n ast.Node, x ast.Expr) []analysis.SuggestedFix { var buf bytes.Buffer - if err := format.Node(&buf, token.NewFileSet(), x); err != nil { + if err := format.Node(&buf, v.Fset, x); err != nil { return nil } diff --git a/pkg/visitor/star.go b/pkg/visitor/star.go index 384e7cb..e8f1147 100644 --- a/pkg/visitor/star.go +++ b/pkg/visitor/star.go @@ -28,19 +28,19 @@ func (v Visitor) visitStar(x *ast.StarExpr) bool { t := v.TypesInfo.Types[x.X].Type var message string if p, ok := t.Underlying().(*types.Pointer); ok { - if !v.isZeroSizedType(p.Elem()) { + if !v.zeroSizedType(p.Elem()) { return true } // *t where t is a pointer to a zero-size variable. message = fmt.Sprintf("pointer to zero-size variable of type %q", p.Elem()) - } else if v.isZeroSizedType(t) { + } else if v.zeroSizedType(t) { // *t where t is a zero-sized type. message = fmt.Sprintf("pointer to zero-sized type %q", t) } else { return true } - fixes := removeOp(x, x.X) + fixes := v.removeOp(x, x.X) v.report(x, message, fixes) return fixes == nil diff --git a/pkg/visitor/unary.go b/pkg/visitor/unary.go index 381c555..2fc7496 100644 --- a/pkg/visitor/unary.go +++ b/pkg/visitor/unary.go @@ -30,12 +30,12 @@ func (v Visitor) visitUnary(x *ast.UnaryExpr) bool { // &... t := v.TypesInfo.Types[x.X].Type - if !v.isZeroSizedType(t) { + if !v.zeroSizedType(t) { return true } message := fmt.Sprintf("address of zero-size variable of type %q", t) - fixes := removeOp(x, x.X) + fixes := v.removeOp(x, x.X) v.report(x, message, fixes) return fixes == nil diff --git a/pkg/visitor/visitor.go b/pkg/visitor/visitor.go index f677934..22fe373 100644 --- a/pkg/visitor/visitor.go +++ b/pkg/visitor/visitor.go @@ -30,6 +30,7 @@ type Run struct { Visitor ZeroTrace bool Basic bool + Generated bool } // Visitor is an AST visitor for analyzing usage of pointers to zero-size variables. @@ -50,31 +51,33 @@ func (v Run) Run() { log.Fatal("inspector result missing") } - if v.ZeroTrace { + if v.ZeroTrace && len(v.Detected) > 0 { + log.Printf("found zero-sized types in %q:\n", v.Pass.Pkg.Path()) for name := range v.Detected { - log.Printf("found zero-sized type %q", name) + log.Printf("- %s\n", name) } } } // visitFunc determines parameters and function to call for inspector.Nodes. func (v Run) visitFunc() ([]ast.Node, func(ast.Node, bool) bool) { + types := make([]ast.Node, 0, 5) //nolint:mnd + var f func(ast.Node, bool) bool + + types = append(types, (*ast.BinaryExpr)(nil), (*ast.CallExpr)(nil)) + if !v.Generated { + types = append(types, (*ast.File)(nil)) + } + if v.Basic { - return []ast.Node{ - (*ast.BinaryExpr)(nil), - (*ast.CallExpr)(nil), - (*ast.FuncDecl)(nil), - }, - v.visitBasic + types = append(types, (*ast.FuncDecl)(nil)) + f = v.visitBasic + } else { + types = append(types, (*ast.StarExpr)(nil), (*ast.UnaryExpr)(nil)) + f = v.visit } - return []ast.Node{ - (*ast.StarExpr)(nil), - (*ast.UnaryExpr)(nil), - (*ast.BinaryExpr)(nil), - (*ast.CallExpr)(nil), - }, - v.visit + return types, f } // visitBasic is the main functions called by inspector.Nodes for basic analysis. @@ -93,6 +96,9 @@ func (v Visitor) visitBasic(n ast.Node, push bool) bool { case *ast.FuncDecl: return v.visitFunc(x) + case *ast.File: + return !ast.IsGenerated(x) + default: return true } @@ -117,6 +123,9 @@ func (v Visitor) visit(n ast.Node, push bool) bool { case *ast.CallExpr: return v.visitCall(x) + case *ast.File: + return !ast.IsGenerated(x) + default: return true } diff --git a/pkg/visitor/zerosized.go b/pkg/visitor/zerosized.go index 6e14042..7e8d2f0 100644 --- a/pkg/visitor/zerosized.go +++ b/pkg/visitor/zerosized.go @@ -16,35 +16,29 @@ package visitor -import ( - "go/types" -) +import "go/types" -// zeroSizedTypePointer checks wether t is a pointer to a zero-sized type. -// It returns the element type and true if it is, false otherwise. +// zeroSizedTypePointer checks whether t is a pointer to a zero-sized type. +// It returns true, and the element type if it is, false otherwise. func (v Visitor) zeroSizedTypePointer(t types.Type) (types.Type, bool) { - if p, ok := t.Underlying().(*types.Pointer); ok && v.isZeroSizedType(p.Elem()) { + if p, ok := t.Underlying().(*types.Pointer); ok && v.zeroSizedType(p.Elem()) { return p.Elem(), true } return nil, false } -// isZeroSizedType determines whether t is a zero-sized type not excluded from detection. -func (v Visitor) isZeroSizedType(t types.Type) bool { +// zeroSizedType determines whether t is a zero-sized type not excluded from detection. +func (v Visitor) zeroSizedType(t types.Type) bool { if !zeroSized(t) { return false } // zero-sized type, check if the type's name is in the Excludes set. name := t.String() - if v.Excludes.Has(name) { - return false - } - v.Detected.Insert(name) - return true + return !v.Excludes.Has(name) } // zeroSized determines whether t is provably a zero-sized type. @@ -52,11 +46,7 @@ func zeroSized(t types.Type) bool { switch x := t.Underlying().(type) { case *types.Array: // array type, check if the array length is zero or if the element type is zero-sized. - if x.Len() == 0 { - return true - } - - return zeroSized(x.Elem()) + return x.Len() == 0 || zeroSized(x.Elem()) case *types.Struct: // struct type, check if all fields have zero-sized types.