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

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)
+	}
 }

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)
 	}
 }
 

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)
 }

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)
 		}

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:

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:

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
 		}