Skip to content

Commit

Permalink
internal/lsp: fix builds and tests for go1.12+
Browse files Browse the repository at this point in the history
Seems we've drifted a bit from go1.12 support, mostly due to error
wrapping.

Fix this, as well as some assorted other failures.

I haven't tested 1.12 interactively.

For golang/go#39146

Change-Id: Id347ead2a13e89b76d2ae0047750e6b6b49911eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250941
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
  • Loading branch information
findleyr committed Aug 27, 2020
1 parent 17fd2f2 commit df83f4e
Show file tree
Hide file tree
Showing 32 changed files with 114 additions and 93 deletions.
2 changes: 1 addition & 1 deletion internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ func (v *View) maybeReinitialize() {

func (v *View) setBuildInformation(ctx context.Context, folder span.URI, options source.Options) error {
if err := checkPathCase(folder.Filename()); err != nil {
return fmt.Errorf("invalid workspace configuration: %w", err)
return errors.Errorf("invalid workspace configuration: %w", err)
}
// Make sure to get the `go env` before continuing with initialization.
modFile, err := v.setGoEnv(ctx, options.Env)
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/cmd/prepare_rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ package cmd

import (
"context"
"errors"
"flag"
"fmt"

"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/tool"
errors "golang.org/x/xerrors"
)

// prepareRename implements the prepare_rename verb for gopls.
Expand Down Expand Up @@ -68,7 +68,7 @@ func (r *prepareRename) Run(ctx context.Context, args ...string) error {
}
result, err := conn.PrepareRename(ctx, &p)
if err != nil {
return fmt.Errorf("prepare_rename failed: %w", err)
return errors.Errorf("prepare_rename failed: %w", err)
}
if result == nil {
return ErrInvalidRenamePosition
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package cmd

import (
"context"
"errors"
"flag"
"fmt"
"io"
Expand All @@ -22,6 +21,7 @@ import (
"golang.org/x/tools/internal/lsp/lsprpc"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/tool"
errors "golang.org/x/xerrors"
)

// Serve is a struct that exposes the configurable parts of the LSP server as
Expand Down
8 changes: 4 additions & 4 deletions internal/lsp/debug/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/xerrors"
errors "golang.org/x/xerrors"
)

type instanceKeyType int
Expand Down Expand Up @@ -151,7 +151,7 @@ func (st *State) Clients() []*Client {
return clients
}

// View returns the View that matches the supplied id.
// Client returns the Client matching the supplied id.
func (st *State) Client(id string) *Client {
for _, c := range st.Clients() {
if c.Session.ID() == id {
Expand Down Expand Up @@ -338,7 +338,7 @@ func (i *Instance) SetLogFile(logfile string, isDaemon bool) (func(), error) {
}
f, err := os.Create(logfile)
if err != nil {
return nil, fmt.Errorf("unable to create log file: %w", err)
return nil, errors.Errorf("unable to create log file: %w", err)
}
closeLog = func() {
defer f.Close()
Expand Down Expand Up @@ -480,7 +480,7 @@ func makeGlobalExporter(stderr io.Writer) event.Exporter {

if event.IsLog(ev) {
// Don't log context cancellation errors.
if err := keys.Err.Get(ev); xerrors.Is(err, context.Canceled) {
if err := keys.Err.Get(ev); errors.Is(err, context.Canceled) {
return ctx
}
// Make sure any log messages without an instance go to stderr.
Expand Down
46 changes: 23 additions & 23 deletions internal/lsp/fake/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package fake
import (
"bufio"
"context"
"errors"
"fmt"
"path/filepath"
"regexp"
Expand All @@ -18,6 +17,7 @@ import (
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
errors "golang.org/x/xerrors"
)

// Editor is a fake editor client. It keeps track of client state and can be
Expand Down Expand Up @@ -118,7 +118,7 @@ func (e *Editor) Connect(ctx context.Context, conn jsonrpc2.Conn, hooks ClientHo
func (e *Editor) Shutdown(ctx context.Context) error {
if e.Server != nil {
if err := e.Server.Shutdown(ctx); err != nil {
return fmt.Errorf("Shutdown: %w", err)
return errors.Errorf("Shutdown: %w", err)
}
}
return nil
Expand All @@ -130,7 +130,7 @@ func (e *Editor) Exit(ctx context.Context) error {
// Not all LSP clients issue the exit RPC, but we do so here to ensure that
// we gracefully handle it on multi-session servers.
if err := e.Server.Exit(ctx); err != nil {
return fmt.Errorf("Exit: %w", err)
return errors.Errorf("Exit: %w", err)
}
}
return nil
Expand All @@ -150,7 +150,7 @@ func (e *Editor) Close(ctx context.Context) error {
// connection closed itself
return nil
case <-ctx.Done():
return fmt.Errorf("connection not closed: %w", ctx.Err())
return errors.Errorf("connection not closed: %w", ctx.Err())
}
}

Expand Down Expand Up @@ -222,14 +222,14 @@ func (e *Editor) initialize(ctx context.Context, withoutWorkspaceFolders bool, e
if e.Server != nil {
resp, err := e.Server.Initialize(ctx, params)
if err != nil {
return fmt.Errorf("initialize: %w", err)
return errors.Errorf("initialize: %w", err)
}
e.mu.Lock()
e.serverCapabilities = resp.Capabilities
e.mu.Unlock()

if err := e.Server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
return fmt.Errorf("initialized: %w", err)
return errors.Errorf("initialized: %w", err)
}
}
// TODO: await initial configuration here, or expect gopls to manage that?
Expand Down Expand Up @@ -271,7 +271,7 @@ func (e *Editor) OpenFileWithContent(ctx context.Context, path, content string)
if err := e.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{
TextDocument: item,
}); err != nil {
return fmt.Errorf("DidOpen: %w", err)
return errors.Errorf("DidOpen: %w", err)
}
}
return nil
Expand Down Expand Up @@ -313,7 +313,7 @@ func (e *Editor) CreateBuffer(ctx context.Context, path, content string) error {
if err := e.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{
TextDocument: item,
}); err != nil {
return fmt.Errorf("DidOpen: %w", err)
return errors.Errorf("DidOpen: %w", err)
}
}
return nil
Expand All @@ -334,7 +334,7 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error {
if err := e.Server.DidClose(ctx, &protocol.DidCloseTextDocumentParams{
TextDocument: e.textDocumentIdentifier(path),
}); err != nil {
return fmt.Errorf("DidClose: %w", err)
return errors.Errorf("DidClose: %w", err)
}
}
return nil
Expand All @@ -350,10 +350,10 @@ func (e *Editor) textDocumentIdentifier(path string) protocol.TextDocumentIdenti
// the filesystem.
func (e *Editor) SaveBuffer(ctx context.Context, path string) error {
if err := e.OrganizeImports(ctx, path); err != nil {
return fmt.Errorf("organizing imports before save: %w", err)
return errors.Errorf("organizing imports before save: %w", err)
}
if err := e.FormatBuffer(ctx, path); err != nil {
return fmt.Errorf("formatting before save: %w", err)
return errors.Errorf("formatting before save: %w", err)
}
return e.SaveBufferWithoutActions(ctx, path)
}
Expand All @@ -379,11 +379,11 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro
TextDocument: docID,
Reason: protocol.Manual,
}); err != nil {
return fmt.Errorf("WillSave: %w", err)
return errors.Errorf("WillSave: %w", err)
}
}
if err := e.sandbox.Workdir.WriteFile(ctx, path, content); err != nil {
return fmt.Errorf("writing %q: %w", path, err)
return errors.Errorf("writing %q: %w", path, err)
}
if e.Server != nil {
params := &protocol.DidSaveTextDocumentParams{
Expand All @@ -396,7 +396,7 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro
params.Text = &content
}
if err := e.Server.DidSave(ctx, params); err != nil {
return fmt.Errorf("DidSave: %w", err)
return errors.Errorf("DidSave: %w", err)
}
}
return nil
Expand All @@ -417,7 +417,7 @@ func contentPosition(content string, offset int) (Pos, error) {
line++
}
if err := scanner.Err(); err != nil {
return Pos{}, fmt.Errorf("scanning content: %w", err)
return Pos{}, errors.Errorf("scanning content: %w", err)
}
// Scan() will drop the last line if it is empty. Correct for this.
if strings.HasSuffix(content, "\n") && offset == start {
Expand Down Expand Up @@ -565,7 +565,7 @@ func (e *Editor) editBufferLocked(ctx context.Context, path string, edits []Edit
}
if e.Server != nil {
if err := e.Server.DidChange(ctx, params); err != nil {
return fmt.Errorf("DidChange: %w", err)
return errors.Errorf("DidChange: %w", err)
}
}
return nil
Expand All @@ -583,15 +583,15 @@ func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (stri

resp, err := e.Server.Definition(ctx, params)
if err != nil {
return "", Pos{}, fmt.Errorf("definition: %w", err)
return "", Pos{}, errors.Errorf("definition: %w", err)
}
if len(resp) == 0 {
return "", Pos{}, nil
}
newPath := e.sandbox.Workdir.URIToPath(resp[0].URI)
newPos := fromProtocolPosition(resp[0].Range.Start)
if err := e.OpenFile(ctx, newPath); err != nil {
return "", Pos{}, fmt.Errorf("OpenFile: %w", err)
return "", Pos{}, errors.Errorf("OpenFile: %w", err)
}
return newPath, newPos, nil
}
Expand All @@ -603,7 +603,7 @@ func (e *Editor) Symbol(ctx context.Context, query string) ([]SymbolInformation,

resp, err := e.Server.Symbol(ctx, params)
if err != nil {
return nil, fmt.Errorf("symbol: %w", err)
return nil, errors.Errorf("symbol: %w", err)
}
var res []SymbolInformation
for _, si := range resp {
Expand Down Expand Up @@ -658,7 +658,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
}
actions, err := e.Server.CodeAction(ctx, params)
if err != nil {
return fmt.Errorf("textDocument/codeAction: %w", err)
return errors.Errorf("textDocument/codeAction: %w", err)
}
for _, action := range actions {
var match bool
Expand All @@ -679,7 +679,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
}
edits := convertEdits(change.Edits)
if err := e.EditBuffer(ctx, path, edits); err != nil {
return fmt.Errorf("editing buffer %q: %w", path, err)
return errors.Errorf("editing buffer %q: %w", path, err)
}
}
// Execute any commands. The specification says that commands are
Expand Down Expand Up @@ -716,7 +716,7 @@ func (e *Editor) FormatBuffer(ctx context.Context, path string) error {
params.TextDocument.URI = e.sandbox.Workdir.URI(path)
resp, err := e.Server.Formatting(ctx, params)
if err != nil {
return fmt.Errorf("textDocument/formatting: %w", err)
return errors.Errorf("textDocument/formatting: %w", err)
}
e.mu.Lock()
defer e.mu.Unlock()
Expand Down Expand Up @@ -850,7 +850,7 @@ func (e *Editor) Hover(ctx context.Context, path string, pos Pos) (*protocol.Mar

resp, err := e.Server.Hover(ctx, params)
if err != nil {
return nil, Pos{}, fmt.Errorf("hover: %w", err)
return nil, Pos{}, errors.Errorf("hover: %w", err)
}
if resp == nil {
return nil, Pos{}, nil
Expand Down
7 changes: 3 additions & 4 deletions internal/lsp/fake/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/txtar"
errors "golang.org/x/xerrors"
)

// Sandbox holds a collection of temporary resources to use for working with Go
Expand Down Expand Up @@ -203,7 +204,7 @@ func (sb *Sandbox) RunGoCommand(ctx context.Context, verb string, args ...string
// check if we need to send any any "watched" file events.
if sb.Workdir != nil {
if err := sb.Workdir.CheckForFileChanges(ctx); err != nil {
return fmt.Errorf("checking for file changes: %w", err)
return errors.Errorf("checking for file changes: %w", err)
}
}
return nil
Expand All @@ -213,9 +214,7 @@ func (sb *Sandbox) RunGoCommand(ctx context.Context, verb string, args ...string
func (sb *Sandbox) Close() error {
var goCleanErr error
if sb.gopath != "" {
if err := sb.RunGoCommand(context.Background(), "clean", "-modcache"); err != nil {
goCleanErr = fmt.Errorf("cleaning modcache: %v", err)
}
goCleanErr = sb.RunGoCommand(context.Background(), "clean", "-modcache")
}
err := os.RemoveAll(sb.basedir)
if err != nil || goCleanErr != nil {
Expand Down
16 changes: 8 additions & 8 deletions internal/lsp/fake/workdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package fake

import (
"context"
"fmt"
"io/ioutil"
"os"
"path/filepath"
Expand All @@ -16,6 +15,7 @@ import (

"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/span"
errors "golang.org/x/xerrors"
)

// FileEvent wraps the protocol.FileEvent so that it can be associated with a
Expand Down Expand Up @@ -48,12 +48,12 @@ func (w *Workdir) WriteInitialFiles(txt string) error {
files := unpackTxt(txt)
for name, data := range files {
if err := w.writeFileData(name, string(data)); err != nil {
return fmt.Errorf("writing to workdir: %w", err)
return errors.Errorf("writing to workdir: %w", err)
}
}
// Poll to capture the current file state.
if _, err := w.pollFiles(); err != nil {
return fmt.Errorf("polling files: %w", err)
return errors.Errorf("polling files: %w", err)
}
return nil
}
Expand Down Expand Up @@ -134,7 +134,7 @@ func (w *Workdir) ChangeFilesOnDisk(ctx context.Context, events []FileEvent) err
case protocol.Deleted:
fp := w.filePath(e.Path)
if err := os.Remove(fp); err != nil {
return fmt.Errorf("removing %q: %w", e.Path, err)
return errors.Errorf("removing %q: %w", e.Path, err)
}
case protocol.Changed, protocol.Created:
if _, err := w.writeFile(ctx, e.Path, e.Content); err != nil {
Expand All @@ -150,7 +150,7 @@ func (w *Workdir) ChangeFilesOnDisk(ctx context.Context, events []FileEvent) err
func (w *Workdir) RemoveFile(ctx context.Context, path string) error {
fp := w.filePath(path)
if err := os.Remove(fp); err != nil {
return fmt.Errorf("removing %q: %w", path, err)
return errors.Errorf("removing %q: %w", path, err)
}
evts := []FileEvent{{
Path: path,
Expand Down Expand Up @@ -205,7 +205,7 @@ func (w *Workdir) writeFile(ctx context.Context, path, content string) (FileEven
fp := w.filePath(path)
_, err := os.Stat(fp)
if err != nil && !os.IsNotExist(err) {
return FileEvent{}, fmt.Errorf("checking if %q exists: %w", path, err)
return FileEvent{}, errors.Errorf("checking if %q exists: %w", path, err)
}
var changeType protocol.FileChangeType
if os.IsNotExist(err) {
Expand All @@ -228,10 +228,10 @@ func (w *Workdir) writeFile(ctx context.Context, path, content string) (FileEven
func (w *Workdir) writeFileData(path string, content string) error {
fp := w.filePath(path)
if err := os.MkdirAll(filepath.Dir(fp), 0755); err != nil {
return fmt.Errorf("creating nested directory: %w", err)
return errors.Errorf("creating nested directory: %w", err)
}
if err := ioutil.WriteFile(fp, []byte(content), 0644); err != nil {
return fmt.Errorf("writing %q: %w", path, err)
return errors.Errorf("writing %q: %w", path, err)
}
return nil
}
Expand Down
Loading

0 comments on commit df83f4e

Please sign in to comment.