go/analysis: add unusedwrite pass

This checker checks for instances of writes to struct fields and arrays
that are never read.

Change-Id: Ibeb4fc45d7b87a2ea571ba23a29ec44529623f0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287034
Reviewed-by: Tim King <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Trust: Tim King <[email protected]>

Author: guodongli-google

go/analysis/passes/unusedwrite/testdata/src/a/unusedwrite.go

@@ -0,0 +1,75 @@
+package a
+
+type T1 struct{ x int }
+
+type T2 struct {
+	x int
+	y int
+}
+
+type T3 struct{ y *T1 }
+
+func BadWrites() {
+	// Test struct field writes.
+	var s1 T1
+	s1.x = 10 // want "unused write to field x"
+
+	// Test array writes.
+	var s2 [10]int
+	s2[1] = 10 // want "unused write to array index 1:int"
+
+	// Test range variables of struct type.
+	s3 := []T1{T1{x: 100}}
+	for i, v := range s3 {
+		v.x = i // want "unused write to field x"
+	}
+
+	// Test the case where a different field is read after the write.
+	s4 := []T2{T2{x: 1, y: 2}}
+	for i, v := range s4 {
+		v.x = i // want "unused write to field x"
+		_ = v.y
+	}
+}
+
+func (t T1) BadValueReceiverWrite(v T2) {
+	t.x = 10 // want "unused write to field x"
+	v.y = 20 // want "unused write to field y"
+}
+
+func GoodWrites(m map[int]int) {
+	// A map is copied by reference such that a write will affect the original map.
+	m[1] = 10
+
+	// Test struct field writes.
+	var s1 T1
+	s1.x = 10
+	print(s1.x)
+
+	// Test array writes.
+	var s2 [10]int
+	s2[1] = 10
+	// Current the checker doesn't distinguish index 1 and index 2.
+	_ = s2[2]
+
+	// Test range variables of struct type.
+	s3 := []T1{T1{x: 100}}
+	for i, v := range s3 { // v is a copy
+		v.x = i
+		_ = v.x // still a usage
+	}
+
+	// Test an object with multiple fields.
+	o := &T2{x: 10, y: 20}
+	print(o)
+
+	// Test an object of embedded struct/pointer type.
+	t1 := &T1{x: 10}
+	t2 := &T3{y: t1}
+	print(t2)
+}
+
+func (t *T1) GoodPointerReceiverWrite(v *T2) {
+	t.x = 10
+	v.y = 20
+}

go/analysis/passes/unusedwrite/unusedwrite.go

@@ -0,0 +1,184 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package unusedwrite checks for unused writes to the elements of a struct or array object.
+package unusedwrite
+
+import (
+	"fmt"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/ssa"
+)
+
+// Doc is a documentation string.
+const Doc = `checks for unused writes
+
+The analyzer reports instances of writes to struct fields and
+arrays that are never read. Specifically, when a struct object
+or an array is copied, its elements are copied implicitly by
+the compiler, and any element write to this copy does nothing
+with the original object.
+
+For example:
+
+	type T struct { x int }
+	func f(input []T) {
+		for i, v := range input {  // v is a copy
+			v.x = i  // unused write to field x
+		}
+	}
+
+Another example is about non-pointer receiver:
+
+	type T struct { x int }
+	func (t T) f() {  // t is a copy
+		t.x = i  // unused write to field x
+	}
+`
+
+// Analyzer reports instances of writes to struct fields and arrays
+//that are never read.
+var Analyzer = &analysis.Analyzer{
+	Name:     "unusedwrite",
+	Doc:      Doc,
+	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	// Check the writes to struct and array objects.
+	checkStore := func(store *ssa.Store) {
+		// Consider field/index writes to an object whose elements are copied and not shared.
+		// MapUpdate is excluded since only the reference of the map is copied.
+		switch addr := store.Addr.(type) {
+		case *ssa.FieldAddr:
+			if isDeadStore(store, addr.X, addr) {
+				// Report the bug.
+				pass.Reportf(store.Pos(),
+					"unused write to field %s",
+					getFieldName(addr.X.Type(), addr.Field))
+			}
+		case *ssa.IndexAddr:
+			if isDeadStore(store, addr.X, addr) {
+				// Report the bug.
+				pass.Reportf(store.Pos(),
+					"unused write to array index %s", addr.Index)
+			}
+		}
+	}
+
+	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+	for _, fn := range ssainput.SrcFuncs {
+		// Visit each block. No need to visit fn.Recover.
+		for _, blk := range fn.Blocks {
+			for _, instr := range blk.Instrs {
+				// Identify writes.
+				if store, ok := instr.(*ssa.Store); ok {
+					checkStore(store)
+				}
+			}
+		}
+	}
+	return nil, nil
+}
+
+// isDeadStore determines whether a field/index write to an object is dead.
+// Argument "obj" is the object, and "addr" is the instruction fetching the field/index.
+func isDeadStore(store *ssa.Store, obj ssa.Value, addr ssa.Instruction) bool {
+	// Consider only struct or array objects.
+	if !hasStructOrArrayType(obj) {
+		return false
+	}
+	// Check liveness: if the value is used later, then don't report the write.
+	for _, ref := range *obj.Referrers() {
+		if ref == store || ref == addr {
+			continue
+		}
+		switch ins := ref.(type) {
+		case ssa.CallInstruction:
+			return false
+		case *ssa.FieldAddr:
+			// Check whether the same field is used.
+			if ins.X == obj {
+				if faddr, ok := addr.(*ssa.FieldAddr); ok {
+					if faddr.Field == ins.Field {
+						return false
+					}
+				}
+			}
+			// Otherwise another field is used, and this usage doesn't count.
+			continue
+		case *ssa.IndexAddr:
+			if ins.X == obj {
+				return false
+			}
+			continue // Otherwise another object is used
+		case *ssa.Lookup:
+			if ins.X == obj {
+				return false
+			}
+			continue // Otherwise another object is used
+		case *ssa.Store:
+			if ins.Val == obj {
+				return false
+			}
+			continue // Otherwise other object is stored
+		default: // consider live if the object is used in any other instruction
+			return false
+		}
+	}
+	return true
+}
+
+// isStructOrArray returns whether the underlying type is struct or array.
+func isStructOrArray(tp types.Type) bool {
+	if named, ok := tp.(*types.Named); ok {
+		tp = named.Underlying()
+	}
+	switch tp.(type) {
+	case *types.Array:
+		return true
+	case *types.Struct:
+		return true
+	}
+	return false
+}
+
+// hasStructOrArrayType returns whether a value is of struct or array type.
+func hasStructOrArrayType(v ssa.Value) bool {
+	if instr, ok := v.(ssa.Instruction); ok {
+		if alloc, ok := instr.(*ssa.Alloc); ok {
+			// Check the element type of an allocated register (which always has pointer type)
+			// e.g., for
+			//   func (t T) f() { ...}
+			// the receiver object is of type *T:
+			//   t0 = local T (t)   *T
+			if tp, ok := alloc.Type().(*types.Pointer); ok {
+				return isStructOrArray(tp.Elem())
+			}
+			return false
+		}
+	}
+	return isStructOrArray(v.Type())
+}
+
+// getFieldName returns the name of a field in a struct.
+// It the field is not found, then it returns the string format of the index.
+//
+// For example, for struct T {x int, y int), getFieldName(*T, 1) returns "y".
+func getFieldName(tp types.Type, index int) string {
+	if pt, ok := tp.(*types.Pointer); ok {
+		tp = pt.Elem()
+	}
+	if named, ok := tp.(*types.Named); ok {
+		tp = named.Underlying()
+	}
+	if stp, ok := tp.(*types.Struct); ok {
+		return stp.Field(index).Name()
+	}
+	return fmt.Sprintf("%d", index)
+}

go/analysis/passes/unusedwrite/unusedwrite_test.go

@@ -0,0 +1,17 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unusedwrite_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/unusedwrite"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, unusedwrite.Analyzer, "a")
+}