From e785f050b606d624af4c787b5961c8e02dc2a3fb Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Fri, 15 Nov 2013 14:26:29 -0800
Subject: [PATCH] go.tools/go/types: add missing checks for short variable
 declarations

Fixes golang/go#6766.

R=adonovan
CC=golang-dev
https://golang.org/cl/24330044
---
 go/types/assignments.go     | 24 +++++++++++++-----------
 go/types/stmt.go            | 34 +++++++++++++++++++++-------------
 go/types/testdata/stmt0.src | 20 +++++++++++++++++++-
 3 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/go/types/assignments.go b/go/types/assignments.go
index 88544f9e48..c51fb360fc 100644
--- a/go/types/assignments.go
+++ b/go/types/assignments.go
@@ -347,11 +347,12 @@ func (check *checker) assignVars(lhs, rhs []ast.Expr) {
 	check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r)
 }
 
-func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
+func (check *checker) shortVarDecl(pos token.Pos, lhs, rhs []ast.Expr) {
 	scope := check.topScope
 
 	// collect lhs variables
-	vars := make([]*Var, len(lhs))
+	var newVars []*Var
+	var lhsVars = make([]*Var, len(lhs))
 	for i, lhs := range lhs {
 		var obj *Var
 		if ident, _ := lhs.(*ast.Ident); ident != nil {
@@ -368,6 +369,7 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
 			} else {
 				// declare new variable
 				obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
+				newVars = append(newVars, obj)
 			}
 			if obj != nil {
 				check.recordObject(ident, obj)
@@ -378,18 +380,18 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
 		if obj == nil {
 			obj = NewVar(lhs.Pos(), check.pkg, "_", nil) // dummy variable
 		}
-		vars[i] = obj
+		lhsVars[i] = obj
 	}
 
-	check.initVars(vars, rhs, token.NoPos)
+	check.initVars(lhsVars, rhs, token.NoPos)
 
-	// declare variables
-	n := scope.Len()
-	for _, obj := range vars {
-		scope.Insert(obj)
-	}
-	if n == scope.Len() {
-		check.errorf(lhs[0].Pos(), "no new variables on left side of :=")
+	// declare new variables
+	if len(newVars) > 0 {
+		for _, obj := range newVars {
+			check.declare(scope, nil, obj) // recordObject already called
+		}
+	} else {
+		check.errorf(pos, "no new variables on left side of :=")
 	}
 }
 
diff --git a/go/types/stmt.go b/go/types/stmt.go
index 060b67a3a9..05c8d27197 100644
--- a/go/types/stmt.go
+++ b/go/types/stmt.go
@@ -235,7 +235,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 				return
 			}
 			if s.Tok == token.DEFINE {
-				check.shortVarDecl(s.Lhs, s.Rhs)
+				check.shortVarDecl(s.TokPos, s.Lhs, s.Rhs)
 			} else {
 				// regular assignment
 				check.assignVars(s.Lhs, s.Rhs)
@@ -570,8 +570,9 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 		rhs := [2]Type{key, val}
 
 		if decl {
-			// declaration; variable scope starts after the range clause
-			var idents []*ast.Ident
+			// short variable declaration; variable scope starts after the range clause
+			// (the for loop opens a new scope, so variables on the lhs never redeclare
+			// previously declared variables)
 			var vars []*Var
 			for i, lhs := range lhs {
 				if lhs == nil {
@@ -579,17 +580,20 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 				}
 
 				// determine lhs variable
-				name := "_" // dummy, in case lhs is not an identifier
-				ident, _ := lhs.(*ast.Ident)
-				if ident != nil {
-					name = ident.Name
+				var obj *Var
+				if ident, _ := lhs.(*ast.Ident); ident != nil {
+					// declare new variable
+					name := ident.Name
+					obj = NewVar(ident.Pos(), check.pkg, name, nil)
+					check.recordObject(ident, obj)
+					// _ variables don't count as new variables
+					if name != "_" {
+						vars = append(vars, obj)
+					}
 				} else {
 					check.errorf(lhs.Pos(), "cannot declare %s", lhs)
+					obj = NewVar(lhs.Pos(), check.pkg, "_", nil) // dummy variable
 				}
-				idents = append(idents, ident)
-
-				obj := NewVar(lhs.Pos(), check.pkg, name, nil)
-				vars = append(vars, obj)
 
 				// initialize lhs variable
 				x.mode = value
@@ -599,8 +603,12 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 			}
 
 			// declare variables
-			for i, ident := range idents {
-				check.declare(check.topScope, ident, vars[i])
+			if len(vars) > 0 {
+				for _, obj := range vars {
+					check.declare(check.topScope, nil, obj) // recordObject already called
+				}
+			} else {
+				check.errorf(s.TokPos, "no new variables on left side of :=")
 			}
 		} else {
 			// ordinary assignment
diff --git a/go/types/testdata/stmt0.src b/go/types/testdata/stmt0.src
index ba8d325b56..deae9d9461 100644
--- a/go/types/testdata/stmt0.src
+++ b/go/types/testdata/stmt0.src
@@ -137,7 +137,17 @@ func issue6487() {
 	_ = &m /* ERROR "cannot take address" */ ["foo"].x
 }
 
-func shortVarDecls() {
+func issue6766a() {
+	a, a /* ERROR redeclared */ := 1, 2
+	_ = a
+	a, b, b /* ERROR redeclared */ := 1, 2, 3
+	_ = b
+	c, c /* ERROR redeclared */, b := 1, 2, 3
+	_ = c
+	a, b := /* ERROR no new variables */ 1, 2
+}
+
+func shortVarDecls1() {
 	const c = 0
 	type d int
 	a, b, c /* ERROR "cannot assign" */ , d /* ERROR "cannot assign" */  := 1, "zwei", 3.0, 4
@@ -684,6 +694,14 @@ func rangeloops2() {
 	for _, r /* ERROR cannot assign */ = range "foo" {}
 }
 
+func issue6766b() {
+	for _ := /* ERROR no new variables */ range "" {}
+	for a, a /* ERROR redeclared */ := range "" { _ = a }
+	var a int
+	_ = a
+	for a, a /* ERROR redeclared */ := range []int{1, 2, 3} { _ = a }
+}
+
 func labels0() {
 	goto L0
 	goto L1