cmd/vet: skip self-assigns with side effects

The existing logic for whether the left and right parts of an assignment
were equal only checked that the gofmt representation of the two was
equal. This only checks that the ASTs were equal.

However, that method is flawed. For example, if either of the
expressions contains a function call, the expressions may actually be
different even if their ASTs are the same. An obvious case is a func
call to math/rand to get a random integer, such as the one added in the
test.

If either of the expressions may have side effects, simply skip the
check. Reuse the logic from bool.go's hasSideEffects.

Fixes #22174.

Change-Id: Ied7f7543dc2bb8852e817230756c6d23bc801d90
Reviewed-on: https://go-review.googlesource.com/69116
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

src/cmd/vet/assign.go

@@ -37,6 +37,9 @@ func checkAssignStmt(f *File, node ast.Node) {
 	}
 	for i, lhs := range stmt.Lhs {
 		rhs := stmt.Rhs[i]
+		if hasSideEffects(lhs) || hasSideEffects(rhs) {
+			continue // expressions may not be equal
+		}
 		if reflect.TypeOf(lhs) != reflect.TypeOf(rhs) {
 			continue // short-circuit the heavy-weight gofmt check
 		}

src/cmd/vet/testdata/assign.go

@@ -6,13 +6,26 @@
 
 package testdata
 
+import "math/rand"
+
 type ST struct {
 	x int
+	l []int
 }
 
-func (s *ST) SetX(x int) {
+func (s *ST) SetX(x int, ch chan 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"
+
+	s.l[0] = s.l[0] // ERROR "self-assignment of s.l.0. to s.l.0."
+
+	// Bail on any potential side effects to avoid false positives
+	s.l[num()] = s.l[num()]
+	rng := rand.New(rand.NewSource(0))
+	s.l[rng.Intn(len(s.l))] = s.l[rng.Intn(len(s.l))]
+	s.l[<-ch] = s.l[<-ch]
 }
+
+func num() int { return 2 }