internal/lsp: make definition use the lsp protocol

instead of driving the source pacakge directly, it indirects through the lsp
protocol (the same way check does)
We are normalizing on all the command lines doing this, so that server mode
is more viable in the future.

Change-Id: Ib5f2a059a44a5c60a53129c554e3cc14ca72c4a8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/170577
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cmd/check.go

@@ -8,8 +8,6 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"go/token"
-	"io/ioutil"
 
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/span"
@@ -54,38 +52,27 @@ func (c *check) Run(ctx context.Context, args ...string) error {
 	client := &checkClient{
 		diagnostics: make(chan entry),
 	}
-	client.app = c.app
+	checking := map[span.URI]*protocol.ColumnMapper{}
-	checking := map[span.URI][]byte{}
 	// now we ready to kick things off
-	server, err := c.app.connect(ctx, client)
+	_, err := c.app.connect(ctx, client)
 	if err != nil {
 		return err
 	}
 	for _, arg := range args {
 		uri := span.FileURI(arg)
-		content, err := ioutil.ReadFile(arg)
+		m, err := client.AddFile(ctx, uri)
 		if err != nil {
 			return err
 		}
-		checking[uri] = content
+		checking[uri] = m
-		p := &protocol.DidOpenTextDocumentParams{}
-		p.TextDocument.URI = string(uri)
-		p.TextDocument.Text = string(content)
-		if err := server.DidOpen(ctx, p); err != nil {
-			return err
-		}
 	}
 	// now wait for results
 	for entry := range client.diagnostics {
 		//TODO:timeout?
-		content, found := checking[entry.uri]
+		m, found := checking[entry.uri]
 		if !found {
 			continue
 		}
-		fset := token.NewFileSet()
-		f := fset.AddFile(string(entry.uri), -1, len(content))
-		f.SetLinesForContent(content)
-		m := protocol.NewColumnMapper(entry.uri, fset, f, content)
 		for _, d := range entry.diagnostics {
 			spn, err := m.RangeSpan(d.Range)
 			if err != nil {

internal/lsp/cmd/cmd.go

@@ -14,6 +14,7 @@ import (
 	"go/ast"
 	"go/parser"
 	"go/token"
+	"io/ioutil"
 	"log"
 	"net"
 	"os"
@@ -118,7 +119,13 @@ func (app *Application) commands() []tool.Application {
 	}
 }
 
-func (app *Application) connect(ctx context.Context, client protocol.Client) (protocol.Server, error) {
+type cmdClient interface {
+	protocol.Client
+
+	prepare(app *Application, server protocol.Server)
+}
+
+func (app *Application) connect(ctx context.Context, client cmdClient) (protocol.Server, error) {
 	var server protocol.Server
 	switch app.Remote {
 	case "":
@@ -150,6 +157,7 @@ func (app *Application) connect(ctx context.Context, client protocol.Client) (pr
 			"configuration": true,
 		},
 	}
+	client.prepare(app, server)
 	if _, err := server.Initialize(ctx, params); err != nil {
 		return nil, err
 	}
@@ -161,7 +169,9 @@ func (app *Application) connect(ctx context.Context, client protocol.Client) (pr
 
 type baseClient struct {
 	protocol.Server
-	app *Application
+	app    *Application
+	server protocol.Server
+	fset   *token.FileSet
 }
 
 func (c *baseClient) ShowMessage(ctx context.Context, p *protocol.ShowMessageParams) error { return nil }
@@ -217,3 +227,30 @@ func (c *baseClient) ApplyEdit(ctx context.Context, p *protocol.ApplyWorkspaceEd
 func (c *baseClient) PublishDiagnostics(ctx context.Context, p *protocol.PublishDiagnosticsParams) error {
 	return nil
 }
+
+func (c *baseClient) prepare(app *Application, server protocol.Server) {
+	c.app = app
+	c.server = server
+	c.fset = token.NewFileSet()
+}
+
+func (c *baseClient) AddFile(ctx context.Context, uri span.URI) (*protocol.ColumnMapper, error) {
+	fname, err := uri.Filename()
+	if err != nil {
+		return nil, fmt.Errorf("%v: %v", uri, err)
+	}
+	content, err := ioutil.ReadFile(fname)
+	if err != nil {
+		return nil, fmt.Errorf("%v: %v", uri, err)
+	}
+	f := c.fset.AddFile(fname, -1, len(content))
+	f.SetLinesForContent(content)
+	m := protocol.NewColumnMapper(uri, c.fset, f, content)
+	p := &protocol.DidOpenTextDocumentParams{}
+	p.TextDocument.URI = string(uri)
+	p.TextDocument.Text = string(content)
+	if err := c.server.DidOpen(ctx, p); err != nil {
+		return nil, fmt.Errorf("%v: %v", uri, err)
+	}
+	return m, nil
+}

internal/lsp/cmd/definition.go

@@ -5,18 +5,15 @@
 package cmd
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"flag"
 	"fmt"
-	"go/types"
 	"os"
+	"strings"
 
 	guru "golang.org/x/tools/cmd/guru/serial"
-	"golang.org/x/tools/internal/lsp/cache"
+	"golang.org/x/tools/internal/lsp/protocol"
-	"golang.org/x/tools/internal/lsp/source"
-	"golang.org/x/tools/internal/lsp/xlog"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/tool"
 )
@@ -31,9 +28,9 @@ type Definition struct {
 // help is still valid.
 // They refer to "Set" in "flag.FlagSet" from the DetailedHelp method below.
 const (
-	exampleLine   = 47
+	exampleLine   = 44
 	exampleColumn = 47
-	exampleOffset = 1359
+	exampleOffset = 1270
 )
 
 // definition implements the definition noun for the query command.
@@ -62,31 +59,67 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
 	if len(args) != 1 {
 		return tool.CommandLineErrorf("definition expects 1 argument")
 	}
-	log := xlog.New(xlog.StdSink{})
+	client := &baseClient{}
-	view := cache.NewView(ctx, log, "definition_test", span.FileURI(d.query.app.Config.Dir), &d.query.app.Config)
+	server, err := d.query.app.connect(ctx, client)
+	if err != nil {
+		return err
+	}
 	from := span.Parse(args[0])
-	f, err := view.GetFile(ctx, from.URI())
+	m, err := client.AddFile(ctx, from.URI())
 	if err != nil {
 		return err
 	}
-	converter := span.NewTokenConverter(view.FileSet(), f.GetToken(ctx))
+	loc, err := m.Location(from)
-	rng, err := from.Range(converter)
 	if err != nil {
 		return err
 	}
-	ident, err := source.Identifier(ctx, view, f, rng.Start)
+	p := protocol.TextDocumentPositionParams{
+		TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},
+		Position:     loc.Range.Start,
+	}
+	locs, err := server.Definition(ctx, &p)
 	if err != nil {
 		return fmt.Errorf("%v: %v", from, err)
 	}
-	if ident == nil {
+
+	if len(locs) == 0 {
 		return fmt.Errorf("%v: not an identifier", from)
 	}
+	hover, err := server.Hover(ctx, &p)
+	if err != nil {
+		return fmt.Errorf("%v: %v", from, err)
+	}
+	if hover == nil {
+		return fmt.Errorf("%v: not an identifier", from)
+	}
+	m, err = client.AddFile(ctx, span.NewURI(locs[0].URI))
+	if err != nil {
+		return fmt.Errorf("%v: %v", from, err)
+	}
+	definition, err := m.Span(locs[0])
+	if err != nil {
+		return fmt.Errorf("%v: %v", from, err)
+	}
+	//TODO: either work out how to request plain text, or
+	//use a less kludgy way of cleaning the markdown
+	description := hover.Contents.Value
+	if v := strings.TrimPrefix(description, "```go"); v != description {
+		description = strings.TrimSuffix(v, "```")
+	}
+	description = strings.TrimSpace(description)
 	var result interface{}
 	switch d.query.Emulate {
 	case "":
-		result, err = buildDefinition(ctx, view, ident)
+		result = &Definition{
+			Span:        definition,
+			Description: description,
+		}
 	case emulateGuru:
-		result, err = buildGuruDefinition(ctx, view, ident)
+		pos := span.New(definition.URI(), definition.Start(), definition.Start())
+		result = &guru.Definition{
+			ObjPos: fmt.Sprint(pos),
+			Desc:   description,
+		}
 	default:
 		return fmt.Errorf("unknown emulation for definition: %s", d.query.Emulate)
 	}
@@ -108,82 +141,3 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
 	}
 	return nil
 }
-
-func buildDefinition(ctx context.Context, view source.View, ident *source.IdentifierInfo) (*Definition, error) {
-	content, err := ident.Hover(ctx, nil)
-	if err != nil {
-		return nil, err
-	}
-	spn, err := ident.Declaration.Range.Span()
-	if err != nil {
-		return nil, err
-	}
-	return &Definition{
-		Span:        spn,
-		Description: content,
-	}, nil
-}
-
-func buildGuruDefinition(ctx context.Context, view source.View, ident *source.IdentifierInfo) (*guru.Definition, error) {
-	spn, err := ident.Declaration.Range.Span()
-	if err != nil {
-		return nil, err
-	}
-	pkg := ident.File.GetPackage(ctx)
-	// guru does not support ranges
-	if !spn.IsPoint() {
-		spn = span.New(spn.URI(), spn.Start(), spn.Start())
-	}
-	// Behavior that attempts to match the expected output for guru. For an example
-	// of the format, see the associated definition tests.
-	buf := &bytes.Buffer{}
-	q := types.RelativeTo(pkg.GetTypes())
-	qualifyName := ident.Declaration.Object.Pkg() != pkg.GetTypes()
-	name := ident.Name
-	var suffix interface{}
-	switch obj := ident.Declaration.Object.(type) {
-	case *types.TypeName:
-		fmt.Fprint(buf, "type")
-	case *types.Var:
-		if obj.IsField() {
-			qualifyName = false
-			fmt.Fprint(buf, "field")
-			suffix = obj.Type()
-		} else {
-			fmt.Fprint(buf, "var")
-		}
-	case *types.Func:
-		fmt.Fprint(buf, "func")
-		typ := obj.Type()
-		if obj.Type() != nil {
-			if sig, ok := typ.(*types.Signature); ok {
-				buf := &bytes.Buffer{}
-				if recv := sig.Recv(); recv != nil {
-					if named, ok := recv.Type().(*types.Named); ok {
-						fmt.Fprintf(buf, "(%s).%s", named.Obj().Name(), name)
-					}
-				}
-				if buf.Len() == 0 {
-					buf.WriteString(name)
-				}
-				types.WriteSignature(buf, sig, q)
-				name = buf.String()
-			}
-		}
-	default:
-		fmt.Fprintf(buf, "unknown [%T]", obj)
-	}
-	fmt.Fprint(buf, " ")
-	if qualifyName {
-		fmt.Fprintf(buf, "%s.", ident.Declaration.Object.Pkg().Path())
-	}
-	fmt.Fprint(buf, name)
-	if suffix != nil {
-		fmt.Fprint(buf, " ")
-		fmt.Fprint(buf, suffix)
-	}
-	return &guru.Definition{
-		ObjPos: fmt.Sprint(spn),
-		Desc:   buf.String(),
-	}, nil
-}

internal/lsp/source/diagnostics.go

@@ -85,25 +85,7 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
 	for _, diag := range diags {
 		spn := span.Parse(diag.Pos)
 		if spn.IsPoint() && diag.Kind == packages.TypeError {
-			// Don't set a range if it's anything other than a type error.
+			spn = pointToSpan(ctx, v, spn)
-			if diagFile, err := v.GetFile(ctx, spn.URI()); err == nil {
-				tok := diagFile.GetToken(ctx)
-				if tok == nil {
-					v.Logger().Errorf(ctx, "Could not matching tokens for diagnostic: %v", diagFile.URI())
-					continue
-				}
-				content := diagFile.GetContent(ctx)
-				c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok)
-				s, err := spn.WithOffset(c)
-				//we just don't bother producing an error if this failed
-				if err == nil {
-					start := s.Start()
-					offset := start.Offset()
-					if l := bytes.IndexAny(content[offset:], " \n,():;[]"); l > 0 {
-						spn = span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+l, offset+l))
-					}
-				}
-			}
 		}
 		diagnostic := Diagnostic{
 			Span:     spn,
@@ -142,6 +124,39 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
 	return reports, nil
 }
 
+func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span {
+	// Don't set a range if it's anything other than a type error.
+	diagFile, err := v.GetFile(ctx, spn.URI())
+	if err != nil {
+		v.Logger().Errorf(ctx, "Could find file for diagnostic: %v", spn.URI())
+		return spn
+	}
+	tok := diagFile.GetToken(ctx)
+	if tok == nil {
+		v.Logger().Errorf(ctx, "Could not find tokens for diagnostic: %v", spn.URI())
+		return spn
+	}
+	content := diagFile.GetContent(ctx)
+	if content == nil {
+		v.Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI())
+		return spn
+	}
+	c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok)
+	s, err := spn.WithOffset(c)
+	//we just don't bother producing an error if this failed
+	if err != nil {
+		v.Logger().Errorf(ctx, "invalid span for diagnostic: %v: %v", spn.URI(), err)
+		return spn
+	}
+	start := s.Start()
+	offset := start.Offset()
+	width := bytes.IndexAny(content[offset:], " \n,():;[]")
+	if width <= 0 {
+		return spn
+	}
+	return span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width))
+}
+
 func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.URI][]Diagnostic {
 	return map[span.URI][]Diagnostic{
 		uri: []Diagnostic{{