Skip to content

Commit

Permalink
Check for JSON decoder; use gotypesalias; skip generated files
Browse files Browse the repository at this point in the history
Signed-off-by: Oliver Eikemeier <[email protected]>
  • Loading branch information
eikemeier committed Sep 18, 2024
1 parent 3a9780e commit 3a6ba38
Show file tree
Hide file tree
Showing 19 changed files with 199 additions and 86 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,7 @@ issues:
linters:
- govet
text: "lostcancel"
- path: ^main\.go$
linters:
- gocheckcompilerdirectives
text: "go:debug"
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -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
)
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
@@ -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=
2 changes: 2 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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.
Expand All @@ -69,6 +73,7 @@ func run(pass *analysis.Pass) (any, error) {
},
ZeroTrace: ZeroTrace,
Basic: Basic,
Generated: Generated,
}
v.Run()

Expand Down
2 changes: 1 addition & 1 deletion pkg/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/testdata/a/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package a

import (
"encoding/json"
"errors"
"fmt"

Expand Down Expand Up @@ -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)

Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/testdata/a/testdata.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package a

import (
"encoding/json"
"errors"
"fmt"

Expand Down Expand Up @@ -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)

Expand Down
7 changes: 7 additions & 0 deletions pkg/analyzer/testdata/basic/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package basic

import (
"encoding/json"
"errors"
"fmt"
)
Expand Down Expand Up @@ -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")
}
Expand Down
71 changes: 71 additions & 0 deletions pkg/analyzer/testdata/basic/testdata.go.golden
Original file line number Diff line number Diff line change
@@ -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)
57 changes: 33 additions & 24 deletions pkg/visitor/call.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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).
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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("{}")
Expand Down
20 changes: 10 additions & 10 deletions pkg/visitor/cmp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit 3a6ba38

Please sign in to comment.