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>

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index 01c97afc07898509debaabee3a88ca8561e578ef..ea778fd18878739d105b017f6234d89f2b0ec6aa 100644 (file)
--- a/src/go/types/expr.go
+++ b/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

diff --git a/src/go/types/scope.go b/src/go/types/scope.go
index b5d34d6e655af9ab06a96b48790119088830aa44..39e42d758add8eed1e840b3b436c3d285524a210 100644 (file)
--- a/src/go/types/scope.go
+++ b/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)

diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go
index 618d1e5fbf2249ea9c5396f7c2c4747875363e7e..1292f5cec183fa4ca71de941095c8f8c0f7f13ea 100644 (file)
--- a/src/go/types/stmt.go
+++ b/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)
+               }
        }
 }
 

diff --git a/src/go/types/testdata/vardecl.src b/src/go/types/testdata/vardecl.src
index 35f44e6c48b3aff4cda1e0dc32ac5f10d3fee939..197dec2d5dc6900270b1820b05cd3ec472e5bc1e 100644 (file)
--- a/src/go/types/testdata/vardecl.src
+++ b/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

diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index 5f1587bf0f54c5f843c6fadc9898fec445929821..2272ac06450581ac849632c28cfc3e29181d655e 100644 (file)
--- a/src/go/types/typexpr.go
+++ b/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)