From 5d0367bbd5c7b3f7fe8cd68eaa44c10d1ad41f99 Mon Sep 17 00:00:00 2001 From: Oliver Eikemeier Date: Tue, 9 Jul 2024 12:17:30 +0200 Subject: [PATCH] better diagnostics, add basic mode; add errors.Is Signed-off-by: Oliver Eikemeier --- .envrc | 1 - .github/codecov.yml | 2 +- .github/workflows/test.yml | 76 +++++------ README.md | 3 +- main.go | 59 ++++++++ pkg/analyzer/analyzer.go | 13 +- pkg/analyzer/analyzer_test.go | 5 + pkg/analyzer/testdata/excluded.txt | 2 + pkg/analyzer/testdata/src/a/testdata.go | 80 ++++++++++- .../testdata/src/a/testdata.go.golden | 80 ++++++++++- pkg/analyzer/testdata/src/basic/testdata.go | 56 ++++++++ pkg/visitor/binary.go | 9 +- pkg/visitor/call.go | 128 ++++++++++++++---- pkg/visitor/cmp.go | 87 ++++++++++++ pkg/visitor/report.go | 6 +- pkg/visitor/star.go | 18 ++- pkg/visitor/unary.go | 5 +- pkg/visitor/visitor.go | 53 +++++++- pkg/visitor/zerosize.go | 21 +-- 19 files changed, 585 insertions(+), 119 deletions(-) delete mode 100644 .envrc create mode 100644 pkg/analyzer/testdata/excluded.txt create mode 100644 pkg/analyzer/testdata/src/basic/testdata.go create mode 100644 pkg/visitor/cmp.go diff --git a/.envrc b/.envrc deleted file mode 100644 index 764b5d6..0000000 --- a/.envrc +++ /dev/null @@ -1 +0,0 @@ -export GOEXPERIMENT=rangefunc diff --git a/.github/codecov.yml b/.github/codecov.yml index 861ff8c..9870e13 100644 --- a/.github/codecov.yml +++ b/.github/codecov.yml @@ -4,4 +4,4 @@ coverage: project: false patch: false ignore: - - atomic/nocopy.go + - main.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4426716..ae9b69d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,42 +1,42 @@ --- name: Test "on": - push: - branches: - - main - pull_request: - branches: - - main + push: + branches: + - main + pull_request: + branches: + - main jobs: - test: - name: Test on Go ${{ matrix.go }} - permissions: - checks: write - contents: read - pull-requests: read - statuses: write - runs-on: ubuntu-24.04 - strategy: - matrix: - go: ["1.22", "1.21"] - env: - GOTOOLCHAIN: local - steps: - - name: ✔ Check out - uses: actions/checkout@v4 - - name: 🐹 Set up Go ${{ matrix.go }} - uses: actions/setup-go@v5 - with: - go-version: ${{ matrix.go }} - check-latest: true - - name: 🧸 golangci-lint - uses: golangci/golangci-lint-action@v6 - with: - version: v1.59.1 - - name: 🔨 Test - run: go test -coverprofile=cover.out ./... - - name: 🧑🏻‍💻 codecov - uses: codecov/codecov-action@v4 - with: - files: ./cover.out - token: ${{ secrets.CODECOV_TOKEN }} + test: + name: Test on Go ${{ matrix.go }} + permissions: + checks: write + contents: read + pull-requests: read + statuses: write + runs-on: ubuntu-24.04 + strategy: + matrix: + go: ["1.22", "1.21"] + env: + GOTOOLCHAIN: local + steps: + - name: ✔ Check out + uses: actions/checkout@v4 + - name: 🐹 Set up Go ${{ matrix.go }} + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go }} + check-latest: true + - name: 🧸 golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.59.1 + - name: 🔨 Test + run: go test -coverprofile=cover.out ./... + - name: 🧑🏻‍💻 codecov + uses: codecov/codecov-action@v4 + with: + files: ./cover.out + token: ${{ secrets.CODECOV_TOKEN }} diff --git a/README.md b/README.md index 53203f2..d3f0417 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ Usage: `zerolint [-flag] [package]` Flags: - **-c** int display offending line with this many lines of context (default -1) -- **-excluded** `` read exluded types from this file +- **-basic** basic analysis only +- **-excluded** `` read excluded types from this file - **-zerotrace** trace found zero-sized types - **-fix** apply all suggested fixes diff --git a/main.go b/main.go index 7256c28..c253e4e 100644 --- a/main.go +++ b/main.go @@ -14,13 +14,72 @@ // // SPDX-License-Identifier: Apache-2.0 +// This is the main program for the zerolint linter. package main import ( + "flag" + "fmt" + "os" + "runtime/debug" + "fillmore-labs.com/zerolint/pkg/analyzer" "golang.org/x/tools/go/analysis/singlechecker" ) func main() { + a := analyzer.Analyzer + addVersionFlag(&a.Flags) singlechecker.Main(analyzer.Analyzer) } + +func addVersionFlag(s *flag.FlagSet) { + if s.Lookup("V") == nil { + s.Var(versionFlag{}, "V", "print version and exit") + } +} + +type versionFlag struct{} + +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 + } + + 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 from %s on %s\n", + progname, version, goVersion, revision, time) + + os.Exit(0) + + return nil +} diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index d759c4b..11104f3 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -40,9 +40,13 @@ var Analyzer = &analysis.Analyzer{ //nolint:gochecknoglobals 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") } -var ZeroTrace bool //nolint:gochecknoglobals +var ( + ZeroTrace bool //nolint:gochecknoglobals + Basic bool //nolint:gochecknoglobals +) func run(pass *analysis.Pass) (any, error) { excludes, err := ReadExcludes() @@ -50,7 +54,12 @@ func run(pass *analysis.Pass) (any, error) { return nil, err } - v := visitor.Visitor{Pass: pass, Excludes: excludes, ZeroTrace: ZeroTrace} + v := visitor.Visitor{ + Pass: pass, + Excludes: excludes, + ZeroTrace: ZeroTrace, + Basic: Basic, + } v.Run() return any(nil), nil diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 39e44bf..1f18ec7 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -27,5 +27,10 @@ func TestAnalyzer(t *testing.T) { //nolint:paralleltest dir := analysistest.TestData() a := analyzer.Analyzer + analyzer.Basic = true + analysistest.Run(t, dir, a, "basic") + + analyzer.Basic = false + analyzer.Excludes = dir + "/excluded.txt" analysistest.RunWithSuggestedFixes(t, dir, a, "a") } diff --git a/pkg/analyzer/testdata/excluded.txt b/pkg/analyzer/testdata/excluded.txt new file mode 100644 index 0000000..933609c --- /dev/null +++ b/pkg/analyzer/testdata/excluded.txt @@ -0,0 +1,2 @@ +# zerolint exclusions for a +a.C diff --git a/pkg/analyzer/testdata/src/a/testdata.go b/pkg/analyzer/testdata/src/a/testdata.go index b101a76..a982e9e 100644 --- a/pkg/analyzer/testdata/src/a/testdata.go +++ b/pkg/analyzer/testdata/src/a/testdata.go @@ -17,13 +17,59 @@ package a import ( + "errors" "fmt" ) +type empty struct{} + +type typedError[T any] struct { + _ [0]T +} + +func (*typedError[_]) Error() string { // want "pointer to zero-size type" + return "an error" +} + +var ( + _ error = &typedError[any]{} // want "address of zero-size variable" + ErrOne = &(typedError[int]{}) // want "address of zero-size variable" + ErrTwo = (new)(typedError[float64]) // want "new called on zero-size type" +) + +type myErrors struct{} + +func (myErrors) Is(err, target error) bool { + return false +} + +var myErrs = myErrors{} + func Exported() { var x [0]string var y [0]string + if errors.Is(nil, ErrOne) { + fmt.Println("nil") + } + + if myErrs.Is(ErrOne, ErrTwo) { + fmt.Println("nil") + } + + if errors.Is(func() error { // want "comparison of pointer to zero-size variable" + return ErrOne + }(), ErrTwo) { + fmt.Println("equal") + } + + var err *typedError[int] // want "pointer to zero-size type" + _ = errors.As(ErrOne, &err) + + _ = (new)(struct{}) // want "new called on zero-size type" + + _ = new(empty) // want "new called on zero-size type" + xp, yp := &x, &y // want "address of zero-size variable" "address of zero-size variable" _ = *xp // want "pointer to zero-size variable" @@ -36,6 +82,10 @@ func Exported() { fmt.Println("not equal") } + if xp == nil { + fmt.Println("nil") + } + _, _ = any(xp).((*[0]string)) // want "pointer to zero-size type" switch any(xp).(type) { @@ -44,6 +94,16 @@ func Exported() { } } +func Undiagnosed() { + i := 1 + 1 + i = *&i + + f := func(int) {} + f(i) + + _ = !false +} + type A [0]string type B = A @@ -62,7 +122,9 @@ func (g *greeter) String() string { // want "pointer to zero-size type" var _ fmt.Stringer = &greeter{} // want "address of zero-size variable" -var _ fmt.Stringer = (*greeter)(nil) // want "cast to pointer to zero-size variable" +var _ fmt.Stringer = (*greeter)(nil) // want "cast of nil to pointer to zero-size variable" + +var _ fmt.Stringer = new(greeter) // want "new called on zero-size type" type greeter2[T any] [5][5][0]T @@ -71,3 +133,19 @@ func (g *greeter2[T]) String() string { // want "pointer to zero-size type" } var _ fmt.Stringer = &greeter2[int]{} // want "address of zero-size variable" + +type C struct{} + +func (*C) String() string { + return "hello, world" +} + +var _ fmt.Stringer = (*C)(nil) + +type D struct{ _ int } + +func (*D) String() string { + return "hello, world" +} + +var _ fmt.Stringer = (*D)(nil) diff --git a/pkg/analyzer/testdata/src/a/testdata.go.golden b/pkg/analyzer/testdata/src/a/testdata.go.golden index 0068c11..d19c1d5 100644 --- a/pkg/analyzer/testdata/src/a/testdata.go.golden +++ b/pkg/analyzer/testdata/src/a/testdata.go.golden @@ -17,13 +17,59 @@ package a import ( + "errors" "fmt" ) +type empty struct{} + +type typedError[T any] struct { + _ [0]T +} + +func (typedError[_]) Error() string { // want "pointer to zero-size type" + return "an error" +} + +var ( + _ error = typedError[any]{} // want "address of zero-size variable" + ErrOne = (typedError[int]{}) // want "address of zero-size variable" + ErrTwo = typedError[float64]{} // want "new called on zero-size type" +) + +type myErrors struct{} + +func (myErrors) Is(err, target error) bool { + return false +} + +var myErrs = myErrors{} + func Exported() { var x [0]string var y [0]string + if errors.Is(nil, ErrOne) { + fmt.Println("nil") + } + + if myErrs.Is(ErrOne, ErrTwo) { + fmt.Println("nil") + } + + if errors.Is(func() error { // want "comparison of pointer to zero-size variable" + return ErrOne + }(), ErrTwo) { + fmt.Println("equal") + } + + var err typedError[int] // want "pointer to zero-size type" + _ = errors.As(ErrOne, &err) + + _ = struct{}{} // want "new called on zero-size type" + + _ = empty{} // want "new called on zero-size type" + xp, yp := x, y // want "address of zero-size variable" "address of zero-size variable" _ = xp // want "pointer to zero-size variable" @@ -36,6 +82,10 @@ func Exported() { fmt.Println("not equal") } + if xp == nil { + fmt.Println("nil") + } + _, _ = any(xp).(([0]string)) // want "pointer to zero-size type" switch any(xp).(type) { @@ -44,6 +94,16 @@ func Exported() { } } +func Undiagnosed() { + i := 1 + 1 + i = *&i + + f := func(int) {} + f(i) + + _ = !false +} + type A [0]string type B = A @@ -62,7 +122,9 @@ func (g greeter) String() string { // want "pointer to zero-size type" var _ fmt.Stringer = greeter{} // want "address of zero-size variable" -var _ fmt.Stringer = greeter{} // want "cast to pointer to zero-size variable" +var _ fmt.Stringer = greeter{} // want "cast of nil to pointer to zero-size variable" + +var _ fmt.Stringer = greeter{} // want "new called on zero-size type" type greeter2[T any] [5][5][0]T @@ -71,3 +133,19 @@ func (g greeter2[T]) String() string { // want "pointer to zero-size type" } var _ fmt.Stringer = greeter2[int]{} // want "address of zero-size variable" + +type C struct{} + +func (*C) String() string { + return "hello, world" +} + +var _ fmt.Stringer = (*C)(nil) + +type D struct{ _ int } + +func (*D) String() string { + return "hello, world" +} + +var _ fmt.Stringer = (*D)(nil) diff --git a/pkg/analyzer/testdata/src/basic/testdata.go b/pkg/analyzer/testdata/src/basic/testdata.go new file mode 100644 index 0000000..bbe2ce5 --- /dev/null +++ b/pkg/analyzer/testdata/src/basic/testdata.go @@ -0,0 +1,56 @@ +// 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 ( + "errors" + "fmt" +) + +type nyError struct{} + +func (*nyError) Error() string { + return "my error" +} + +var ( + ErrOne = &nyError{} + ErrTwo = new(nyError) +) + +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 *nyError + _ = errors.As(ErrOne, &err) + + 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") + } +} diff --git a/pkg/visitor/binary.go b/pkg/visitor/binary.go index 9f1b924..414dba1 100644 --- a/pkg/visitor/binary.go +++ b/pkg/visitor/binary.go @@ -26,12 +26,5 @@ func (v Visitor) visitBinary(x *ast.BinaryExpr) bool { return true } - if !v.isZeroPointer(x.X) || !v.isZeroPointer(x.Y) { - return true - } - - const message = "comparison of pointers to zero-size variables" - v.report(x.Pos(), x.End(), message, nil) - - return true + return v.visitCmp(x, x.X, x.Y) } diff --git a/pkg/visitor/call.go b/pkg/visitor/call.go index 75b187b..1aef4a0 100644 --- a/pkg/visitor/call.go +++ b/pkg/visitor/call.go @@ -18,57 +18,127 @@ package visitor import ( "bytes" + "fmt" "go/ast" "go/format" "go/token" + "go/types" "golang.org/x/tools/go/analysis" ) -func (v Visitor) visitCall(x *ast.CallExpr) bool { - if !isSingleNil(x.Args) || !v.isZeroPointer(x.Fun) { +func (v Visitor) visitCallBasic(x *ast.CallExpr) bool { // only [errors.Is] + if len(x.Args) != 2 { //nolint:mnd + return true + } + if s, ok := unwrap(x.Fun).(*ast.SelectorExpr); !ok || !v.isErrorsIs(s) { return true } - var fixes []analysis.SuggestedFix - s, ok := unwrap(x.Fun).(*ast.StarExpr) - if ok { - var buf bytes.Buffer - if err := format.Node(&buf, token.NewFileSet(), s.X); err == nil { - buf.WriteString("{}") - edit := analysis.TextEdit{ - Pos: x.Pos(), - End: x.End(), - NewText: buf.Bytes(), - } - fixes = []analysis.SuggestedFix{ - {Message: "change to pure type", TextEdits: []analysis.TextEdit{edit}}, - } - } + 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) } - const message = "cast to pointer to zero-size variable" - v.report(x.Pos(), x.End(), message, fixes) + switch f := unwrap(x.Fun).(type) { + case *ast.SelectorExpr: // check for [errors.Is] + if len(x.Args) != 2 || !v.isErrorsIs(f) { + return true + } + + return v.visitCmp(x, x.Args[0], x.Args[1]) - return fixes == nil + case *ast.Ident: // check for [builtin.new] + if !tF.IsBuiltin() || f.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) + v.report(x, message, fixes) + + return false + } + } + + return true } -func isSingleNil(args []ast.Expr) bool { - if len(args) != 1 { +func (v Visitor) isErrorsIs(f *ast.SelectorExpr) bool { + if f.Sel.Name != "Is" { return false } - i, ok := args[0].(*ast.Ident) + path, ok := v.pkgImportPath(f.X) + + return ok && (path == "errors" || path == "golang.org/x/exp/errors") +} + +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 + } + } - return ok && i.Name == "nil" + return "", false +} + +func (v Visitor) visitCast(x *ast.CallExpr, t types.Type) bool { + if len(x.Args) != 1 || !v.TypesInfo.Types[x.Args[0]].IsNil() { + return true + } + + p, ok := t.Underlying().(*types.Pointer) + if !ok { + return true + } + + e := p.Elem() + if !v.isZeroSizeType(e) { + return true + } + + 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 } func unwrap(e ast.Expr) ast.Expr { x := e - for { - p, ok := x.(*ast.ParenExpr) - if !ok { - return x - } + for p, ok := x.(*ast.ParenExpr); ok; p, ok = x.(*ast.ParenExpr) { x = p.X } + + return x +} + +func makePure(n ast.Node, x ast.Expr) []analysis.SuggestedFix { + var fixes []analysis.SuggestedFix + var buf bytes.Buffer + if err := format.Node(&buf, token.NewFileSet(), x); err == nil { + buf.WriteString("{}") + edit := analysis.TextEdit{ + Pos: n.Pos(), + End: n.End(), + NewText: buf.Bytes(), + } + fixes = []analysis.SuggestedFix{ + {Message: "change to pure type", TextEdits: []analysis.TextEdit{edit}}, + } + } + + return fixes } diff --git a/pkg/visitor/cmp.go b/pkg/visitor/cmp.go new file mode 100644 index 0000000..a2adf71 --- /dev/null +++ b/pkg/visitor/cmp.go @@ -0,0 +1,87 @@ +// 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 visitor + +import ( + "fmt" + "go/ast" + "go/types" +) + +func (v Visitor) visitCmp(n ast.Node, x, y ast.Expr) bool { //nolint:cyclop + var p [2]struct { + name string + isZeroPointer bool + isInterface bool + } + + for i, z := range []ast.Expr{x, y} { + t := v.TypesInfo.Types[z] + if t.IsNil() { + return true + } + switch x := t.Type.Underlying().(type) { + case *types.Pointer: + e := x.Elem() + p[i].name = e.String() + p[i].isZeroPointer = v.isZeroSizeType(e) + + case *types.Interface: + p[i].name = t.Type.String() + p[i].isInterface = true + + default: + p[i].name = t.Type.String() + } + } + + var message string + switch { + case p[0].isZeroPointer && p[1].isZeroPointer: + message = comparisonMessage(p[0].name, p[1].name) + + case p[0].isZeroPointer: + message = comparisonIMessage(p[0].name, p[1].name, p[1].isInterface) + + case p[1].isZeroPointer: + message = comparisonIMessage(p[1].name, p[0].name, p[0].isInterface) + + default: + return true + } + + v.report(n, message, nil) + + return true +} + +func comparisonMessage(xType, yType string) string { + if xType == yType { + return fmt.Sprintf("comparison of pointers to zero-size variables of type %q", xType) + } + + 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 { + var isIf string + if isInterface { + isIf = "interface " + } + + return fmt.Sprintf("comparison of pointer to zero-size variable of type %q with %s%q", zType, isIf, withType) +} diff --git a/pkg/visitor/report.go b/pkg/visitor/report.go index e5eca23..4d5aacb 100644 --- a/pkg/visitor/report.go +++ b/pkg/visitor/report.go @@ -42,10 +42,10 @@ func removeOp(n ast.Node, x ast.Expr) []analysis.SuggestedFix { return fixes } -func (v Visitor) report(pos, end token.Pos, message string, fixes []analysis.SuggestedFix) { +func (v Visitor) report(rng analysis.Range, message string, fixes []analysis.SuggestedFix) { v.Report(analysis.Diagnostic{ - Pos: pos, - End: end, + Pos: rng.Pos(), + End: rng.End(), Category: "zero-size", Message: message, URL: "https://pkg.go.dev/fillmore-labs.com/zerolint", diff --git a/pkg/visitor/star.go b/pkg/visitor/star.go index 9e539db..62b3c5b 100644 --- a/pkg/visitor/star.go +++ b/pkg/visitor/star.go @@ -17,25 +17,33 @@ package visitor import ( + "fmt" "go/ast" + "go/types" ) func (v Visitor) visitStar(x *ast.StarExpr) bool { - var message string t := v.TypesInfo.Types[x.X].Type + var message string + + p, ok := t.Underlying().(*types.Pointer) switch { - case v.isZeroPointerType(t): - message = "pointer to zero-size variable" + case ok: + e := p.Elem() + 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 = "pointer to zero-size type" + message = fmt.Sprintf("pointer to zero-size type %q", t.String()) default: return true } fixes := removeOp(x, x.X) - v.report(x.Pos(), x.End(), message, fixes) + v.report(x, message, fixes) return fixes == nil } diff --git a/pkg/visitor/unary.go b/pkg/visitor/unary.go index 0d15bc1..b1e99fc 100644 --- a/pkg/visitor/unary.go +++ b/pkg/visitor/unary.go @@ -17,6 +17,7 @@ package visitor import ( + "fmt" "go/ast" "go/token" ) @@ -31,9 +32,9 @@ func (v Visitor) visitUnary(x *ast.UnaryExpr) bool { return true } + message := fmt.Sprintf("address of zero-size variable of type %q", t.String()) fixes := removeOp(x, x.X) - const message = "address of zero-size variable" - v.report(x.Pos(), x.End(), message, fixes) + v.report(x, message, fixes) return fixes == nil } diff --git a/pkg/visitor/visitor.go b/pkg/visitor/visitor.go index 4e777b5..b34896a 100644 --- a/pkg/visitor/visitor.go +++ b/pkg/visitor/visitor.go @@ -18,6 +18,7 @@ package visitor import ( "go/ast" + "log" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -27,20 +28,58 @@ import ( type Visitor struct { *analysis.Pass Excludes map[string]struct{} + Detected map[string]struct{} ZeroTrace bool + Basic bool } func (v Visitor) Run() { - in := v.ResultOf[inspect.Analyzer].(*inspector.Inspector) //nolint:forcetypeassert + in, ok := v.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + log.Fatal("inspector result missing") + } + + v.Excludes["runtime.Func"] = struct{}{} + v.Detected = make(map[string]struct{}) - types := []ast.Node{ - (*ast.StarExpr)(nil), - (*ast.UnaryExpr)(nil), - (*ast.BinaryExpr)(nil), - (*ast.CallExpr)(nil), + if v.Basic { + types := []ast.Node{ + (*ast.BinaryExpr)(nil), + (*ast.CallExpr)(nil), + } + in.Nodes(types, v.visitBasic) + } else { + types := []ast.Node{ + (*ast.StarExpr)(nil), + (*ast.UnaryExpr)(nil), + (*ast.BinaryExpr)(nil), + (*ast.CallExpr)(nil), + } + in.Nodes(types, v.visit) + } + + if v.ZeroTrace { + for name := range v.Detected { + log.Printf("found zero-size type %q", name) + } + } +} + +func (v Visitor) visitBasic(n ast.Node, push bool) bool { + if !push { + return true } - in.Nodes(types, v.visit) + switch x := n.(type) { + case *ast.BinaryExpr: + return v.visitBinary(x) + + case *ast.CallExpr: + return v.visitCallBasic(x) + + default: + return true + } } func (v Visitor) visit(n ast.Node, push bool) bool { diff --git a/pkg/visitor/zerosize.go b/pkg/visitor/zerosize.go index f70b1d6..dd379c9 100644 --- a/pkg/visitor/zerosize.go +++ b/pkg/visitor/zerosize.go @@ -17,26 +17,9 @@ package visitor import ( - "go/ast" "go/types" - "log" ) -func (v Visitor) isZeroPointer(x ast.Expr) bool { - t := v.TypesInfo.Types[x].Type - - return v.isZeroPointerType(t) -} - -func (v Visitor) isZeroPointerType(t types.Type) bool { - p, ok := t.(*types.Pointer) - if !ok { - return false - } - - return v.isZeroSizeType(p.Elem()) -} - func (v Visitor) isZeroSizeType(t types.Type) bool { if !zeroSize(t) { return false @@ -47,9 +30,7 @@ func (v Visitor) isZeroSizeType(t types.Type) bool { return false } - if v.ZeroTrace { - log.Printf("found zero-size type %q", t.String()) - } + v.Detected[name] = struct{}{} return true }