internal/lsp/lsprpc: add test for definition outside of workspace

Add regression tests for GoToDefinition. In particular, exercise the
panic from golang/go#37045.

Updates golang/go#37045
Updates golang/go#36879

Change-Id: I67b562acd293f47907de0435c14b62c1a22cf2ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218322
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/fake/client.go

@@ -97,10 +97,7 @@ func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceE
 		return &protocol.ApplyWorkspaceEditResponse{FailureReason: "Edit.Changes is unsupported"}, nil
 	}
 	for _, change := range params.Edit.DocumentChanges {
-		path, err := c.ws.URIToPath(change.TextDocument.URI)
-		if err != nil {
-			return nil, err
-		}
+		path := c.ws.URIToPath(change.TextDocument.URI)
 		var edits []Edit
 		for _, lspEdit := range change.Edits {
 			edits = append(edits, fromProtocolTextEdit(lspEdit))

internal/lsp/fake/edit.go

@@ -54,6 +54,19 @@ func fromProtocolTextEdit(textEdit protocol.TextEdit) Edit {
 	}
 }
 
+// inText reports whether p is a valid position in the text buffer.
+func inText(p Pos, content []string) bool {
+	if p.Line < 0 || p.Line >= len(content) {
+		return false
+	}
+	// Note the strict right bound: the column indexes character _separators_,
+	// not characters.
+	if p.Column < 0 || p.Column > len(content[p.Line]) {
+		return false
+	}
+	return true
+}
+
 // editContent implements a simplistic, inefficient algorithm for applying text
 // edits to our buffer representation. It returns an error if the edit is
 // invalid for the current content.
@@ -61,23 +74,10 @@ func editContent(content []string, edit Edit) ([]string, error) {
 	if edit.End.Line < edit.Start.Line || (edit.End.Line == edit.Start.Line && edit.End.Column < edit.Start.Column) {
 		return nil, fmt.Errorf("invalid edit: end %v before start %v", edit.End, edit.Start)
 	}
-	// inText reports whether a position is within the bounds of the current
-	// text.
-	inText := func(p Pos) bool {
-		if p.Line < 0 || p.Line >= len(content) {
-			return false
-		}
-		// Note the strict right bound: the column indexes character _separators_,
-		// not characters.
-		if p.Column < 0 || p.Column > len(content[p.Line]) {
-			return false
-		}
-		return true
-	}
-	if !inText(edit.Start) {
+	if !inText(edit.Start, content) {
 		return nil, fmt.Errorf("start position %v is out of bounds", edit.Start)
 	}
-	if !inText(edit.End) {
+	if !inText(edit.End, content) {
 		return nil, fmt.Errorf("end position %v is out of bounds", edit.End)
 	}
 	// Splice the edit text in between the first and last lines of the edit.

internal/lsp/fake/editor.go

@@ -322,6 +322,41 @@ func (e *Editor) doEdits(ctx context.Context, path string, edits []Edit) (*proto
 	return params, nil
 }
 
-// TODO: expose more client functionality, for example GoToDefinition, Hover,
-// CodeAction, Rename, Completion, etc.  setting the content of an entire
-// buffer, etc.
+// GoToDefinition jumps to the definition of the symbol at the given position
+// in an open buffer.
+func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (string, Pos, error) {
+	if err := e.checkBufferPosition(path, pos); err != nil {
+		return "", Pos{}, err
+	}
+	params := &protocol.DefinitionParams{}
+	params.TextDocument.URI = e.ws.URI(path)
+	params.Position = pos.toProtocolPosition()
+
+	resp, err := e.server.Definition(ctx, params)
+	if err != nil {
+		return "", Pos{}, fmt.Errorf("Definition: %v", err)
+	}
+	if len(resp) == 0 {
+		return "", Pos{}, nil
+	}
+	newPath := e.ws.URIToPath(resp[0].URI)
+	newPos := fromProtocolPosition(resp[0].Range.Start)
+	e.OpenFile(ctx, newPath)
+	return newPath, newPos, nil
+}
+
+func (e *Editor) checkBufferPosition(path string, pos Pos) error {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	buf, ok := e.buffers[path]
+	if !ok {
+		return fmt.Errorf("buffer %q is not open", path)
+	}
+	if !inText(pos, buf.content) {
+		return fmt.Errorf("position %v is invalid in buffer %q", pos, path)
+	}
+	return nil
+}
+
+// TODO: expose more client functionality, for example Hover, CodeAction,
+// Rename, Completion, etc.  setting the content of an entire buffer, etc.

internal/lsp/fake/workspace.go

@@ -86,6 +86,10 @@ func (w *Workspace) AddWatcher(watcher func(context.Context, []FileEvent)) {
 // filePath returns the absolute filesystem path to a the workspace-relative
 // path.
 func (w *Workspace) filePath(path string) string {
+	fp := filepath.FromSlash(path)
+	if filepath.IsAbs(fp) {
+		return fp
+	}
 	return filepath.Join(w.workdir, filepath.FromSlash(path))
 }
 
@@ -94,13 +98,15 @@ func (w *Workspace) URI(path string) protocol.DocumentURI {
 	return toURI(w.filePath(path))
 }
 
-// URIToPath converts a uri to a workspace-relative path.
-func (w *Workspace) URIToPath(uri protocol.DocumentURI) (string, error) {
-	prefix := w.RootURI() + "/"
-	if !strings.HasPrefix(uri, prefix) {
-		return "", fmt.Errorf("uri %q outside of workspace", uri)
+// URIToPath converts a uri to a workspace-relative path (or an absolute path,
+// if the uri is outside of the workspace).
+func (w *Workspace) URIToPath(uri protocol.DocumentURI) string {
+	root := w.RootURI() + "/"
+	if strings.HasPrefix(uri, root) {
+		return strings.TrimPrefix(uri, root)
 	}
-	return strings.TrimPrefix(uri, prefix), nil
+	filename := span.NewURI(string(uri)).Filename()
+	return filepath.ToSlash(filename)
 }
 
 func toURI(fp string) protocol.DocumentURI {

internal/lsp/lsprpc/definition_test.go

@@ -0,0 +1,101 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package lsprpc
+
+import (
+	"fmt"
+	"path"
+	"testing"
+	"time"
+
+	"golang.org/x/tools/internal/lsp/fake"
+)
+
+const internalDefinition = `
+-- go.mod --
+module mod
+
+go 1.12
+-- main.go --
+package main
+
+import "fmt"
+
+func main() {
+	fmt.Println(message)
+}
+-- const.go --
+package main
+
+const message = "Hello World."
+`
+
+func TestGoToInternalDefinition(t *testing.T) {
+	t.Parallel()
+	ctx, env, cleanup := setupEnv(t, internalDefinition)
+	defer cleanup()
+
+	if err := env.editor.OpenFile(ctx, "main.go"); err != nil {
+		t.Fatal(err)
+	}
+	name, pos, err := env.editor.GoToDefinition(ctx, "main.go", fake.Pos{Line: 5, Column: 13})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if want := "const.go"; name != want {
+		t.Errorf("GoToDefinition: got file %q, want %q", name, want)
+	}
+	if want := (fake.Pos{Line: 2, Column: 6}); pos != want {
+		t.Errorf("GoToDefinition: got position %v, want %v", pos, want)
+	}
+}
+
+const stdlibDefinition = `
+-- go.mod --
+module mod
+
+go 1.12
+-- main.go --
+package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func main() {
+	fmt.Println(time.Now())
+}`
+
+func TestGoToStdlibDefinition(t *testing.T) {
+	t.Parallel()
+	ctx, env, cleanup := setupEnv(t, stdlibDefinition)
+	defer cleanup()
+
+	if err := env.editor.OpenFile(ctx, "main.go"); err != nil {
+		t.Fatal(err)
+	}
+	name, pos, err := env.editor.GoToDefinition(ctx, "main.go", fake.Pos{Line: 8, Column: 19})
+	if err != nil {
+		t.Fatal(err)
+	}
+	fmt.Println(time.Now())
+	if got, want := path.Base(name), "time.go"; got != want {
+		t.Errorf("GoToDefinition: got file %q, want %q", name, want)
+	}
+
+	// Test that we can jump to definition from outside our workspace.
+	// See golang.org/issues/37045.
+	newName, newPos, err := env.editor.GoToDefinition(ctx, name, pos)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if newName != name {
+		t.Errorf("GoToDefinition is not idempotent: got %q, want %q", newName, name)
+	}
+	if newPos != pos {
+		t.Errorf("GoToDefinition is not idempotent: got %v, want %v", newPos, pos)
+	}
+}

internal/lsp/lsprpc/diagnostics_test.go

@@ -43,7 +43,7 @@ func setupEnv(t *testing.T, txt string) (context.Context, testEnvironment, func(
 	t.Helper()
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 
-	ws, err := fake.NewWorkspace("get-diagnostics", []byte(txt))
+	ws, err := fake.NewWorkspace("lsprpc", []byte(txt))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -100,10 +100,7 @@ func (w diagnosticsWatcher) await(ctx context.Context, expected ...string) (map[
 		case <-ctx.Done():
 			return nil, ctx.Err()
 		case d := <-w.diagnostics:
-			pth, err := w.ws.URIToPath(d.URI)
-			if err != nil {
-				return nil, err
-			}
+			pth := w.ws.URIToPath(d.URI)
 			if expectedSet[pth] {
 				got[pth] = d
 			}