go/types: avoid repeated "declared but not used" errors for closure variables

At the end of type-checking a function or closure, unused local variables
are reported by looking at all variables in the function scope and its
nested children scopes. If a nested scope belonged to a nested function
(closure), that scope would be searched twice, leading to multiple error
messages for unused variables.

This CL introduces an internal-only marker to identify function scopes
so that they can be ignored where needed.

Fixes #22524.

Change-Id: If58cc17b2f0615a16f33ea262f50dffd0e86d0f0
Reviewed-on: https://go-review.googlesource.com/75251
Reviewed-by: Alan Donovan <adonovan@google.com>

src/go/types/expr.go

@@ -1030,6 +1030,15 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
 			// Anonymous functions are considered part of the
 			// init expression/func declaration which contains
 			// them: use existing package-level declaration info.
+			//
+			// TODO(gri) We delay type-checking of regular (top-level)
+			//           function bodies until later. Why don't we do
+			//           it for closures of top-level expressions?
+			//           (We can't easily do it for local closures
+			//           because the surrounding scopes must reflect
+			//           the exact position where the closure appears
+			//           in the source; e.g., variables declared below
+			//           must not be visible).
 			check.funcBody(check.decl, "", sig, e.Body)
 			x.mode = value
 			x.typ = sig

src/go/types/scope.go

@@ -28,12 +28,13 @@ type Scope struct {
 	elems    map[string]Object // lazily allocated
 	pos, end token.Pos         // scope extent; may be invalid
 	comment  string            // for debugging only
+	isFunc   bool              // set if this is a function scope (internal use only)
 }
 
 // NewScope returns a new, empty scope contained in the given parent
 // scope, if any. The comment is for debugging only.
 func NewScope(parent *Scope, pos, end token.Pos, comment string) *Scope {
-	s := &Scope{parent, nil, nil, pos, end, comment}
+	s := &Scope{parent, nil, nil, pos, end, comment, false}
 	// don't add children to Universe scope!
 	if parent != nil && parent != Universe {
 		parent.children = append(parent.children, s)

src/go/types/stmt.go

@@ -72,7 +72,11 @@ func (check *Checker) usage(scope *Scope) {
 	}
 
 	for _, scope := range scope.children {
-		check.usage(scope)
+		// Don't go inside closure scopes a second time;
+		// they are handled explicitly by funcBody.
+		if !scope.isFunc {
+			check.usage(scope)
+		}
 	}
 }
 

src/go/types/testdata/vardecl.src

@@ -151,6 +151,13 @@ func (r T) _(a, b, c int) (u, v, w int) {
 	return
 }
 
+// Unused variables in closures must lead to only one error (issue #22524).
+func _() {
+	_ = func() {
+		var x /* ERROR declared but not used */ int
+	}
+}
+
 // Invalid (unused) expressions must not lead to spurious "declared but not used errors"
 func _() {
 	var a, b, c int

src/go/types/typexpr.go

@@ -143,6 +143,7 @@ func (check *Checker) typ(e ast.Expr) Type {
 // funcType type-checks a function or method type.
 func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast.FuncType) {
 	scope := NewScope(check.scope, token.NoPos, token.NoPos, "function")
+	scope.isFunc = true
 	check.recordScope(ftyp, scope)
 
 	recvList, _ := check.collectParams(scope, recvPar, false)