From: David Symonds <dsymonds@golang.org>
Date: Tue, 5 Mar 2013 22:55:04 +0000 (+1100)
Subject: vet: check for useless assignments.
X-Git-Tag: go1.1rc2~680
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a22361d68d6d93ab2b06e4b608e19796e033218b;p=gostls13.git

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
index 0000000000..a11f0f875f
--- /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 b354d8d77d..887cc06424 100644
--- 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
index 0000000000..8e0f45e532
--- /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"
+}