cmd/guru: definition: try parser-based resolution first

The parser does intra-file resolution.  It's only a best-effort but
it's extremely fast, so we try it first and fall back to the loader-based
implementation if it fails.

Also:
- factor definitionResult so it doesn't depend on either go/types.Object
  or go/ast.Object.
- guessImportPath: make importPath=="" result an error,
  since you can't import the empty string.  Simplify importQueryPackage.
- Make fastQueryPos continue in face of minor parse errors.
- describe: eliminate old TODO; suppress log statement.

Change-Id: I30a32487e9fd9d5308b1580ec12220cad2c16299
Reviewed-on: https://go-review.googlesource.com/19507
Reviewed-by: Daniel Morsing <daniel.morsing@gmail.com>

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
 }

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
 			}
 

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

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
 	}
 

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 {