Merge pull request #196 from sourcegraph/sg/perf2

langserver: add godef-based hover backend

langserver/definition.go

@@ -7,7 +7,6 @@ import (
 	"go/ast"
 	"go/build"
 	"go/token"
-	"io/ioutil"
 	"log"
 	"path/filepath"
 
@@ -23,7 +22,8 @@ var UseBinaryPkgCache = false
 
 func (h *LangHandler) handleDefinition(ctx context.Context, conn jsonrpc2.JSONRPC2, req *jsonrpc2.Request, params lsp.TextDocumentPositionParams) ([]lsp.Location, error) {
 	if UseBinaryPkgCache {
-		return h.handleDefinitionGodef(ctx, conn, req, params)
+		_, _, locs, err := h.definitionGodef(ctx, params)
+		return locs, err
 	}
 
 	res, err := h.handleXDefinition(ctx, conn, req, params)
@@ -37,28 +37,38 @@ func (h *LangHandler) handleDefinition(ctx context.Context, conn jsonrpc2.JSONRP
 	return locs, nil
 }
 
-func (h *LangHandler) handleDefinitionGodef(ctx context.Context, conn jsonrpc2.JSONRPC2, req *jsonrpc2.Request, params lsp.TextDocumentPositionParams) ([]lsp.Location, error) {
+var testOSToVFSPath func(osPath string) string
+
+func (h *LangHandler) definitionGodef(ctx context.Context, params lsp.TextDocumentPositionParams) (*token.FileSet, *godef.Result, []lsp.Location, error) {
+	// In the case of testing, our OS paths and VFS paths do not match. In the
+	// real world, this is never the case. Give the test suite the opportunity
+	// to correct the path now.
+	vfsURI := params.TextDocument.URI
+	if testOSToVFSPath != nil {
+		vfsURI = pathToURI(testOSToVFSPath(uriToFilePath(vfsURI)))
+	}
+
 	// Read file contents and calculate byte offset.
-	filename := h.FilePath(params.TextDocument.URI)
-	contents, err := ioutil.ReadFile(filename)
+	contents, err := h.readFile(ctx, vfsURI)
 	if err != nil {
-		return nil, err
+		return nil, nil, nil, err
 	}
+	filename := h.FilePath(params.TextDocument.URI)
 	offset, valid, why := offsetForPosition(contents, params.Position)
 	if !valid {
-		return nil, fmt.Errorf("invalid position: %s:%d:%d (%s)", filename, params.Position.Line, params.Position.Character, why)
+		return nil, nil, nil, fmt.Errorf("invalid position: %s:%d:%d (%s)", filename, params.Position.Line, params.Position.Character, why)
 	}
 
 	// Invoke godef to determine the position of the definition.
 	fset := token.NewFileSet()
 	res, err := godef.Godef(fset, offset, filename, contents)
 	if err != nil {
-		return nil, err
+		return nil, nil, nil, err
 	}
 	if res.Package != nil {
 		// TODO: return directory location. This right now at least matches our
 		// other implementation.
-		return []lsp.Location{}, nil
+		return fset, res, []lsp.Location{}, nil
 	}
 	loc := goRangeToLSPLocation(fset, res.Start, res.End)
 
@@ -69,7 +79,8 @@ func (h *LangHandler) handleDefinitionGodef(ctx context.Context, conn jsonrpc2.J
 		loc.URI = pathToURI(filepath.Join(build.Default.GOROOT, "/src/builtin/builtin.go"))
 		loc.Range = lsp.Range{}
 	}
-	return []lsp.Location{loc}, nil
+
+	return fset, res, []lsp.Location{loc}, nil
 }
 
 func (h *LangHandler) handleXDefinition(ctx context.Context, conn jsonrpc2.JSONRPC2, req *jsonrpc2.Request, params lsp.TextDocumentPositionParams) ([]symbolLocationInformation, error) {

langserver/hover.go

@@ -6,8 +6,12 @@ import (
 	"fmt"
 	"go/ast"
 	"go/build"
+	"go/format"
+	"go/parser"
 	"go/token"
 	"go/types"
+	"path/filepath"
+	"sort"
 	"strings"
 
 	doc "github.com/slimsag/godocmd"
@@ -16,6 +20,10 @@ import (
 )
 
 func (h *LangHandler) handleHover(ctx context.Context, conn jsonrpc2.JSONRPC2, req *jsonrpc2.Request, params lsp.TextDocumentPositionParams) (*lsp.Hover, error) {
+	if UseBinaryPkgCache {
+		return h.handleHoverGodef(ctx, conn, req, params)
+	}
+
 	if !isFileURI(params.TextDocument.URI) {
 		return nil, &jsonrpc2.Error{
 			Code:    jsonrpc2.CodeInvalidParams,
@@ -245,3 +253,272 @@ func prettyPrintTypesString(s string) string {
 	}
 	return b.String()
 }
+
+func (h *LangHandler) handleHoverGodef(ctx context.Context, conn jsonrpc2.JSONRPC2, req *jsonrpc2.Request, params lsp.TextDocumentPositionParams) (*lsp.Hover, error) {
+	// First perform the equivalent of a textDocument/definition request in
+	// order to resolve the definition position.
+	fset, res, _, err := h.definitionGodef(ctx, params)
+	if err != nil {
+		return nil, err
+	}
+
+	// If our target is a package import statement or package selector, then we
+	// handle that separately now.
+	if res.Package != nil {
+		// res.Package.Name is invalid since it was imported with FindOnly, so
+		// import normally now.
+		bpkg, err := build.Default.ImportDir(res.Package.Dir, 0)
+		if err != nil {
+			return nil, err
+		}
+
+		// Parse the entire dir into its respective AST packages.
+		pkgs, err := parser.ParseDir(fset, res.Package.Dir, nil, parser.ParseComments)
+		if err != nil {
+			return nil, err
+		}
+		pkg := pkgs[bpkg.Name]
+
+		// Find the package doc comments.
+		pkgFiles := make([]*ast.File, 0, len(pkg.Files))
+		for _, f := range pkg.Files {
+			pkgFiles = append(pkgFiles, f)
+		}
+		comments := packageDoc(pkgFiles, bpkg.Name)
+
+		return &lsp.Hover{
+			Contents: maybeAddComments(comments, []lsp.MarkedString{{Language: "go", Value: fmt.Sprintf("package %s (%q)", bpkg.Name, bpkg.ImportPath)}}),
+
+			// TODO(slimsag): I think we can add Range here, but not exactly
+			// sure. res.Start and res.End are only present if it's a package
+			// selector, not an import statement. Since Range is optional,
+			// we're omitting it here.
+		}, nil
+	}
+
+	loc := goRangeToLSPLocation(fset, res.Start, res.End)
+
+	if loc.URI == "file://" {
+		// TODO: builtins do not have valid URIs or locations.
+		return &lsp.Hover{}, nil
+	}
+
+	filename := uriToFilePath(loc.URI)
+
+	// Parse the entire dir into its respective AST packages.
+	pkgs, err := parser.ParseDir(fset, filepath.Dir(filename), nil, parser.ParseComments)
+	if err != nil {
+		return nil, err
+	}
+
+	// Locate the AST package that contains the file we're interested in.
+	foundImportPath, foundPackage, err := packageForFile(pkgs, filename)
+	if err != nil {
+		return nil, err
+	}
+
+	// Create documentation for the package.
+	docPkg := doc.New(foundPackage, foundImportPath, doc.AllDecls)
+
+	// Locate the target in the docs.
+	target := fset.Position(res.Start)
+	docObject := findDocTarget(fset, target, docPkg)
+	if docObject == nil {
+		return nil, fmt.Errorf("failed to find doc object for %s", target)
+	}
+
+	contents, node := fmtDocObject(fset, docObject, target)
+	r := rangeForNode(fset, node)
+	return &lsp.Hover{
+		Contents: contents,
+		Range:    &r,
+	}, nil
+}
+
+// packageForFile returns the import path and pkg from pkgs that contains the
+// named file.
+func packageForFile(pkgs map[string]*ast.Package, filename string) (string, *ast.Package, error) {
+	for path, pkg := range pkgs {
+		for pkgFile := range pkg.Files {
+			if pkgFile == filename {
+				return path, pkg, nil
+			}
+		}
+	}
+	return "", nil, fmt.Errorf("failed to find %q in packages %q", filename, pkgs)
+}
+
+// inRange tells if x is in the range of a-b inclusive.
+func inRange(x, a, b token.Position) bool {
+	if x.Filename != a.Filename || x.Filename != b.Filename {
+		return false
+	}
+	return x.Offset >= a.Offset && x.Offset <= b.Offset
+}
+
+// findDocTarget walks an input *doc.Package and locates the *doc.Value,
+// *doc.Type, or *doc.Func for the given target position.
+func findDocTarget(fset *token.FileSet, target token.Position, in interface{}) interface{} {
+	switch v := in.(type) {
+	case *doc.Package:
+		for _, x := range v.Consts {
+			if r := findDocTarget(fset, target, x); r != nil {
+				return r
+			}
+		}
+		for _, x := range v.Types {
+			if r := findDocTarget(fset, target, x); r != nil {
+				return r
+			}
+		}
+		for _, x := range v.Vars {
+			if r := findDocTarget(fset, target, x); r != nil {
+				return r
+			}
+		}
+		for _, x := range v.Funcs {
+			if r := findDocTarget(fset, target, x); r != nil {
+				return r
+			}
+		}
+		return nil
+	case *doc.Value:
+		if inRange(target, fset.Position(v.Decl.Pos()), fset.Position(v.Decl.End())) {
+			return v
+		}
+		return nil
+	case *doc.Type:
+		if inRange(target, fset.Position(v.Decl.Pos()), fset.Position(v.Decl.End())) {
+			return v
+		}
+		return nil
+	case *doc.Func:
+		if inRange(target, fset.Position(v.Decl.Pos()), fset.Position(v.Decl.End())) {
+			return v
+		}
+		return nil
+	default:
+		panic("unreachable")
+	}
+}
+
+// fmtDocObject formats one of:
+//
+// *doc.Value
+// *doc.Type
+// *doc.Func
+//
+func fmtDocObject(fset *token.FileSet, x interface{}, target token.Position) ([]lsp.MarkedString, ast.Node) {
+	switch v := x.(type) {
+	case *doc.Value: // Vars and Consts
+		// Sort the specs by distance to find the one nearest to target.
+		sort.Sort(byDistance{v.Decl.Specs, fset, target})
+		spec := v.Decl.Specs[0].(*ast.ValueSpec)
+
+		// Use the doc directly above the var inside a var() block, or if there
+		// is none, fall back to the doc directly above the var() block.
+		doc := spec.Doc.Text()
+		if doc == "" {
+			doc = v.Doc
+		}
+
+		// Create a copy of the spec with no doc for formatting separately.
+		cpy := *spec
+		cpy.Doc = nil
+		value := v.Decl.Tok.String() + " " + fmtNode(fset, &cpy)
+		return maybeAddComments(doc, []lsp.MarkedString{{Language: "go", Value: value}}), spec
+
+	case *doc.Type: // Type declarations
+		spec := v.Decl.Specs[0].(*ast.TypeSpec)
+
+		// Handle interfaces methods and struct fields separately now.
+		switch s := spec.Type.(type) {
+		case *ast.InterfaceType:
+			// Find the method that is an exact match for our target position.
+			for _, field := range s.Methods.List {
+				if fset.Position(field.Pos()).Offset == target.Offset {
+					// An exact match.
+					value := fmt.Sprintf("func (%s).%s%s", spec.Name.Name, field.Names[0].Name, strings.TrimPrefix(fmtNode(fset, field.Type), "func"))
+					return maybeAddComments(field.Doc.Text(), []lsp.MarkedString{{Language: "go", Value: value}}), field
+				}
+			}
+
+		case *ast.StructType:
+			// Find the field that is an exact match for our target position.
+			for _, field := range s.Fields.List {
+				if fset.Position(field.Pos()).Offset == target.Offset {
+					// An exact match.
+					value := fmt.Sprintf("struct field %s %s", field.Names[0], fmtNode(fset, field.Type))
+					return maybeAddComments(field.Doc.Text(), []lsp.MarkedString{{Language: "go", Value: value}}), field
+				}
+			}
+		}
+
+		// Formatting of all type declarations: structs, interfaces, integers, etc.
+		name := v.Decl.Tok.String() + " " + spec.Name.Name + " " + typeName(fset, spec.Type)
+		res := []lsp.MarkedString{{Language: "go", Value: name}}
+
+		doc := spec.Doc.Text()
+		if doc == "" {
+			doc = v.Doc
+		}
+		res = maybeAddComments(doc, res)
+
+		if n := typeName(fset, spec.Type); n == "interface" || n == "struct" {
+			res = append(res, lsp.MarkedString{Language: "go", Value: fmtNode(fset, spec.Type)})
+		}
+		return res, spec
+
+	case *doc.Func: // Functions
+		return maybeAddComments(v.Doc, []lsp.MarkedString{{Language: "go", Value: fmtNode(fset, v.Decl)}}), v.Decl
+	default:
+		panic("unreachable")
+	}
+}
+
+// typeName returns the name of typ, shortening interface and struct types to
+// just "interface" and "struct" rather than their full contents (incl. methods
+// and fields).
+func typeName(fset *token.FileSet, typ ast.Expr) string {
+	switch typ.(type) {
+	case *ast.InterfaceType:
+		return "interface"
+	case *ast.StructType:
+		return "struct"
+	default:
+		return fmtNode(fset, typ)
+	}
+}
+
+// fmtNode formats the given node as a string.
+func fmtNode(fset *token.FileSet, n ast.Node) string {
+	var buf bytes.Buffer
+	err := format.Node(&buf, fset, n)
+	if err != nil {
+		panic("unreachable")
+	}
+	return buf.String()
+}
+
+// byDistance sorts specs by distance to the target position.
+type byDistance struct {
+	specs  []ast.Spec
+	fset   *token.FileSet
+	target token.Position
+}
+
+func (b byDistance) Len() int      { return len(b.specs) }
+func (b byDistance) Swap(i, j int) { b.specs[i], b.specs[j] = b.specs[j], b.specs[i] }
+func (b byDistance) Less(ii, jj int) bool {
+	i := b.fset.Position(b.specs[ii].Pos())
+	j := b.fset.Position(b.specs[jj].Pos())
+	return abs(b.target.Offset-i.Offset) < abs(b.target.Offset-j.Offset)
+}
+
+// abs returns the absolute value of x.
+func abs(x int) int {
+	if x < 0 {
+		return -x
+	}
+	return x
+}