From 60066754fd6d080e6f0b08d88369beea4b54b801 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 26 Feb 2013 14:33:24 -0800 Subject: [PATCH] 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 | 33 +++++++++++++++++++++++++------ src/pkg/exp/gotype/gotype_test.go | 6 +++--- src/pkg/go/types/api.go | 9 ++++++--- src/pkg/go/types/check.go | 24 ++++++++++++++++++---- src/pkg/go/types/errors.go | 2 +- src/pkg/go/types/objects.go | 6 ++++-- src/pkg/go/types/stmt.go | 3 +++ 7 files changed, 64 insertions(+), 19 deletions(-) diff --git a/src/pkg/exp/gotype/gotype.go b/src/pkg/exp/gotype/gotype.go index a9042ee05be..db673f30ee9 100644 --- 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 03c114013a6..9e2fad01545 100644 --- 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 b38b9a50d6d..536f0c6f8da 100644 --- 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 1a0fb04ae23..cf8d20de1f3 100644 --- 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 6dd32849379..62ee5479172 100644 --- 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 c2f46752163..02291d34c56 100644 --- 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 a8fe61fcf9c..730b0608eef 100644 --- 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 }