vet: check for useless assignments.

The only check so far is for self-assignments of the form "expr = expr",
but even that found one instance in the standard library.

R=r, adg, mtj, rsc
CC=golang-dev
https://golang.org/cl/7455048

diff --git a/src/cmd/vet/assign.go b/src/cmd/vet/assign.go
new file mode 100644 (file)
index 0000000..a11f0f8
--- /dev/null
+++ b/src/cmd/vet/assign.go
@@ -0,0 +1,44 @@
+// Copyright 2013 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.
+
+/*
+This file contains the code to check for useless assignments.
+*/
+
+package main
+
+import (
+       "go/ast"
+       "go/token"
+       "reflect"
+)
+
+// TODO: should also check for assignments to struct fields inside methods
+// that are on T instead of *T.
+
+// checkAssignStmt checks for assignments of the form "<expr> = <expr>".
+// These are almost always useless, and even when they aren't they are usually a mistake.
+func (f *File) checkAssignStmt(stmt *ast.AssignStmt) {
+       if !vet("assign") {
+               return
+       }
+       if stmt.Tok != token.ASSIGN {
+               return // ignore :=
+       }
+       if len(stmt.Lhs) != len(stmt.Rhs) {
+               // If LHS and RHS have different cardinality, they can't be the same.
+               return
+       }
+       for i, lhs := range stmt.Lhs {
+               rhs := stmt.Rhs[i]
+               if reflect.TypeOf(lhs) != reflect.TypeOf(rhs) {
+                       continue // short-circuit the heavy-weight gofmt check
+               }
+               le := f.gofmt(lhs)
+               re := f.gofmt(rhs)
+               if le == re {
+                       f.Warnf(stmt.Pos(), "self-assignment of %s to %s", re, le)
+               }
+       }
+}

diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go
index b354d8d77de32aca6587c251a367ff1e4ea44644..887cc0642401a1261604e07d59593c676f89e349 100644 (file)
--- a/src/cmd/vet/main.go
+++ b/src/cmd/vet/main.go
@@ -30,6 +30,7 @@ var exitCode = 0
 // a flag is set explicitly.
 var report = map[string]*bool{
        "all":        flag.Bool("all", true, "check everything; disabled if any explicit check is requested"),
+       "assign":     flag.Bool("assign", false, "check for useless assignments"),
        "atomic":     flag.Bool("atomic", false, "check for common mistaken usages of the sync/atomic package"),
        "buildtags":  flag.Bool("buildtags", false, "check that +build tags are valid"),
        "composites": flag.Bool("composites", false, "check that composite literals used type-tagged elements"),
@@ -338,6 +339,7 @@ func (f *File) Visit(node ast.Node) ast.Visitor {
 
 // walkAssignStmt walks an assignment statement
 func (f *File) walkAssignStmt(stmt *ast.AssignStmt) {
+       f.checkAssignStmt(stmt)
        f.checkAtomicAssignment(stmt)
 }
 

diff --git a/src/cmd/vet/test_assign.go b/src/cmd/vet/test_assign.go
new file mode 100644 (file)
index 0000000..8e0f45e
--- /dev/null
+++ b/src/cmd/vet/test_assign.go
@@ -0,0 +1,20 @@
+// Copyright 2013 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.
+
+// This file contains tests for the useless-assignment checker.
+
+// +build vet_test
+
+package main
+
+type ST struct {
+       x int
+}
+
+func (s *ST) SetX(x int) {
+       // Accidental self-assignment; it should be "s.x = x"
+       x = x // ERROR "self-assignment of x to x"
+       // Another mistake
+       s.x = s.x // ERROR "self-assignment of s.x to s.x"
+}