go.tools/go/types: "declared but not used" error for type switches

Also:
- added more tests
- removed Var.Used accessor: it's not meaningful for clients since
  it does not reflect actual use/def information
- fixed position for short variable declaration errors

R=adonovan
CC=golang-dev
https://golang.org/cl/13240051

go/types/assignments.go

@@ -355,7 +355,7 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
 		scope.Insert(obj)
 	}
 	if n == scope.Len() {
-		check.errorf(vars[0].Pos(), "no new variables on left side of :=")
+		check.errorf(lhs[0].Pos(), "no new variables on left side of :=")
 	}
 }
 

go/types/check.go

@@ -37,6 +37,7 @@ type checker struct {
 	methods     map[string][]*Func     // maps type names to associated methods
 	conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls)
 	untyped     map[ast.Expr]exprInfo  // map of expressions without final type
+	lhsVarsList [][]*Var               // type switch lhs variable sets, for 'declared but not used' errors
 
 	firstErr error // first error encountered
 	Info           // collected type info

go/types/objects.go

@@ -179,7 +179,6 @@ func NewField(pos token.Pos, pkg *Package, name string, typ Type, anonymous bool
 }
 
 func (obj *Var) Anonymous() bool { return obj.anonymous }
-func (obj *Var) Used() bool      { return obj.used }
 func (obj *Var) String() string  { return obj.toString("var", obj.typ) }
 
 // A Func represents a declared function, concrete method, or abstract

go/types/resolver.go

@@ -391,8 +391,26 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	}
 
 	// Phase 5: Check for declared but not used packages and variables.
-
 	// Note: must happen after checking all functions because closures may affect outer scopes
+
+	// Each set of implicitly declared lhs variables of a type switch acts collectively
+	// as a single lhs variable. If any one was 'used', all of them are 'used'. Handle
+	// them before the general analysis.
+	for _, vars := range check.lhsVarsList {
+		// len(vars) > 0
+		var used bool
+		for _, v := range vars {
+			if v.used {
+				used = true
+			}
+			v.used = true // avoid later error
+		}
+		if !used {
+			v := vars[0]
+			check.errorf(v.pos, "%s declared but not used", v.name)
+		}
+	}
+
 	for _, f := range check.funcList {
 		// spec: "Implementation restriction: A compiler may make it illegal to
 		// declare a variable inside a function body if the variable is never used."

go/types/stmt.go

@@ -209,12 +209,12 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 	case *ast.GoStmt:
 		var x operand
 		check.call(&x, s.Call)
-		// TODO(gri) If a builtin is called, the builtin must be valid this context.
+		// TODO(gri) If a builtin is called, the builtin must be valid in this context.
 
 	case *ast.DeferStmt:
 		var x operand
 		check.call(&x, s.Call)
-		// TODO(gri) If a builtin is called, the builtin must be valid this context.
+		// TODO(gri) If a builtin is called, the builtin must be valid in this context.
 
 	case *ast.ReturnStmt:
 		sig := check.funcSig
@@ -371,7 +371,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 				check.invalidAST(s.Pos(), "incorrect form of type switch guard")
 				return
 			}
-			check.recordObject(lhs, nil) // lhs is implicitly declared in each cause clause
+			check.recordObject(lhs, nil) // lhs variable is implicitly declared in each cause clause
 
 			rhs = guard.Rhs[0]
 
@@ -398,6 +398,7 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 		}
 
 		check.multipleDefaults(s.Body.List)
+		var lhsVars []*Var // set of implicitly declared lhs variables
 		for _, s := range s.Body.List {
 			clause, _ := s.(*ast.CaseClause)
 			if clause == nil {
@@ -423,20 +424,25 @@ func (check *checker) stmt(s ast.Stmt, fallthroughOk bool) {
 					T = x.typ
 				}
 				obj := NewVar(lhs.Pos(), check.pkg, lhs.Name, T)
-				// For now we mark all implicitly declared variables as used. If we don't,
-				// we will get an error for each implicitly declared but unused variable,
-				// even if there are others belonging to the same type switch which are used.
-				// The right solution will count any use of an implicit variable in this
-				// switch as a use for all of them, but we cannot make that decision until
-				// we have seen all code, including possibly nested closures.
-				// TODO(gri) Fix this!
-				obj.used = true
+				// For the "declared but not used" error, all lhs variables act as
+				// one; i.e., if any one of them is 'used', all of them are 'used'.
+				// Collect them for later analysis.
+				lhsVars = append(lhsVars, obj)
 				check.declareObj(check.topScope, nil, obj)
 				check.recordImplicit(clause, obj)
 			}
 			check.stmtList(clause.Body, false)
 			check.closeScope()
 		}
+		// If a lhs variable was declared but there were no case clauses, make sure
+		// we have at least one (dummy) 'unused' variable to force an error message.
+		if len(lhsVars) == 0 && lhs != nil {
+			lhsVars = []*Var{NewVar(lhs.Pos(), check.pkg, lhs.Name, x.typ)}
+		}
+		// Record lhs variables for this type switch, if any.
+		if len(lhsVars) > 0 {
+			check.lhsVarsList = append(check.lhsVarsList, lhsVars)
+		}
 
 	case *ast.SelectStmt:
 		check.multipleDefaults(s.Body.List)

go/types/testdata/stmt0.src

@@ -275,7 +275,7 @@ func typeswitches() {
 	default /* ERROR "multiple defaults" */ :
 	}
 
-	switch x := x.(type) {}
+	switch x /* ERROR "declared but not used" */ := x.(type) {}
 
 	switch x := x.(type) {
 	case int:

go/types/testdata/vardecl.src

@@ -94,8 +94,35 @@ func (r T) _(a, b, c int) (u, v, w int) {
 		x /* ERROR "declared but not used" */ := 0
 	}
 
-	// TODO(gri) Add test cases for type switches once they work
-	//           correctly with implicitly declared variables.
+	var t interface{}
+	switch t /* ERROR "declared but not used" */ := t.(type) {}
+
+	switch t /* ERROR "declared but not used" */ := t.(type) {
+	case int:
+	}
+
+	switch t /* ERROR "declared but not used" */ := t.(type) {
+	case int:
+	case float32, complex64:
+		t = nil
+	}
+
+	switch t := t.(type) {
+	case int:
+	case float32, complex64:
+		_ = t
+	}
+
+	switch t := t.(type) {
+	case int:
+	case float32:
+	case string:
+		_ = func() string {
+			return t
+		}
+	}
+
+	switch t := t; t /* ERROR "declared but not used" */ := t.(type) {}
 
 	var z1 /* ERROR "declared but not used" */ int
 	var z2 int