go.tools/go/types: 'declared but not used' checks for variables

- updated all tests to conform to stricter rules
- TODO: check for implicitly declared variables in type switches

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

go/types/api_test.go

@@ -132,7 +132,7 @@ func TestScopesInfo(t *testing.T) {
 		{`package p; func _(t interface{}) { switch t := t; t.(type) {} }`, []string{
 			"file:", "func:t", "type switch:t",
 		}},
-		{`package p; func _(t interface{}) { switch x := t.(type) { case int: } }`, []string{
+		{`package p; func _(t interface{}) { switch x := t.(type) { case int: _ = x } }`, []string{
 			"file:", "func:t", "type switch:", "case:x", // x implicitly declared
 		}},
 		{`package p; func _() { select{} }`, []string{

go/types/assignments.go

@@ -111,8 +111,11 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type {
 		return nil
 	}
 
-	// Don't evaluate lhs if it is the (possibly parenthesized) blank identifier.
-	if ident, _ := unparen(lhs).(*ast.Ident); ident != nil && ident.Name == "_" {
+	// Determine if the lhs is a (possibly parenthesized) identifier.
+	ident, _ := unparen(lhs).(*ast.Ident)
+
+	// Don't evaluate lhs if it is the blank identifier.
+	if ident != nil && ident.Name == "_" {
 		check.recordObject(ident, nil)
 		// If the lhs is untyped, determine the default type.
 		// The spec is unclear about this, but gc appears to
@@ -131,8 +134,26 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type {
 		return typ
 	}
 
+	// If the lhs is an identifier denoting a variable v, this assignment
+	// is not a 'use' of v. Remember current value of v.used and restore
+	// after evaluating the lhs via check.expr.
+	var v *Var
+	var v_used bool
+	if ident != nil {
+		if obj := check.topScope.LookupParent(ident.Name); obj != nil {
+			v, _ = obj.(*Var)
+			if v != nil {
+				v_used = v.used
+			}
+		}
+	}
+
 	var z operand
 	check.expr(&z, lhs)
+	if v != nil {
+		v.used = v_used // restore v.used
+	}
+
 	if z.mode == invalid || z.typ == Typ[Invalid] {
 		return nil
 	}

go/types/gcimporter.go

@@ -490,7 +490,7 @@ func (p *gcParser) parseField() (*Var, string) {
 	if p.tok == scanner.String {
 		tag = p.expect(scanner.String)
 	}
-	return NewFieldVar(token.NoPos, pkg, name, typ, anonymous), tag
+	return NewField(token.NoPos, pkg, name, typ, anonymous), tag
 }
 
 // StructType = "struct" "{" [ FieldList ] "}" .

go/types/objects.go

@@ -13,8 +13,7 @@ import (
 	"code.google.com/p/go.tools/go/exact"
 )
 
-// TODO(gri) All objects looks very similar now. Maybe just have single Object struct with an object kind?
-// TODO(gri) Document factory, accessor methods, and fields.
+// TODO(gri) Document factory, accessor methods, and fields. General clean-up.
 
 // An Object describes a named language entity such as a package,
 // constant, type, variable, function (incl. methods), or label.
@@ -162,19 +161,25 @@ func (obj *TypeName) String() string { return obj.toString("type", obj.typ.Under
 type Var struct {
 	object
 
-	anonymous bool // if set, this variable is an anonymous struct field, and name is the type name
+	anonymous bool // if set, the variable is an anonymous struct field, and name is the type name
 	visited   bool // for initialization cycle detection
+	used      bool // if set, the variable was 'used'
 }
 
 func NewVar(pos token.Pos, pkg *Package, name string, typ Type) *Var {
-	return &Var{object{nil, pos, pkg, name, typ}, false, false}
+	return &Var{object: object{nil, pos, pkg, name, typ}}
 }
 
-func NewFieldVar(pos token.Pos, pkg *Package, name string, typ Type, anonymous bool) *Var {
-	return &Var{object{nil, pos, pkg, name, typ}, anonymous, false}
+func NewParam(pos token.Pos, pkg *Package, name string, typ Type) *Var {
+	return &Var{object: object{nil, pos, pkg, name, typ}, used: true} // parameters are always 'used'
+}
+
+func NewField(pos token.Pos, pkg *Package, name string, typ Type, anonymous bool) *Var {
+	return &Var{object: object{nil, pos, pkg, name, typ}, anonymous: anonymous}
 }
 
 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

@@ -381,13 +381,34 @@ func (check *checker) resolveFiles(files []*ast.File) {
 			}
 			fmt.Println("---", s)
 		}
-		check.topScope = f.sig.scope // open the function scope
+
+		check.topScope = f.sig.scope // open function scope
 		check.funcSig = f.sig
 		check.stmtList(f.body.List, false)
 		if f.sig.results.Len() > 0 && !check.isTerminating(f.body, "") {
 			check.errorf(f.body.Rbrace, "missing return")
 		}
 	}
+
+	// Phase 5: Check for declared but not used packages and variables.
+
+	// Note: must happen after checking all functions because closures may affect outer scopes
+	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."
+		check.usage(f.sig.scope)
+	}
+}
+
+func (check *checker) usage(scope *Scope) {
+	for _, obj := range scope.elems {
+		if v, _ := obj.(*Var); v != nil && !v.used {
+			check.errorf(v.pos, "%s declared but not used", v.name)
+		}
+	}
+	for _, scope := range scope.children {
+		check.usage(scope)
+	}
 }
 
 // objDecl type-checks the declaration of obj in its respective file scope.

go/types/resolver_test.go

@@ -46,10 +46,11 @@ var sources = []string{
 	func (T) _() {}
 	var i I
 	var _ = i.m
-	func _(s []int) { for i, x := range s {} }
+	func _(s []int) { for i, x := range s { _, _ = i, x } }
 	func _(x interface{}) {
 		switch x := x.(type) {
 		case int:
+			_ = x
 		}
 	}
 	`,

go/types/stmt.go

@@ -423,6 +423,14 @@ 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
 				check.declareObj(check.topScope, nil, obj)
 				check.recordImplicit(clause, obj)
 			}

go/types/testdata/builtins.src

@@ -11,27 +11,26 @@ import "unsafe"
 func _append() {
 	var x int
 	var s []byte
-	_0 := append /* ERROR "argument" */ ()
-	_1 := append("foo" /* ERROR "not a typed slice" */)
-	_2 := append(nil /* ERROR "not a typed slice" */, s)
-	_3 := append(x /* ERROR "not a typed slice" */, s)
-	_4 := append(s)
+	_ = append /* ERROR "argument" */ ()
+	_ = append("foo" /* ERROR "not a typed slice" */)
+	_ = append(nil /* ERROR "not a typed slice" */, s)
+	_ = append(x /* ERROR "not a typed slice" */, s)
+	_ = append(s)
 	append /* ERROR "not used" */ (s)
 }
 
 func _cap() {
 	var a [10]bool
 	var p *[20]int
-	var s []int
 	var c chan string
-	_0 := cap /* ERROR "argument" */ ()
-	_1 := cap /* ERROR "argument" */ (1, 2)
-	_2 := cap(42 /* ERROR "invalid" */)
+	_ = cap /* ERROR "argument" */ ()
+	_ = cap /* ERROR "argument" */ (1, 2)
+	_ = cap(42 /* ERROR "invalid" */)
 	const _3 = cap(a)
 	assert(_3 == 10)
 	const _4 = cap(p)
 	assert(_4 == 20)
-	_5 := cap(c)
+	_ = cap(c)
 	cap /* ERROR "not used" */ (c)
 
 	// issue 4744
@@ -54,7 +53,6 @@ func _complex() {
 	var f32 float32
 	var f64 float64
 	var c64 complex64
-	var c128 complex128
 	_ = complex /* ERROR "argument" */ ()
 	_ = complex /* ERROR "argument" */ (1)
 	_ = complex(true /* ERROR "invalid argument" */ , 0)
@@ -116,6 +114,7 @@ func _copy() {
 	n1 := copy(s, a[0:])            // n1 == 6, s == []int{0, 1, 2, 3, 4, 5}
 	n2 := copy(s, s[2:])            // n2 == 4, s == []int{2, 3, 4, 5, 4, 5}
 	n3 := copy(b, "Hello, World!")  // n3 == 5, b == []byte("Hello")
+	_, _, _ = n1, n2, n3
 }
 
 func _delete() {
@@ -149,13 +148,13 @@ func _imag() {
 	f32 = imag /* ERROR "cannot assign" */ (c128)
 	f64 = imag /* ERROR "cannot assign" */ (c64)
 	imag /* ERROR "not used" */ (c64)
+	_, _ = f32, f64
 }
 
 func _len() {
 	const c = "foobar"
 	var a [10]bool
 	var p *[20]int
-	var s []int
 	var m map[string]complex128
 	_ = len /* ERROR "argument" */ ()
 	_ = len /* ERROR "argument" */ (1, 2)
@@ -292,6 +291,7 @@ func _real() {
 	f32 = real /* ERROR "cannot assign" */ (c128)
 	f64 = real /* ERROR "cannot assign" */ (c64)
 	real /* ERROR "not used" */ (c64)
+	_, _ = f32, f64
 }
 
 func _recover() {

go/types/testdata/const0.src

@@ -270,4 +270,5 @@ func _() {
 	iota := 123
 	const x = iota /* ERROR "is not constant" */
 	var y = iota
+	_ = y
 }

go/types/testdata/decls0.src

@@ -61,7 +61,7 @@ func init /* ERROR "missing function body" */ ()
 
 func _() { const init = 0 }
 func _() { type init int }
-func _() { var init int }
+func _() { var init int; _ = init }
 
 // invalid array types
 type (
@@ -231,4 +231,5 @@ func _() {
 	var i BlankI
 	var x BlankT
 	i = x /* ERROR "cannot assign" */
+	_ = i
 }

go/types/testdata/decls1.src

@@ -110,6 +110,10 @@ func _() {
 		m2a, m2b, m2c /* ERROR "missing init expr for m2c" */ = 1, 2
 		m3a, m3b = 1, 2, 3 /* ERROR "extra init expr 3" */
 	)
+
+	_, _ = m1a, m1b
+	_, _, _ = m2a, m2b, m2c
+	_, _ = m3a, m3b
 }
 
 // Declaration of parameters and results

go/types/testdata/expr2.src

@@ -14,6 +14,7 @@ func _bool() {
 	var b bool
 	var x, y float32
 	b = x < y
+	_ = b
 	_ = struct{b bool}{x < y}
 }
 

go/types/testdata/expr3.src

@@ -17,10 +17,14 @@ func indexes() {
 	_ = a[- /* ERROR "negative" */ 1]
 	_ = a[- /* ERROR "negative" */ 1 :]
 	_ = a[: - /* ERROR "negative" */ 1]
+
 	var a0 int
 	a0 = a[0]
+	_ = a0
 	var a1 int32
 	a1 = a /* ERROR "cannot assign" */ [1] 
+	_ = a1
+
 	_ = a[9]
 	_ = a[10 /* ERROR "index .* out of bounds" */ ]
 	_ = a[1 /* ERROR "overflows" */ <<100]
@@ -67,8 +71,10 @@ func indexes() {
 	_ = t[: - /* ERROR "negative" */ 1]
 	var t0 byte
 	t0 = t[0]
+	_ = t0
 	var t1 rune
 	t1 = t /* ERROR "cannot assign" */ [2]
+	_ = t1
 	_ = ("foo" + "bar")[5]
 	_ = ("foo" + "bar")[6 /* ERROR "index .* out of bounds" */ ]
 
@@ -78,10 +84,12 @@ func indexes() {
 	_ = c[: - /* ERROR "negative" */ 1]
 	var c0 byte
 	c0 = c[0]
+	_ = c0
 	var c2 float32
 	c2 = c /* ERROR "cannot assign" */ [2]
 	_ = c[3 /* ERROR "index .* out of bounds" */ ]
 	_ = ""[0 /* ERROR "index .* out of bounds" */ ]
+	_ = c2
 
 	_ = s[1<<30] // no compile-time error here
 
@@ -94,6 +102,7 @@ func indexes() {
 	ss = "foo"[i:j]
 	ms = "foo" /* ERROR "cannot assign" */ [1:2]
 	ms = "foo" /* ERROR "cannot assign" */ [i:j]
+	_, _ = ss, ms
 }
 
 type T struct {
@@ -111,6 +120,7 @@ func method_expressions() {
 
 	var f func(*T) = T /* ERROR "not in method set" */ .m
 	var g func(*T) = (*T).m
+	_, _ = f, g
 
 	_ = T /* ERROR "has no method" */ .y
 	_ = ( /* ERROR "has no method" */ *T).y
@@ -195,8 +205,10 @@ func array_literals() {
 	var a14 [4]int
 	a13 = a1
 	a14 = a1 /* ERROR "cannot assign" */
+	_, _ = a13, a14
 	
 	a2 := [...]int{- /* ERROR "negative" */ 1: 0}
+	_ = a2
 
 	a3 := [...]int{0, 1, 2, 0 /* ERROR "duplicate index" */ : 0, 3: 3, 4}
 	assert(len(a3) == 5) // somewhat arbitrary
@@ -295,6 +307,7 @@ func type_asserts() {
 	var e interface{}
 	var ok bool
 	x, ok = e.(int)
+	_ = ok
 
 	var t I
 	_ = t /* ERROR "use of .* outside type switch" */ .(type)

go/types/testdata/shifts.src

@@ -65,6 +65,7 @@ func shifts2() {
 		v float32 = 1 /* ERROR "must be integer" */ <<s   // illegal: 1 has type float32, cannot shift
 		w int64 = 1.0<<33  // 1.0<<33 is a constant shift expression
 	)
+	_, _, _, _, _, _, _, _, _, _, _, _ = i, j, k, m, n, o, p, u, u1, u2, v, w
 }
 
 func shifts3(a int16, b float32) {
@@ -76,6 +77,7 @@ func shifts3(a int16, b float32) {
 	)
 	x := 1.0 /* ERROR "must be integer" */ <<s + 1
 	shifts3(1.0 << s, 1 /* ERROR "must be integer" */ >> s)
+	_, _, _ = u, v, x
 }
 
 func shifts4() {
@@ -176,7 +178,6 @@ func shifts6() {
 	_ = append(b, 1.0<<s)
 	_ = append(b, 1.1 /* ERROR "must be integer" */ <<s)
 
-	var c []float32
 	_ = append(b, 1<<s)
 	_ = append(b, 1.0<<s) // should fail - see TODO in append code
 	_ = append(b, 1.1 /* ERROR "must be integer" */ <<s)
@@ -195,6 +196,8 @@ func shifts7() {
 	// shifts of shifts
 	var s uint
 	var x int
+	_ = x
+
 	_ = 1<<(1<<s)
 	_ = 1<<(1.<<s)
 	_ = 1. /* ERROR "integer" */ <<(1<<s)

go/types/testdata/stmt0.src

@@ -17,6 +17,7 @@ func assignments0() (int, int) {
 	a, b, c = 1, 2, 3
 	a, b, c = 1 /* ERROR "assignment count mismatch" */ , 2
 	a, b, c = 1 /* ERROR "assignment count mismatch" */ , 2, 3, 4
+	_, _, _ = a, b, c
 
 	a = f0 /* ERROR "used as value" */ ()
 	a = f1()
@@ -43,6 +44,7 @@ func assignments1() {
 	s = b /* ERROR "cannot assign" */
 
 	v0, v1, v2 := 1 /* ERROR "mismatch" */ , 2, 3, 4
+	_, _, _ = v0, v1, v2
 
 	b = true
 
@@ -136,6 +138,7 @@ func selects() {
 		ch <- x
 	case t, ok := <-ch:
 		x = t
+		_ = ok
 	case <-sc /* ERROR "cannot receive from send-only channel" */ :
 	}
 	select {
@@ -277,6 +280,7 @@ func typeswitches() {
 	switch x := x.(type) {
 	case int:
 		var y int = x
+		_ = y
 	}
 
 	switch x := i /* ERROR "not an interface" */ .(type) {}
@@ -284,12 +288,16 @@ func typeswitches() {
 	switch t := x.(type) {
 	case nil:
 		var v bool = t /* ERROR "cannot initialize" */
+		_ = v
 	case int:
 		var v int = t
+		_ = v
 	case float32, complex64:
 		var v float32 = t /* ERROR "cannot initialize" */
+		_ = v
 	default:
 		var v float32 = t /* ERROR "cannot initialize" */
+		_ = v
 	}
 
 	var t I
@@ -359,42 +367,53 @@ func rangeloops() {
 	for i := range a {
 		var ii int
 		ii = i
+		_ = ii
 	}
 	for i, x := range a {
 		var ii int
 		ii = i
+		_ = ii
 		var xx float64
 		xx = x /* ERROR "cannot assign" */
+		_ = xx
 	}
 	var ii int
 	var xx float32
-	for ii, xx := range a {}
+	for ii, xx = range a {}
+	_, _ = ii, xx
 
 	for i := range b {
 		var ii int
 		ii = i
+		_ = ii
 	}
 	for i, x := range b {
 		var ii int
 		ii = i
+		_ = ii
 		var xx string
 		xx = x
+		_ = xx
 	}
 
 	for i := range s {
 		var ii int
 		ii = i
+		_ = ii
 	}
 	for i, x := range s {
 		var ii int
 		ii = i
+		_ = ii
 		var xx rune
 		xx = x
+		_ = xx
 	}
 
 	for _, x := range p {
 		var xx complex128
 		xx = x
+		_ = xx
 	}
 
 	for _, x := range pp /* ERROR "cannot range over" */ {}
@@ -402,10 +421,12 @@ func rangeloops() {
 	for k := range m {
 		var kk int32
 		kk = k /* ERROR "cannot assign" */
+		_ = kk
 	}
 	for k, v := range m {
 		var kk int
 		kk = k
+		_ = kk
 		if v {}
 	}
 
@@ -413,18 +434,21 @@ func rangeloops() {
 	for e := range c {
 		var ee int
 		ee = e
+		_ = ee
 	}
 	for _ = range sc /* ERROR "cannot range over send-only channel" */ {}
 	for _ = range rc {}
 
 	// constant strings
 	const cs = "foo"
-	for i, x := range cs {}
+	for i, x := range cs { _, _ = i, x }
 	for i, x := range "" {
 		var ii int
 		ii = i
+		_ = ii
 		var xx rune
 		xx = x
+		_ = xx
 	}
 }
 

go/types/testdata/vardecl.src

@@ -60,4 +60,64 @@ var (
 	_, _, _ /* ERROR "missing init expr for _" */ int = 1, 2
 )
 
+// Variables declared in function bodies must be 'used'.
+type T struct{}
+func (r T) _(a, b, c int) (u, v, w int) {
+	var x1 /* ERROR "declared but not used" */ int
+	var x2 /* ERROR "declared but not used" */ int
+	x1 = 1
+	(x2) = 2
+
+	y1 /* ERROR "declared but not used" */ := 1
+	y2 /* ERROR "declared but not used" */ := 2
+	y1 = 1
+	(y1) = 2
+
+	{
+		var x1 /* ERROR "declared but not used" */ int
+		var x2 /* ERROR "declared but not used" */ int
+		x1 = 1
+		(x2) = 2
+
+		y1 /* ERROR "declared but not used" */ := 1
+		y2 /* ERROR "declared but not used" */ := 2
+		y1 = 1
+		(y1) = 2
+	}
+
+	if x /* ERROR "declared but not used" */ := 0; a < b {}
+
+	switch x /* ERROR "declared but not used" */, y := 0, 1; a {
+	case 0:
+		_ = y
+	case 1:
+		x /* ERROR "declared but not used" */ := 0
+	}
+
+	// TODO(gri) Add test cases for type switches once they work
+	//           correctly with implicitly declared variables.
+
+	var z1 /* ERROR "declared but not used" */ int
+	var z2 int
+	_ = func(a, b, c int) (u, v, w int) {
+		z1 = a
+		(z1) = b
+		a = z2
+		return
+	}
+
+	var s []int
+	var i /* ERROR "declared but not used" */ , j int
+	for i, j = range s {
+		_ = j
+	}
+
+	for i, j /* ERROR "declared but not used" */ := range s {
+		_ = func() int {
+			return i
+		}
+	}
+	return
+}
+
 // TODO(gri) consolidate other var decl checks in this file

go/types/typexpr.go

@@ -78,6 +78,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool)
 		}
 
 	case *Var:
+		obj.used = true
 		x.mode = variable
 
 	case *Func:
@@ -372,13 +373,13 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 		if len(field.Names) > 0 {
 			// named parameter
 			for _, name := range field.Names {
-				par := NewVar(name.Pos(), check.pkg, name.Name, typ)
+				par := NewParam(name.Pos(), check.pkg, name.Name, typ)
 				check.declareObj(scope, name, par)
 				params = append(params, par)
 			}
 		} else {
 			// anonymous parameter
-			par := NewVar(ftype.Pos(), check.pkg, "", typ)
+			par := NewParam(ftype.Pos(), check.pkg, "", typ)
 			check.recordImplicit(field, par)
 			params = append(params, par)
 		}
@@ -474,7 +475,7 @@ func (check *checker) collectFields(list *ast.FieldList, cycleOk bool) (fields [
 			tags = append(tags, tag)
 		}
 
-		fld := NewFieldVar(pos, check.pkg, name, typ, anonymous)
+		fld := NewField(pos, check.pkg, name, typ, anonymous)
 		check.declareFld(&fset, ident, fld)
 		fields = append(fields, fld)
 	}