cmd/vet: avoid internal error for implicitly declared type switch vars

For type switches using a short variable declaration of the form

   switch t := x.(type) {
   case T1:
   ...

go/types doesn't declare the symbolic variable (t in this example)
with the switch; thus such variables are not found in types.Info.Defs.

Instead they are implicitly declared with each type switch case,
and can be found in types.Info.Implicits.

Adjust the shadowing code accordingly.

Added a test case to verify that the issue is fixed, and a test
case verifying that the shadowing code now considers implicitly
declared variables introduces in type switch cases.

While at it, also fixed the (internal) error reporting to provide
more accurate information.

Fixe #26725.

Change-Id: If408ed9e692bf47c640f81de8f46bf5eb43415b0
Reviewed-on: https://go-review.googlesource.com/135117
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go
index c50d4885a078745ffa2bc10b2a7c59c5469507d9..646adf4d76d5facd2c660f63f87d4dd5f23dc643 100644 (file)
--- a/src/cmd/vet/main.go
+++ b/src/cmd/vet/main.go
@@ -467,6 +467,7 @@ type Package struct {
        path      string
        defs      map[*ast.Ident]types.Object
        uses      map[*ast.Ident]types.Object
+       implicits map[ast.Node]types.Object
        selectors map[*ast.SelectorExpr]*types.Selection
        types     map[ast.Expr]types.TypeAndValue
        spans     map[types.Object]Span

diff --git a/src/cmd/vet/shadow.go b/src/cmd/vet/shadow.go
index 29c952fd8858eae7fb8a4d1747d7798e141e8444..47a48834bfcc073ed18bb038a940791e1d52773c 100644 (file)
--- a/src/cmd/vet/shadow.go
+++ b/src/cmd/vet/shadow.go
@@ -86,14 +86,11 @@ func (s Span) contains(pos token.Pos) bool {
        return s.min <= pos && pos < s.max
 }
 
-// growSpan expands the span for the object to contain the instance represented
-// by the identifier.
-func (pkg *Package) growSpan(ident *ast.Ident, obj types.Object) {
+// growSpan expands the span for the object to contain the source range [pos, end).
+func (pkg *Package) growSpan(obj types.Object, pos, end token.Pos) {
        if *strictShadowing {
                return // No need
        }
-       pos := ident.Pos()
-       end := ident.End()
        span, ok := pkg.spans[obj]
        if ok {
                if span.min > pos {
@@ -232,7 +229,7 @@ func checkShadowing(f *File, ident *ast.Ident) {
                // the shadowing identifier.
                span, ok := f.pkg.spans[shadowed]
                if !ok {
-                       f.Badf(ident.Pos(), "internal error: no range for %q", ident.Name)
+                       f.Badf(shadowed.Pos(), "internal error: no range for %q", shadowed.Name())
                        return
                }
                if !span.contains(ident.Pos()) {

diff --git a/src/cmd/vet/testdata/shadow.go b/src/cmd/vet/testdata/shadow.go
index c55cb2772a9c67d9698094a0c0d9a8ac2a94ba03..d10fde2b811d0378523618410f1b84f2d8934ad8 100644 (file)
--- a/src/cmd/vet/testdata/shadow.go
+++ b/src/cmd/vet/testdata/shadow.go
@@ -57,3 +57,35 @@ func ShadowRead(f *os.File, buf []byte) (err error) {
 func one() int {
        return 1
 }
+
+// Must not complain with an internal error for the
+// implicitly declared type switch variable v.
+func issue26725(x interface{}) int {
+       switch v := x.(type) {
+       case int, int32:
+               if v, ok := x.(int); ok {
+                       return v
+               }
+       case int64:
+               return int(v)
+       }
+       return 0
+}
+
+// Verify that implicitly declared variables from
+// type switches are considered in shadowing analysis.
+func shadowTypeSwitch(a interface{}) {
+       switch t := a.(type) {
+       case int:
+               {
+                       t := 0 // ERROR "declaration of .t. shadows declaration at shadow.go:78"
+                       _ = t
+               }
+               _ = t
+       case uint:
+               {
+                       t := uint(0) // OK because t is not mentioned later in this function
+                       _ = t
+               }
+       }
+}

diff --git a/src/cmd/vet/types.go b/src/cmd/vet/types.go
index 5f8e481e01b68115f603557e3cfc4bd82ed567a5..3ff4b5966de2174b51a95717d5f256df93e5804d 100644 (file)
--- a/src/cmd/vet/types.go
+++ b/src/cmd/vet/types.go
@@ -73,6 +73,7 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) []error {
        }
        pkg.defs = make(map[*ast.Ident]types.Object)
        pkg.uses = make(map[*ast.Ident]types.Object)
+       pkg.implicits = make(map[ast.Node]types.Object)
        pkg.selectors = make(map[*ast.SelectorExpr]*types.Selection)
        pkg.spans = make(map[types.Object]Span)
        pkg.types = make(map[ast.Expr]types.TypeAndValue)
@@ -95,6 +96,7 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) []error {
                Types:      pkg.types,
                Defs:       pkg.defs,
                Uses:       pkg.uses,
+               Implicits:  pkg.implicits,
        }
        typesPkg, err := config.Check(pkg.path, fs, astFiles, info)
        if len(allErrors) == 0 && err != nil {
@@ -103,10 +105,28 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) []error {
        pkg.typesPkg = typesPkg
        // update spans
        for id, obj := range pkg.defs {
-               pkg.growSpan(id, obj)
+               // Ignore identifiers that don't denote objects
+               // (package names, symbolic variables such as t
+               // in t := x.(type) of type switch headers).
+               if obj != nil {
+                       pkg.growSpan(obj, id.Pos(), id.End())
+               }
        }
        for id, obj := range pkg.uses {
-               pkg.growSpan(id, obj)
+               pkg.growSpan(obj, id.Pos(), id.End())
+       }
+       for node, obj := range pkg.implicits {
+               // A type switch with a short variable declaration
+               // such as t := x.(type) doesn't declare the symbolic
+               // variable (t in the example) at the switch header;
+               // instead a new variable t (with specific type) is
+               // declared implicitly for each case. Such variables
+               // are found in the types.Info.Implicits (not Defs)
+               // map. Add them here, assuming they are declared at
+               // the type cases' colon ":".
+               if cc, ok := node.(*ast.CaseClause); ok {
+                       pkg.growSpan(obj, cc.Colon, cc.Colon)
+               }
        }
        return allErrors
 }