checkers: add newDeref checker (go-critic#806)

Finds *new(T) expressions and suggests to simplify them
by using T zero value instead.

For example:

	*new(string) => ""
	*new(bool)   => false
	*new(myInt)  => myInt(0)

Signed-off-by: Iskander Sharipov <[email protected]>

checkers/internal/lintutil/zero_value.go

@@ -0,0 +1,93 @@
+package lintutil
+
+import (
+	"go/ast"
+	"go/constant"
+	"go/token"
+	"go/types"
+)
+
+// IsZeroValue reports whether x represents zero value of its type.
+//
+// The functions is conservative and may return false for zero values
+// if some cases are not handled in a comprehensive way
+// but is should never return true for something that's not a proper zv.
+func IsZeroValue(info *types.Info, x ast.Expr) bool {
+	switch x := x.(type) {
+	case *ast.BasicLit:
+		typ := info.TypeOf(x).Underlying().(*types.Basic)
+		v := info.Types[x].Value
+		var z constant.Value
+		switch {
+		case typ.Kind() == types.String:
+			z = constant.MakeString("")
+		case typ.Info()&types.IsInteger != 0:
+			z = constant.MakeInt64(0)
+		case typ.Info()&types.IsUnsigned != 0:
+			z = constant.MakeUint64(0)
+		case typ.Info()&types.IsFloat != 0:
+			z = constant.MakeFloat64(0)
+		default:
+			return false
+		}
+		return constant.Compare(v, token.EQL, z)
+
+	case *ast.CompositeLit:
+		return len(x.Elts) == 0
+
+	default:
+		// Note that this function is not comprehensive.
+		return false
+	}
+}
+
+// ZeroValueOf returns a zero value expression for typeExpr of type typ.
+// If function can't find such a value, nil is returned.
+func ZeroValueOf(typeExpr ast.Expr, typ types.Type) ast.Expr {
+	switch utyp := typ.Underlying().(type) {
+	case *types.Basic:
+		info := utyp.Info()
+		var zv ast.Expr
+		switch {
+		case info&types.IsInteger != 0:
+			zv = &ast.BasicLit{Kind: token.INT, Value: "0"}
+		case info&types.IsFloat != 0:
+			zv = &ast.BasicLit{Kind: token.FLOAT, Value: "0.0"}
+		case info&types.IsString != 0:
+			zv = &ast.BasicLit{Kind: token.STRING, Value: `""`}
+		case info&types.IsBoolean != 0:
+			zv = &ast.Ident{Name: "false"}
+		}
+		if isDefaultLiteralType(typ) {
+			return zv
+		}
+		return &ast.CallExpr{
+			Fun:  typeExpr,
+			Args: []ast.Expr{zv},
+		}
+
+	case *types.Slice, *types.Map, *types.Pointer, *types.Interface:
+		return &ast.CallExpr{
+			Fun:  typeExpr,
+			Args: []ast.Expr{&ast.Ident{Name: "nil"}},
+		}
+
+	case *types.Array, *types.Struct:
+		return &ast.CompositeLit{Type: typeExpr}
+	}
+
+	return nil
+}
+
+func isDefaultLiteralType(typ types.Type) bool {
+	btyp, ok := typ.(*types.Basic)
+	if !ok {
+		return false
+	}
+	switch btyp.Kind() {
+	case types.Bool, types.Int, types.Float64, types.String:
+		return true
+	default:
+		return false
+	}
+}

checkers/newDeref_checker.go

@@ -0,0 +1,45 @@
+package checkers
+
+import (
+	"go/ast"
+
+	"github.com/go-critic/go-critic/checkers/internal/lintutil"
+	"github.com/go-lintpack/lintpack"
+	"github.com/go-lintpack/lintpack/astwalk"
+	"github.com/go-toolsmith/astcast"
+	"golang.org/x/tools/go/ast/astutil"
+)
+
+func init() {
+	var info lintpack.CheckerInfo
+	info.Name = "newDeref"
+	info.Tags = []string{"style", "experimental"}
+	info.Summary = "Detects immediate dereferencing of `new` expressions"
+	info.Before = `x := *new(bool)`
+	info.After = `x := false`
+
+	collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker {
+		return astwalk.WalkerForExpr(&newDerefChecker{ctx: ctx})
+	})
+}
+
+type newDerefChecker struct {
+	astwalk.WalkHandler
+	ctx *lintpack.CheckerContext
+}
+
+func (c *newDerefChecker) VisitExpr(expr ast.Expr) {
+	deref := astcast.ToStarExpr(expr)
+	call := astcast.ToCallExpr(deref.X)
+	if astcast.ToIdent(call.Fun).Name == "new" {
+		typ := c.ctx.TypesInfo.TypeOf(call.Args[0])
+		zv := lintutil.ZeroValueOf(astutil.Unparen(call.Args[0]), typ)
+		if zv != nil {
+			c.warn(expr, zv)
+		}
+	}
+}
+
+func (c *newDerefChecker) warn(cause, suggestion ast.Expr) {
+	c.ctx.Warn(cause, "replace `%s` with `%s`", cause, suggestion)
+}

checkers/testdata/_importable/examplepkg/examplepkg.go

@@ -0,0 +1,10 @@
+package examplepkg
+
+type StructType struct {
+	A int
+	B int
+}
+
+type InterfaceType interface {
+	Method() int
+}

checkers/testdata/_integration/check_main_only/linttest.golden

@@ -29,6 +29,7 @@ exit status 1
 ./main.go:123:19: importShadow: shadow of imported package 'flag'
 ./main.go:126:6: indexAlloc: consider replacing strings.Index(string(s), sub) with bytes.Index(s, []byte(sub))
 ./main.go:130:6: methodExprCall: consider to change `point.String` to `p.String`
+./main.go:272:6: newDeref: replace `*new(string)` with `""`
 ./main.go:135:3: nilValReturn: returned expr is always nil; replace x with nil
 ./main.go:234:9: offBy1: index expr always panics; maybe you wanted xs[len(xs)-1]?
 ./main.go:140:1: paramTypeCombine: func(x int, y int) could be replaced with func(x, y int)

checkers/testdata/_integration/check_main_only/main.go

@@ -268,5 +268,9 @@ func argOrder(s string) {
 	_ = strings.HasPrefix("$", s)
 }
 
+func newDeref() {
+	_ = *new(string)
+}
+
 func main() {
 }

checkers/testdata/newDeref/negative_tests.go

@@ -0,0 +1,31 @@
+package checker_test
+
+import (
+	"github.com/go-critic/go-critic/checkers/testdata/_importable/examplepkg"
+)
+
+func fixedNewExpressions() {
+	_ = false
+	_ = ""
+	_ = 0
+	_ = 0.0
+	_ = int32(0)
+	_ = float32(0.0)
+	_ = []int(nil)
+	_ = myInt(0)
+	_ = point{}
+	_ = []*point(nil)
+	_ = point{}
+	_ = [4][2]int{}
+	_ = map[int]int(nil)
+	_ = []map[int][]int(nil)
+	_ = (*int)(nil)
+	_ = examplepkg.StructType{}
+
+	_ = interface{}(nil)
+	_ = myEface(nil)
+	_ = nonEmptyIface(nil)
+	_ = underlyingIface(nil)
+	_ = interface{}(nil)
+	_ = examplepkg.InterfaceType(nil)
+}

checkers/testdata/newDeref/positive_tests.go

@@ -0,0 +1,91 @@
+package checker_test
+
+import (
+	"github.com/go-critic/go-critic/checkers/testdata/_importable/examplepkg"
+)
+
+type point struct {
+	x float64
+	y float64
+}
+
+type myInt int
+
+func badNewExpressions() {
+	/*! replace `*new(bool)` with `false` */
+	_ = *new(bool)
+
+	/*! replace `*new(string)` with `""` */
+	_ = *new(string)
+
+	/*! replace `*new(int)` with `0` */
+	_ = *new(int)
+
+	/*! replace `*new(float64)` with `0.0` */
+	_ = *new(float64)
+
+	/*! replace `*new(int32)` with `int32(0)` */
+	_ = *new(int32)
+
+	/*! replace `*new(float32)` with `float32(0.0)` */
+	_ = *new(float32)
+
+	/*! replace `*new([]int)` with `[]int(nil)` */
+	_ = *new([]int)
+
+	/*! replace `*new(myInt)` with `myInt(0)` */
+	_ = *new(myInt)
+
+	/*! replace `*new(point)` with `point{}` */
+	_ = *new(point)
+
+	/*! replace `*new([]*point)` with `[]*point(nil)` */
+	_ = *new([]*point)
+
+	/*! replace `*new((point))` with `point{}` */
+	_ = *new((point))
+
+	/*! replace `*new([4][2]int)` with `[4][2]int{}` */
+	_ = *new([4][2]int)
+
+	/*! replace `*new(map[int]int)` with `map[int]int(nil)` */
+	_ = *new(map[int]int)
+
+	/*! replace `*new([]map[int][]int)` with `[]map[int][]int(nil)` */
+	_ = *new([]map[int][]int)
+
+	/*! replace `*new(*int)` with `(*int)(nil)` */
+	_ = *new(*int)
+
+	/*! replace `*new(examplepkg.StructType)` with `examplepkg.StructType{}` */
+	_ = *new(examplepkg.StructType)
+}
+
+type myEface interface{}
+
+type nonEmptyIface interface {
+	Foo()
+	Bar()
+}
+
+type underlyingIface nonEmptyIface
+
+func interfaceDeref() {
+	/*! replace `*new(interface{})` with `interface{}(nil)` */
+	_ = *new(interface{})
+
+	/*! replace `*new(myEface)` with `myEface(nil)` */
+	_ = *new(myEface)
+
+	/*! replace `*new(nonEmptyIface)` with `nonEmptyIface(nil)` */
+	_ = *new(nonEmptyIface)
+
+	/*! replace `*new(underlyingIface)` with `underlyingIface(nil)` */
+	_ = *new(underlyingIface)
+
+	/*! replace `*new(interface{})` with `interface{}(nil)` */
+	_ = *new(interface{})
+
+	/*! replace `*new(examplepkg.InterfaceType)` with `examplepkg.InterfaceType(nil)` */
+	_ = *new(examplepkg.InterfaceType)
+}