go/types: be more robust in presence of multiple errors

- better documentation of Check
- better handling of (explicit) internal panics
- gotype: don't stop after 1st error

R=adonovan, r
CC=golang-dev
https://golang.org/cl/7406052

diff --git a/src/pkg/exp/gotype/gotype.go b/src/pkg/exp/gotype/gotype.go
index a9042ee05be51046f55e94b4a4e16e821f937b1d..db673f30ee9d0932622ec88e6bb96e07d0122f73 100644 (file)
--- a/src/pkg/exp/gotype/gotype.go
+++ b/src/pkg/exp/gotype/gotype.go
@@ -31,7 +31,7 @@ var (
        printAST      = flag.Bool("ast", false, "print AST")
 )
 
-var exitCode = 0
+var errorCount int
 
 func usage() {
        fmt.Fprintf(os.Stderr, "usage: gotype [flags] [path ...]\n")
@@ -41,7 +41,11 @@ func usage() {
 
 func report(err error) {
        scanner.PrintError(os.Stderr, err)
-       exitCode = 2
+       if list, ok := err.(scanner.ErrorList); ok {
+               errorCount += len(list)
+               return
+       }
+       errorCount++
 }
 
 // parse returns the AST for the Go source src.
@@ -163,10 +167,25 @@ func processFiles(filenames []string, allFiles bool) {
 }
 
 func processPackage(fset *token.FileSet, files []*ast.File) {
-       _, err := types.Check(fset, files)
-       if err != nil {
-               report(err)
+       type bailout struct{}
+       ctxt := types.Context{
+               Error: func(err error) {
+                       if !*allErrors && errorCount >= 10 {
+                               panic(bailout{})
+                       }
+                       report(err)
+               },
        }
+
+       defer func() {
+               switch err := recover().(type) {
+               case nil, bailout:
+               default:
+                       panic(err)
+               }
+       }()
+
+       ctxt.Check(fset, files)
 }
 
 func main() {
@@ -180,5 +199,7 @@ func main() {
                processFiles(flag.Args(), true)
        }
 
-       os.Exit(exitCode)
+       if errorCount > 0 {
+               os.Exit(2)
+       }
 }

diff --git a/src/pkg/exp/gotype/gotype_test.go b/src/pkg/exp/gotype/gotype_test.go
index 03c114013a61b04ee3a6e498cbeec767adb952a7..9e2fad01545d6961a1d85e16951a54c9d4b0994c 100644 (file)
--- a/src/pkg/exp/gotype/gotype_test.go
+++ b/src/pkg/exp/gotype/gotype_test.go
@@ -13,7 +13,7 @@ import (
 )
 
 func runTest(t *testing.T, path string) {
-       exitCode = 0
+       errorCount = 0
 
        *recursive = false
        if suffix := ".go"; strings.HasSuffix(path, suffix) {
@@ -41,8 +41,8 @@ func runTest(t *testing.T, path string) {
                processFiles(files, true)
        }
 
-       if exitCode != 0 {
-               t.Errorf("processing %s failed: exitCode = %d", path, exitCode)
+       if errorCount > 0 {
+               t.Errorf("processing %s failed: %d errors", path, errorCount)
        }
 }
 

diff --git a/src/pkg/go/types/api.go b/src/pkg/go/types/api.go
index b38b9a50d6da317df22726c27f30569ee5c7216c..536f0c6f8daf13cf487cc64d5d4e35f02cb1acce 100644 (file)
--- a/src/pkg/go/types/api.go
+++ b/src/pkg/go/types/api.go
@@ -84,9 +84,12 @@ type Context struct {
 type Importer func(imports map[string]*Package, path string) (pkg *Package, err error)
 
 // Check resolves and typechecks a set of package files within the given
-// context. If there are no errors, Check returns the package, otherwise
-// it returns the first error. If the context's Error handler is nil,
-// Check terminates as soon as the first error is encountered.
+// context. It returns the package and the first error encountered, if
+// any. If the context's Error handler is nil, Check terminates as soon
+// as the first error is encountered; otherwise it continues until the
+// entire package is checked. If there are errors, the package may be
+// only partially type-checked, and the resulting package may be incomplete
+// (missing objects, imports, etc.).
 func (ctxt *Context) Check(fset *token.FileSet, files []*ast.File) (*Package, error) {
        return check(ctxt, fset, files)
 }

diff --git a/src/pkg/go/types/check.go b/src/pkg/go/types/check.go
index 1a0fb04ae23200860261695b38e9b634519ea872..cf8d20de1f3fb962e1886bfde8c5820bbcd523ad 100644 (file)
--- a/src/pkg/go/types/check.go
+++ b/src/pkg/go/types/check.go
@@ -219,9 +219,21 @@ func (check *checker) object(obj Object, cycleOk bool) {
                        obj.Type = Typ[Invalid]
                        return
                }
-               spec := obj.decl.(*ast.ValueSpec)
-               obj.visited = true
-               check.valueSpec(spec.Pos(), obj, spec.Names, spec, 0)
+               switch d := obj.decl.(type) {
+               case *ast.Field:
+                       unreachable() // function parameters are always typed when collected
+               case *ast.ValueSpec:
+                       obj.visited = true
+                       check.valueSpec(d.Pos(), obj, d.Names, d, 0)
+               case *ast.AssignStmt:
+                       // If we reach here, we have a short variable declaration
+                       // where the rhs didn't typecheck and thus the lhs has no
+                       // types.
+                       obj.visited = true
+                       obj.Type = Typ[Invalid]
+               default:
+                       unreachable() // see also function newObj
+               }
 
        case *TypeName:
                if obj.Type != nil {
@@ -412,7 +424,11 @@ func check(ctxt *Context, fset *token.FileSet, files []*ast.File) (pkg *Package,
                        err = check.firsterr
                default:
                        // unexpected panic: don't crash clients
-                       panic(p) // enable for debugging
+                       const debug = true
+                       if debug {
+                               check.dump("INTERNAL PANIC: %v", p)
+                               panic(p)
+                       }
                        // TODO(gri) add a test case for this scenario
                        err = fmt.Errorf("types internal error: %v", p)
                }

diff --git a/src/pkg/go/types/errors.go b/src/pkg/go/types/errors.go
index 6dd32849379c107f6fbd96e818f0162fe85ae749..62ee54791727297999939d0584fc939365190d8f 100644 (file)
--- a/src/pkg/go/types/errors.go
+++ b/src/pkg/go/types/errors.go
@@ -54,7 +54,7 @@ func (check *checker) formatMsg(format string, args []interface{}) string {
        for i, arg := range args {
                switch a := arg.(type) {
                case token.Pos:
-                       args[i] = check.fset.Position(a)
+                       args[i] = check.fset.Position(a).String()
                case ast.Expr:
                        args[i] = exprString(a)
                case Type:

diff --git a/src/pkg/go/types/objects.go b/src/pkg/go/types/objects.go
index c2f46752163744c75e64302a8bc1094372afcda5..02291d34c56ae9d05fec62acb5a2cc7e86abe7c6 100644 (file)
--- a/src/pkg/go/types/objects.go
+++ b/src/pkg/go/types/objects.go
@@ -169,9 +169,11 @@ func newObj(pkg *Package, astObj *ast.Object) Object {
                return &TypeName{Pkg: pkg, Name: name, Type: typ, spec: astObj.Decl.(*ast.TypeSpec)}
        case ast.Var:
                switch astObj.Decl.(type) {
-               case *ast.Field, *ast.ValueSpec, *ast.AssignStmt: // these are ok
+               case *ast.Field: // function parameters
+               case *ast.ValueSpec: // proper variable declarations
+               case *ast.AssignStmt: // short variable declarations
                default:
-                       unreachable()
+                       unreachable() // everything else is not ok
                }
                return &Var{Pkg: pkg, Name: name, Type: typ, decl: astObj.Decl}
        case ast.Fun:

diff --git a/src/pkg/go/types/stmt.go b/src/pkg/go/types/stmt.go
index a8fe61fcf9cd8a2c194016f13f792571c4d0ad55..730b0608eef7c44d864e9c69e7ad04a8ebf05f07 100644 (file)
--- a/src/pkg/go/types/stmt.go
+++ b/src/pkg/go/types/stmt.go
@@ -187,6 +187,9 @@ func (check *checker) assignNtoM(lhs, rhs []ast.Expr, decl bool, iota int) {
                var x operand
                check.expr(&x, rhs[0], nil, iota)
                if x.mode == invalid {
+                       // If decl is set, this leaves the lhs identifiers
+                       // untyped. We catch this when looking up the respective
+                       // object.
                        return
                }