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 <taking@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Trust: Tim King <taking@google.com>
diff --git a/go/analysis/passes/unusedwrite/testdata/src/a/unusedwrite.go b/go/analysis/passes/unusedwrite/testdata/src/a/unusedwrite.go
new file mode 100644
index 0000000..7e43ee4
--- /dev/null
+++ b/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
+}
diff --git a/go/analysis/passes/unusedwrite/unusedwrite.go b/go/analysis/passes/unusedwrite/unusedwrite.go
new file mode 100644
index 0000000..37a0e78
--- /dev/null
+++ b/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)
+}
diff --git a/go/analysis/passes/unusedwrite/unusedwrite_test.go b/go/analysis/passes/unusedwrite/unusedwrite_test.go
new file mode 100644
index 0000000..9658849
--- /dev/null
+++ b/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")
+}
