diff --git a/cmd/guru/definition.go b/cmd/guru/definition.go index d95fbc6f..0b6b7d17 100644 --- a/cmd/guru/definition.go +++ b/cmd/guru/definition.go @@ -8,18 +8,39 @@ import ( "fmt" "go/ast" "go/token" - "go/types" "golang.org/x/tools/cmd/guru/serial" "golang.org/x/tools/go/loader" ) // definition reports the location of the definition of an identifier. -// -// TODO(adonovan): opt: for intra-file references, the parser's -// resolution might be enough; we should start with that. -// func definition(q *Query) error { + // First try the simple resolution done by parser. + // It only works for intra-file references but it is very fast. + // (Extending this approach to all the files of the package, + // resolved using ast.NewPackage, was not worth the effort.) + { + qpos, err := fastQueryPos(q.Pos) + if err != nil { + return err + } + + id, _ := qpos.path[0].(*ast.Ident) + if id == nil { + return fmt.Errorf("no identifier here") + } + + if obj := id.Obj; obj != nil && obj.Pos().IsValid() { + q.Fset = qpos.fset + q.result = &definitionResult{ + pos: obj.Pos(), + descr: fmt.Sprintf("%s %s", obj.Kind, obj.Name), + } + return nil // success + } + } + + // Run the type checker. lconf := loader.Config{Build: q.Build} allowErrors(&lconf) @@ -52,25 +73,30 @@ func definition(q *Query) error { return fmt.Errorf("no object for identifier") } - q.result = &definitionResult{qpos, obj} + if !obj.Pos().IsValid() { + return fmt.Errorf("%s is built in", obj.Name()) + } + + q.result = &definitionResult{ + pos: obj.Pos(), + descr: qpos.objectString(obj), + } return nil } type definitionResult struct { - qpos *queryPos - obj types.Object // object it denotes + pos token.Pos // (nonzero) location of definition + descr string // description of object it denotes } func (r *definitionResult) display(printf printfFunc) { - printf(r.obj, "defined here as %s", r.qpos.objectString(r.obj)) + printf(r.pos, "defined here as %s", r.descr) } func (r *definitionResult) toSerial(res *serial.Result, fset *token.FileSet) { definition := &serial.Definition{ - Desc: r.obj.String(), - } - if pos := r.obj.Pos(); pos != token.NoPos { // Package objects have no Pos() - definition.ObjPos = fset.Position(pos).String() + Desc: r.descr, + ObjPos: fset.Position(r.pos).String(), } res.Definition = definition } diff --git a/cmd/guru/describe.go b/cmd/guru/describe.go index 5edabb7c..fdb94a8a 100644 --- a/cmd/guru/describe.go +++ b/cmd/guru/describe.go @@ -279,14 +279,12 @@ func findInterestingNode(pkginfo *loader.PackageInfo, path []ast.Node) ([]ast.No return path, actionPackage case *ast.ImportSpec: - // TODO(adonovan): fix: why no package object? go/types bug? return path[1:], actionPackage default: // e.g. blank identifier // or y in "switch y := x.(type)" // or code in a _test.go file that's not part of the package. - log.Printf("unknown reference %s in %T\n", n, path[1]) return path, actionUnknown } diff --git a/cmd/guru/guru.go b/cmd/guru/guru.go index d48f3088..960b0619 100644 --- a/cmd/guru/guru.go +++ b/cmd/guru/guru.go @@ -189,16 +189,10 @@ func importQueryPackage(pos string, conf *loader.Config) (string, error) { } filename := fqpos.fset.File(fqpos.start).Name() - // This will not work for ad-hoc packages - // such as $GOROOT/src/net/http/triv.go. - // TODO(adonovan): ensure we report a clear error. _, importPath, err := guessImportPath(filename, conf.Build) if err != nil { return "", err // can't find GOPATH dir } - if importPath == "" { - return "", fmt.Errorf("can't guess import path from %s", filename) - } // Check that it's possible to load the queried package. // (e.g. guru tests contain different 'package' decls in same dir.) @@ -219,6 +213,8 @@ func importQueryPackage(pos string, conf *loader.Config) (string, error) { case 'G': conf.Import(importPath) default: + // This happens for ad-hoc packages like + // $GOROOT/src/net/http/triv.go. return "", fmt.Errorf("package %q doesn't contain file %s", importPath, filename) } diff --git a/cmd/guru/pos.go b/cmd/guru/pos.go index 4566c06b..05064abf 100644 --- a/cmd/guru/pos.go +++ b/cmd/guru/pos.go @@ -114,7 +114,9 @@ func fastQueryPos(pos string) (*queryPos, error) { fset := token.NewFileSet() f, err := parser.ParseFile(fset, filename, nil, 0) - if err != nil { + // ParseFile usually returns a partial file along with an error. + // Only fail if there is no file. + if f == nil { return nil, err } diff --git a/cmd/guru/what.go b/cmd/guru/what.go index 1d6ddee8..ca161350 100644 --- a/cmd/guru/what.go +++ b/cmd/guru/what.go @@ -152,10 +152,15 @@ func guessImportPath(filename string, buildContext *build.Context) (srcdir, impo } } if srcdir == "" { - err = fmt.Errorf("directory %s is not beneath any of these GOROOT/GOPATH directories: %s", + return "", "", fmt.Errorf("directory %s is not beneath any of these GOROOT/GOPATH directories: %s", filepath.Dir(absFile), strings.Join(buildContext.SrcDirs(), ", ")) } - return + if importPath == "" { + // This happens for e.g. $GOPATH/src/a.go, but + // "" is not a valid path for (*go/build).Import. + return "", "", fmt.Errorf("cannot load package in root of source directory %s", srcdir) + } + return srcdir, importPath, nil } func segments(path string) []string {