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 0000000000..7e43ee4369 --- /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 0000000000..37a0e784bc --- /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 0000000000..9658849d0e --- /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") +}