internal/lsp: error handling

cmd/gopls/GOPLS.md

@@ -0,0 +1,16 @@
+# `GOPLS.md`
+
+## development
+
+1. Create a fork of <https://github.com/golang/tools>, e.g., <https://github.com/banaio/tools>.
+2. Clone your fork.
+3. Modify some code in the `internal/lsp` directory. A good candidate is the `Initialize` function
+in `internal/lsp/server.go`.
+4. Run the `cmd/gopls/install-gopls.sh` script to update the `gopls` binary.
+
+```sh
+git clone git@github.com:banaio/tools.git
+cd tools
+vim internal/lsp/server.go # modify some code
+cmd/gopls/install-gopls.sh
+```

cmd/gopls/install-gopls.sh

@@ -0,0 +1,24 @@
+#!/bin/bash -e
+RACE=false
+if [[ "${1}" == "-race" ]]; then
+  RACE=true
+fi
+
+echo -e "\\033[92m ---> currrent cmd/gopls \\033[0m"
+find "$(command -v gopls)" -printf "%c %p\\n"
+
+echo -e "\\033[92m ---> testing internal/lsp (race=${RACE}) \\033[0m"
+if ${RACE}; then
+    go test -race "$(pwd)"/internal/lsp/...
+else
+    go test "$(pwd)"/internal/lsp/...
+fi
+
+if ${RACE}; then
+    go install -a -race "$(pwd)"/cmd/gopls
+else
+    go install -a "$(pwd)"/cmd/gopls
+fi
+
+echo -e "\\033[92m ---> installed cmd/gopls (race=${RACE}) \\033[0m"
+find "$(command -v gopls)" -printf "%c %p\\n"

internal/lsp/cache/file.go

@@ -25,63 +25,68 @@ type File struct {
 }
 
 // GetContent returns the contents of the file, reading it from file system if needed.
-func (f *File) GetContent() []byte {
+func (f *File) GetContent() ([]byte, error) {
 	f.view.mu.Lock()
 	defer f.view.mu.Unlock()
-	f.read()
-	return f.content
+	if f.content != nil {
+		return f.content, nil
+	}
+	content, err := f.read()
+	if err != nil {
+		return nil, err
+	}
+	f.content = content
+	return f.content, nil
 }
 
 func (f *File) GetFileSet() *token.FileSet {
 	return f.view.Config.Fset
 }
 
-func (f *File) GetToken() *token.File {
+func (f *File) GetToken() (*token.File, error) {
 	f.view.mu.Lock()
 	defer f.view.mu.Unlock()
 	if f.token == nil {
 		if err := f.view.parse(f.URI); err != nil {
-			return nil
+			return nil, err
 		}
 	}
-	return f.token
+	return f.token, nil
 }
 
-func (f *File) GetAST() *ast.File {
+func (f *File) GetAST() (*ast.File, error) {
 	f.view.mu.Lock()
 	defer f.view.mu.Unlock()
 	if f.ast == nil {
 		if err := f.view.parse(f.URI); err != nil {
-			return nil
+			return nil, err
 		}
 	}
-	return f.ast
+	return f.ast, nil
 }
 
-func (f *File) GetPackage() *packages.Package {
+func (f *File) GetPackage() (*packages.Package, error) {
 	f.view.mu.Lock()
 	defer f.view.mu.Unlock()
 	if f.pkg == nil {
 		if err := f.view.parse(f.URI); err != nil {
-			return nil
+			return nil, err
 		}
 	}
-	return f.pkg
+	return f.pkg, nil
 }
 
 // read is the internal part of Read that presumes the lock is already held
-func (f *File) read() {
-	if f.content != nil {
-		return
-	}
+func (f *File) read() ([]byte, error) {
 	// we don't know the content yet, so read it
 	filename, err := f.URI.Filename()
 	if err != nil {
-		return
+		return nil, err
 	}
 	content, err := ioutil.ReadFile(filename)
 	if err != nil {
-		return
+		return nil, err
 	}
-	f.content = content
+	// f.content = content
+	return content, nil
 }

internal/lsp/cache/view.go

@@ -46,6 +46,9 @@ func (v *View) FileSet() *token.FileSet {
 }
 
 func (v *View) GetAnalysisCache() *source.AnalysisCache {
+	v.mu.Lock()
+	defer v.mu.Unlock()
+
 	if v.analysisCache == nil {
 		v.analysisCache = source.NewAnalysisCache()
 	}
@@ -91,8 +94,8 @@ func (v *View) SetContent(ctx context.Context, uri source.URI, content []byte) (
 // adds the file to the managed set if needed.
 func (v *View) GetFile(ctx context.Context, uri source.URI) (source.File, error) {
 	v.mu.Lock()
+	defer v.mu.Unlock()
 	f := v.getFile(uri)
-	v.mu.Unlock()
 	return f, nil
 }
 

internal/lsp/cmd/definition.go

@@ -65,7 +65,10 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
 	if err != nil {
 		return err
 	}
-	tok := f.GetToken()
+	tok, err := f.GetToken()
+	if err != nil {
+		return err
+	}
 	pos := tok.Pos(from.Start.Offset)
 	ident, err := source.Identifier(ctx, view, f, pos)
 	if err != nil {
@@ -115,7 +118,10 @@ func buildDefinition(view source.View, ident *source.IdentifierInfo) (*Definitio
 
 func buildGuruDefinition(view source.View, ident *source.IdentifierInfo) (*guru.Definition, error) {
 	loc := newLocation(view.FileSet(), ident.Declaration.Range)
-	pkg := ident.File.GetPackage()
+	pkg, err := ident.File.GetPackage()
+	if err != nil {
+		return nil, err
+	}
 	// guru does not support ranges
 	loc.End = loc.Start
 	// Behavior that attempts to match the expected output for guru. For an example

internal/lsp/format.go

@@ -2,6 +2,7 @@ package lsp
 
 import (
 	"context"
+	"errors"
 
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
@@ -17,7 +18,10 @@ func formatRange(ctx context.Context, v source.View, uri protocol.DocumentURI, r
 	if err != nil {
 		return nil, err
 	}
-	tok := f.GetToken()
+	tok, err := f.GetToken()
+	if err != nil {
+		return nil, err
+	}
 	var r source.Range
 	if rng == nil {
 		r.Start = tok.Pos(0)
@@ -29,15 +33,26 @@ func formatRange(ctx context.Context, v source.View, uri protocol.DocumentURI, r
 	if err != nil {
 		return nil, err
 	}
-	return toProtocolEdits(f, edits), nil
+
+	protocolEdits, err := toProtocolEdits(f, edits)
+	if err != nil {
+		return nil, err
+	}
+	return protocolEdits, nil
 }
 
-func toProtocolEdits(f source.File, edits []source.TextEdit) []protocol.TextEdit {
+func toProtocolEdits(f source.File, edits []source.TextEdit) ([]protocol.TextEdit, error) {
 	if edits == nil {
-		return nil
+		return nil, errors.New("toProtocolEdits, edits == nil")
+	}
+	tok, err := f.GetToken()
+	if err != nil {
+		return nil, err
+	}
+	content, err := f.GetContent()
+	if err != nil {
+		return nil, err
 	}
-	tok := f.GetToken()
-	content := f.GetContent()
 	// When a file ends with an empty line, the newline character is counted
 	// as part of the previous line. This causes the formatter to insert
 	// another unnecessary newline on each formatting. We handle this case by
@@ -56,5 +71,5 @@ func toProtocolEdits(f source.File, edits []source.TextEdit) []protocol.TextEdit
 			NewText: edit.NewText,
 		}
 	}
-	return result
+	return result, nil
 }

internal/lsp/imports.go

@@ -20,7 +20,10 @@ func organizeImports(ctx context.Context, v source.View, uri protocol.DocumentUR
 	if err != nil {
 		return nil, err
 	}
-	tok := f.GetToken()
+	tok, err := f.GetToken()
+	if err != nil {
+		return nil, err
+	}
 	r := source.Range{
 		Start: tok.Pos(0),
 		End:   tok.Pos(tok.Size()),
@@ -29,5 +32,9 @@ func organizeImports(ctx context.Context, v source.View, uri protocol.DocumentUR
 	if err != nil {
 		return nil, err
 	}
-	return toProtocolEdits(f, edits), nil
+	protocolEdits, err := toProtocolEdits(f, edits)
+	if err != nil {
+		return nil, err
+	}
+	return protocolEdits, nil
 }

internal/lsp/lsp_test.go

@@ -396,7 +396,11 @@ func (f formats) test(t *testing.T, s *server) {
 				})
 			}
 		}
-		split := strings.SplitAfter(string(f.GetContent()), "\n")
+		contents, err := f.GetContent()
+		if err != nil {
+			t.Error(err)
+		}
+		split := strings.SplitAfter(string(contents), "\n")
 		got := strings.Join(diff.ApplyEdits(split, ops), "")
 		if gofmted != got {
 			t.Errorf("format failed for %s: expected '%v', got '%v'", filename, gofmted, got)
@@ -440,6 +444,11 @@ func (d definitions) test(t *testing.T, s *server, typ bool) {
 }
 
 func (d definitions) collect(fset *token.FileSet, src, target packagestest.Range) {
-	loc := toProtocolLocation(fset, source.Range(src))
-	d[loc] = toProtocolLocation(fset, source.Range(target))
+	srcLocation, err := toProtocolLocation(fset, source.Range(src))
+	if err == nil {
+		targetLocation, err := toProtocolLocation(fset, source.Range(target))
+		if err == nil {
+			d[*srcLocation] = *targetLocation
+		}
+	}
 }

internal/lsp/position.go

@@ -6,6 +6,7 @@ package lsp
 
 import (
 	"context"
+	"fmt"
 	"go/token"
 	"net/url"
 
@@ -36,18 +37,24 @@ func fromProtocolLocation(ctx context.Context, v *cache.View, loc protocol.Locat
 	if err != nil {
 		return source.Range{}, err
 	}
-	tok := f.GetToken()
+	tok, err := f.GetToken()
+	if err != nil {
+		return source.Range{}, err
+	}
 	return fromProtocolRange(tok, loc.Range), nil
 }
 
 // toProtocolLocation converts from a source range back to a protocol location.
-func toProtocolLocation(fset *token.FileSet, r source.Range) protocol.Location {
+func toProtocolLocation(fset *token.FileSet, r source.Range) (*protocol.Location, error) {
 	tok := fset.File(r.Start)
+	if tok == nil {
+		return nil, fmt.Errorf("cannot get File for position=%+v", r.Start)
+	}
 	uri := source.ToURI(tok.Name())
-	return protocol.Location{
+	return &protocol.Location{
 		URI:   protocol.DocumentURI(uri),
 		Range: toProtocolRange(tok, r),
-	}
+	}, nil
 }
 
 // fromProtocolRange converts a protocol range to a source range.