From 0ab2d38842c745cb95016192ae1f6460043ec9b1 Mon Sep 17 00:00:00 2001 From: Oliver Eikemeier Date: Tue, 30 Jul 2024 12:43:50 +0200 Subject: [PATCH] Improve tests, version Signed-off-by: Oliver Eikemeier --- .github/workflows/test.yml | 8 ++- go.mod | 8 +-- go.sum | 12 ++-- main.go | 35 ++--------- pkg/analyzer/analyzer_test.go | 4 +- pkg/analyzer/testdata/{src => }/a/testdata.go | 19 ++++-- .../testdata/{src => }/a/testdata.go.golden | 19 ++++-- .../testdata/{src => }/basic/testdata.go | 0 pkg/analyzer/testdata/excluded.txt | 4 +- pkg/analyzer/testdata/go.mod | 5 ++ pkg/analyzer/testdata/go.sum | 2 + pkg/visitor/binary.go | 1 + pkg/visitor/call.go | 60 ++++++++++--------- pkg/visitor/cmp.go | 21 ++++--- pkg/visitor/star.go | 16 ++--- pkg/visitor/unary.go | 3 +- pkg/visitor/zerosize.go | 18 ++++++ 17 files changed, 125 insertions(+), 110 deletions(-) rename pkg/analyzer/testdata/{src => }/a/testdata.go (89%) rename pkg/analyzer/testdata/{src => }/a/testdata.go.golden (89%) rename pkg/analyzer/testdata/{src => }/basic/testdata.go (100%) create mode 100644 pkg/analyzer/testdata/go.mod create mode 100644 pkg/analyzer/testdata/go.sum diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ae9b69d..44a7bb3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - go: ["1.22", "1.21"] + go: ["1.23", "1.22", "1.21"] env: GOTOOLCHAIN: local steps: @@ -32,9 +32,11 @@ jobs: - name: 🧸 golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.59.1 + version: v1.60.1 - name: 🔨 Test - run: go test -coverprofile=cover.out ./... + run: | + go get -C ./pkg/analyzer/testdata golang.org/x/exp/errors + go test -coverprofile=cover.out ./... - name: 🧑🏻‍💻 codecov uses: codecov/codecov-action@v4 with: diff --git a/go.mod b/go.mod index 3099ebe..cfa2489 100644 --- a/go.mod +++ b/go.mod @@ -2,11 +2,11 @@ module fillmore-labs.com/zerolint go 1.21 -toolchain go1.22.5 +toolchain go1.23.0 -require golang.org/x/tools v0.23.0 +require golang.org/x/tools v0.24.0 require ( - golang.org/x/mod v0.19.0 // indirect - golang.org/x/sync v0.7.0 // indirect + golang.org/x/mod v0.20.0 // indirect + golang.org/x/sync v0.8.0 // indirect ) diff --git a/go.sum b/go.sum index 025ad22..5b7f1f0 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.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= -golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= -golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg= -golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI= +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/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= diff --git a/main.go b/main.go index c253e4e..e271878 100644 --- a/main.go +++ b/main.go @@ -45,40 +45,15 @@ func (versionFlag) IsBoolFlag() bool { return true } func (versionFlag) Get() any { return nil } func (versionFlag) String() string { return "" } func (versionFlag) Set(_ string) error { - progname, err := os.Executable() - if err != nil { - return err - } + const progname = "zerolint" - var goVersion, version, revision, time string if bi, ok := debug.ReadBuildInfo(); ok { - goVersion = bi.GoVersion - version = bi.Main.Version - var modified string - for _, s := range bi.Settings { - switch s.Key { - case "vcs.revision": - revision = s.Value - - case "vcs.time": - time = s.Value - - case "vcs.modified": - modified = s.Value - } - } - - if len(revision) > 6 { //nolint:mnd - revision = revision[:7] - if len(modified) > 0 { - revision += " (dirty)" - } - } + fmt.Printf("%s version %s build with %s\n", + progname, bi.Main.Version, bi.GoVersion) + } else { + fmt.Printf("%s version (unknown)\n", progname) } - fmt.Printf("%s version %s build with %s from %s on %s\n", - progname, version, goVersion, revision, time) - os.Exit(0) return nil diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 1f18ec7..59bd5c5 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -28,9 +28,9 @@ func TestAnalyzer(t *testing.T) { //nolint:paralleltest a := analyzer.Analyzer analyzer.Basic = true - analysistest.Run(t, dir, a, "basic") + analysistest.Run(t, dir, a, "go.test/basic") analyzer.Basic = false analyzer.Excludes = dir + "/excluded.txt" - analysistest.RunWithSuggestedFixes(t, dir, a, "a") + analysistest.RunWithSuggestedFixes(t, dir, a, "go.test/a") } diff --git a/pkg/analyzer/testdata/src/a/testdata.go b/pkg/analyzer/testdata/a/testdata.go similarity index 89% rename from pkg/analyzer/testdata/src/a/testdata.go rename to pkg/analyzer/testdata/a/testdata.go index a982e9e..7c938f0 100644 --- a/pkg/analyzer/testdata/src/a/testdata.go +++ b/pkg/analyzer/testdata/a/testdata.go @@ -19,6 +19,8 @@ package a import ( "errors" "fmt" + + xerrors "golang.org/x/exp/errors" ) type empty struct{} @@ -49,15 +51,18 @@ func Exported() { var x [0]string var y [0]string - if errors.Is(nil, ErrOne) { + if errors.Is(ErrOne, nil) { fmt.Println("nil") } - if myErrs.Is(ErrOne, ErrTwo) { - fmt.Println("nil") - } + func() { + errors := myErrs + if errors.Is(ErrOne, ErrTwo) { + fmt.Println("one is two") + } + }() - if errors.Is(func() error { // want "comparison of pointer to zero-size variable" + if xerrors.Is(func() error { // want "comparison of pointer to zero-size variable" return ErrOne }(), ErrTwo) { fmt.Println("equal") @@ -116,7 +121,9 @@ func Ptr[T any](v T) *T { return &v } type greeter [5][5]struct{} -func (g *greeter) String() string { // want "pointer to zero-size type" +type greeterAlias = *greeter // want "pointer to zero-size type" + +func (g greeterAlias) String() string { return "hello, world" } diff --git a/pkg/analyzer/testdata/src/a/testdata.go.golden b/pkg/analyzer/testdata/a/testdata.go.golden similarity index 89% rename from pkg/analyzer/testdata/src/a/testdata.go.golden rename to pkg/analyzer/testdata/a/testdata.go.golden index d19c1d5..85cd0c8 100644 --- a/pkg/analyzer/testdata/src/a/testdata.go.golden +++ b/pkg/analyzer/testdata/a/testdata.go.golden @@ -19,6 +19,8 @@ package a import ( "errors" "fmt" + + xerrors "golang.org/x/exp/errors" ) type empty struct{} @@ -49,15 +51,18 @@ func Exported() { var x [0]string var y [0]string - if errors.Is(nil, ErrOne) { + if errors.Is(ErrOne, nil) { fmt.Println("nil") } - if myErrs.Is(ErrOne, ErrTwo) { - fmt.Println("nil") - } + func() { + errors := myErrs + if errors.Is(ErrOne, ErrTwo) { + fmt.Println("one is two") + } + }() - if errors.Is(func() error { // want "comparison of pointer to zero-size variable" + if xerrors.Is(func() error { // want "comparison of pointer to zero-size variable" return ErrOne }(), ErrTwo) { fmt.Println("equal") @@ -116,7 +121,9 @@ func Ptr[T any](v T) *T { return &v } type greeter [5][5]struct{} -func (g greeter) String() string { // want "pointer to zero-size type" +type greeterAlias = greeter // want "pointer to zero-size type" + +func (g greeterAlias) String() string { return "hello, world" } diff --git a/pkg/analyzer/testdata/src/basic/testdata.go b/pkg/analyzer/testdata/basic/testdata.go similarity index 100% rename from pkg/analyzer/testdata/src/basic/testdata.go rename to pkg/analyzer/testdata/basic/testdata.go diff --git a/pkg/analyzer/testdata/excluded.txt b/pkg/analyzer/testdata/excluded.txt index 933609c..3f9f9d0 100644 --- a/pkg/analyzer/testdata/excluded.txt +++ b/pkg/analyzer/testdata/excluded.txt @@ -1,2 +1,2 @@ -# zerolint exclusions for a -a.C +# zerolint exclusions for go.test/a +go.test/a.C diff --git a/pkg/analyzer/testdata/go.mod b/pkg/analyzer/testdata/go.mod new file mode 100644 index 0000000..08ea59c --- /dev/null +++ b/pkg/analyzer/testdata/go.mod @@ -0,0 +1,5 @@ +module go.test + +go 1.21 + +require golang.org/x/exp/errors v0.0.0-20240719175910-8a7402abbf56 diff --git a/pkg/analyzer/testdata/go.sum b/pkg/analyzer/testdata/go.sum new file mode 100644 index 0000000..714891b --- /dev/null +++ b/pkg/analyzer/testdata/go.sum @@ -0,0 +1,2 @@ +golang.org/x/exp/errors v0.0.0-20240719175910-8a7402abbf56 h1:wWtjcEP22Momq1fphl8WkZCnFX615H/yPDX494o7PtU= +golang.org/x/exp/errors v0.0.0-20240719175910-8a7402abbf56/go.mod h1:YgqsNsAu4fTvlab/7uiYK9LJrCIzKg/NiZUIH1/ayqo= diff --git a/pkg/visitor/binary.go b/pkg/visitor/binary.go index 414dba1..362814e 100644 --- a/pkg/visitor/binary.go +++ b/pkg/visitor/binary.go @@ -26,5 +26,6 @@ func (v Visitor) visitBinary(x *ast.BinaryExpr) bool { return true } + // X == Y or X != Y return v.visitCmp(x, x.X, x.Y) } diff --git a/pkg/visitor/call.go b/pkg/visitor/call.go index 1aef4a0..9dbde2b 100644 --- a/pkg/visitor/call.go +++ b/pkg/visitor/call.go @@ -27,7 +27,7 @@ import ( "golang.org/x/tools/go/analysis" ) -func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { // only [errors.Is] +func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { if len(x.Args) != 2 { //nolint:mnd return true } @@ -35,33 +35,37 @@ func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { // only [errors.Is] return true } + // errors.Is(..., ...) return v.visitCmp(x, x.Args[0], x.Args[1]) } func (v Visitor) visitCall(x *ast.CallExpr) bool { - tF := v.TypesInfo.Types[x.Fun] - if tF.IsType() { // check for type casts - return v.visitCast(x, tF.Type) + funType := v.TypesInfo.Types[x.Fun] + if funType.IsType() { // check for type casts + // (T)(...) + return v.visitCast(x, funType.Type) } - switch f := unwrap(x.Fun).(type) { - case *ast.SelectorExpr: // check for [errors.Is] - if len(x.Args) != 2 || !v.isErrorsIs(f) { + switch fun := unwrap(x.Fun).(type) { + case *ast.SelectorExpr: + if len(x.Args) != 2 || !v.isErrorsIs(fun) { return true } + // errors.Is(..., ...) return v.visitCmp(x, x.Args[0], x.Args[1]) - case *ast.Ident: // check for [builtin.new] - if !tF.IsBuiltin() || f.Name != "new" || len(x.Args) != 1 { + case *ast.Ident: + if !funType.IsBuiltin() || fun.Name != "new" || len(x.Args) != 1 { return true } - a := x.Args[0] - tA := v.TypesInfo.Types[a].Type - if v.isZeroSizeType(tA) { - message := fmt.Sprintf("new called on zero-size type %q", tA.String()) - fixes := makePure(x, a) + // new(...) + arg := x.Args[0] + argType := v.TypesInfo.Types[arg].Type + if v.isZeroSizeType(argType) { + message := fmt.Sprintf("new called on zero-size type %q", argType) + fixes := makePure(x, arg) v.report(x, message, fixes) return false @@ -75,19 +79,20 @@ func (v Visitor) isErrorsIs(f *ast.SelectorExpr) bool { if f.Sel.Name != "Is" { return false } - path, ok := v.pkgImportPath(f.X) - return ok && (path == "errors" || path == "golang.org/x/exp/errors") -} + id, ok := f.X.(*ast.Ident) + if !ok { + return false + } -func (v Visitor) pkgImportPath(x ast.Expr) (path string, ok bool) { - if id, ok1 := x.(*ast.Ident); ok1 { - if pkg, ok2 := v.TypesInfo.Uses[id].(*types.PkgName); ok2 { - return pkg.Imported().Path(), true - } + 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" } func (v Visitor) visitCast(x *ast.CallExpr, t types.Type) bool { @@ -96,21 +101,18 @@ func (v Visitor) visitCast(x *ast.CallExpr, t types.Type) bool { } p, ok := t.Underlying().(*types.Pointer) - if !ok { + if !ok || !v.isZeroSizeType(p.Elem()) { return true } - e := p.Elem() - if !v.isZeroSizeType(e) { - return true - } + // (*...)(nil) + message := fmt.Sprintf("cast of nil to pointer to zero-size variable of type %q", p.Elem()) var fixes []analysis.SuggestedFix if s, ok2 := unwrap(x.Fun).(*ast.StarExpr); ok2 { fixes = makePure(x, s.X) } - message := fmt.Sprintf("cast of nil to pointer to zero-size variable of type %q", e.String()) v.report(x, message, fixes) return fixes == nil diff --git a/pkg/visitor/cmp.go b/pkg/visitor/cmp.go index a2adf71..2216f9c 100644 --- a/pkg/visitor/cmp.go +++ b/pkg/visitor/cmp.go @@ -24,7 +24,7 @@ import ( func (v Visitor) visitCmp(n ast.Node, x, y ast.Expr) bool { //nolint:cyclop var p [2]struct { - name string + elem types.Type isZeroPointer bool isInterface bool } @@ -36,29 +36,28 @@ func (v Visitor) visitCmp(n ast.Node, x, y ast.Expr) bool { //nolint:cyclop } switch x := t.Type.Underlying().(type) { case *types.Pointer: - e := x.Elem() - p[i].name = e.String() - p[i].isZeroPointer = v.isZeroSizeType(e) + p[i].elem = x.Elem() + p[i].isZeroPointer = v.isZeroSizeType(x.Elem()) case *types.Interface: - p[i].name = t.Type.String() + p[i].elem = t.Type p[i].isInterface = true default: - p[i].name = t.Type.String() + p[i].elem = t.Type } } var message string switch { case p[0].isZeroPointer && p[1].isZeroPointer: - message = comparisonMessage(p[0].name, p[1].name) + message = comparisonMessage(p[0].elem, p[1].elem) case p[0].isZeroPointer: - message = comparisonIMessage(p[0].name, p[1].name, p[1].isInterface) + message = comparisonIMessage(p[0].elem, p[1].elem, p[1].isInterface) case p[1].isZeroPointer: - message = comparisonIMessage(p[1].name, p[0].name, p[0].isInterface) + message = comparisonIMessage(p[1].elem, p[0].elem, p[0].isInterface) default: return true @@ -69,7 +68,7 @@ func (v Visitor) visitCmp(n ast.Node, x, y ast.Expr) bool { //nolint:cyclop return true } -func comparisonMessage(xType, yType string) string { +func comparisonMessage(xType, yType types.Type) string { if xType == yType { return fmt.Sprintf("comparison of pointers to zero-size variables of type %q", xType) } @@ -77,7 +76,7 @@ func comparisonMessage(xType, yType string) string { return fmt.Sprintf("comparison of pointers to zero-size variables of types %q and %q", xType, yType) } -func comparisonIMessage(zType, withType string, isInterface bool) string { +func comparisonIMessage(zType, withType types.Type, isInterface bool) string { var isIf string if isInterface { isIf = "interface " diff --git a/pkg/visitor/star.go b/pkg/visitor/star.go index 62b3c5b..7ffebb2 100644 --- a/pkg/visitor/star.go +++ b/pkg/visitor/star.go @@ -23,22 +23,18 @@ import ( ) func (v Visitor) visitStar(x *ast.StarExpr) bool { + // *... t := v.TypesInfo.Types[x.X].Type var message string - p, ok := t.Underlying().(*types.Pointer) - switch { - case ok: - e := p.Elem() + if p, ok := t.Underlying().(*types.Pointer); ok { if !v.isZeroSizeType(p.Elem()) { return true } - message = fmt.Sprintf("pointer to zero-size variable of type %q", e.String()) - - case v.isZeroSizeType(t): - message = fmt.Sprintf("pointer to zero-size type %q", t.String()) - - default: + message = fmt.Sprintf("pointer to zero-size variable of type %q", p.Elem()) + } else if v.isZeroSizeType(t) { + message = fmt.Sprintf("pointer to zero-size type %q", t) + } else { return true } diff --git a/pkg/visitor/unary.go b/pkg/visitor/unary.go index b1e99fc..8cbc53e 100644 --- a/pkg/visitor/unary.go +++ b/pkg/visitor/unary.go @@ -27,12 +27,13 @@ func (v Visitor) visitUnary(x *ast.UnaryExpr) bool { return true } + // &... t := v.TypesInfo.Types[x.X].Type if !v.isZeroSizeType(t) { return true } - message := fmt.Sprintf("address of zero-size variable of type %q", t.String()) + message := fmt.Sprintf("address of zero-size variable of type %q", t) fixes := removeOp(x, x.X) v.report(x, message, fixes) diff --git a/pkg/visitor/zerosize.go b/pkg/visitor/zerosize.go index dd379c9..eaaefbc 100644 --- a/pkg/visitor/zerosize.go +++ b/pkg/visitor/zerosize.go @@ -54,6 +54,24 @@ func zeroSize(t types.Type) bool { return true + /* not really useful and doesn't work with '-fix': + case *types.Interface: + for i := 0; i < x.NumEmbeddeds(); i++ { + if zeroSize(x.EmbeddedType(i)) { + return true + } + } + return false + + case *types.Union: + for i := 0; i < x.Len(); i++ { + if !zeroSize(x.Term(i).Type()) { + return false + } + } + return true + */ + default: return false }