internal/lsp: remove source.FileContent
On FileHandle Read now just returns the data hash and error
This makes it more obvious that you should handle the error, rather than hiding
it all in a struct.
We also change the way we get and return content, the main source.File
constructs now hold a FileHandle that then updates on invalidation
Change-Id: I20be1b995355e948244342130eafec056df10081
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180417
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go
index 9936344..dfb97ae 100644
--- a/internal/lsp/cache/external.go
+++ b/internal/lsp/cache/external.go
@@ -41,16 +41,14 @@
return h.identity
}
-func (h *nativeFileHandle) Read(ctx context.Context) *source.FileContent {
- r := &source.FileContent{}
+func (h *nativeFileHandle) Read(ctx context.Context) ([]byte, string, error) {
filename, err := h.identity.URI.Filename()
if err != nil {
- r.Error = err
- return r
+ return nil, "", err
}
- r.Data, r.Error = ioutil.ReadFile(filename)
- if r.Error != nil {
- r.Hash = hashContents(r.Data)
+ data, err := ioutil.ReadFile(filename)
+ if err != nil {
+ return nil, "", err
}
- return r
+ return data, hashContents(data), nil
}
diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go
index e37ddcf..c4b93b9 100644
--- a/internal/lsp/cache/file.go
+++ b/internal/lsp/cache/file.go
@@ -9,6 +9,7 @@
"go/token"
"path/filepath"
"strings"
+ "sync"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
@@ -28,10 +29,10 @@
uris []span.URI
fname string
- view *view
- fh source.FileHandle
- fc *source.FileContent
- token *token.File
+ view *view
+ handleMu sync.Mutex
+ handle source.FileHandle
+ token *token.File
}
func basename(filename string) string {
@@ -51,32 +52,16 @@
return f.view
}
-// Content returns the contents of the file, reading it from file system if needed.
-func (f *fileBase) Content(ctx context.Context) *source.FileContent {
- f.view.mu.Lock()
- defer f.view.mu.Unlock()
-
- f.read(ctx)
- return f.fc
+// Content returns a handle for the contents of the file.
+func (f *fileBase) Handle(ctx context.Context) source.FileHandle {
+ f.handleMu.Lock()
+ defer f.handleMu.Unlock()
+ if f.handle == nil {
+ f.handle = f.view.Session().GetFile(f.URI())
+ }
+ return f.handle
}
func (f *fileBase) FileSet() *token.FileSet {
return f.view.Session().Cache().FileSet()
}
-
-// read is the internal part of GetContent. It assumes that the caller is
-// holding the mutex of the file's view.
-func (f *fileBase) read(ctx context.Context) {
- if err := ctx.Err(); err != nil {
- f.fc = &source.FileContent{Error: err}
- return
- }
- oldFH := f.fh
- f.fh = f.view.Session().GetFile(f.URI())
- // do we already have the right contents?
- if f.fc != nil && f.fh.Identity() == oldFH.Identity() {
- return
- }
- // update the contents
- f.fc = f.fh.Read(ctx)
-}
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 8fdd6f0..9a8ccdb 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -90,11 +90,11 @@
return true
}
// Get file content in case we don't already have it.
- f.read(ctx)
- if f.fc.Error != nil {
+ data, _, err := f.Handle(ctx).Read(ctx)
+ if err != nil {
return true
}
- parsed, _ := parser.ParseFile(f.FileSet(), f.filename(), f.fc.Data, parser.ImportsOnly)
+ parsed, _ := parser.ParseFile(f.FileSet(), f.filename(), data, parser.ImportsOnly)
if parsed == nil {
return true
}
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 84ce104..8795883 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -74,11 +74,11 @@
}
// We don't have a cached AST for this file, so we read its content and parse it.
- gof.read(imp.ctx)
- if gof.fc.Error != nil {
+ data, _, err := gof.Handle(imp.ctx).Read(imp.ctx)
+ if err != nil {
return
}
- src := gof.fc.Data
+ src := data
if src == nil {
parsed[i], errors[i] = nil, fmt.Errorf("no source for %v", filename)
return
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 6bf962d..4dd169f 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -40,7 +40,8 @@
type overlay struct {
session *session
uri span.URI
- content source.FileContent
+ data []byte
+ hash string
}
func (s *session) Shutdown(ctx context.Context) {
@@ -213,10 +214,8 @@
s.overlays[uri] = &overlay{
session: s,
uri: uri,
- content: source.FileContent{
- Data: data,
- Hash: hashContents(data),
- },
+ data: data,
+ hash: hashContents(data),
}
}
@@ -237,11 +236,8 @@
overlays := make(map[string][]byte)
for uri, overlay := range s.overlays {
- if overlay.content.Error != nil {
- continue
- }
if filename, err := uri.Filename(); err == nil {
- overlays[filename] = overlay.content.Data
+ overlays[filename] = overlay.data
}
}
return overlays
@@ -254,12 +250,12 @@
func (o *overlay) Identity() source.FileIdentity {
return source.FileIdentity{
URI: o.uri,
- Version: o.content.Hash,
+ Version: o.hash,
}
}
-func (o *overlay) Read(ctx context.Context) *source.FileContent {
- return &o.content
+func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
+ return o.data, o.hash, nil
}
type debugSession struct{ *session }
@@ -287,9 +283,9 @@
seen[overlay.uri] = f
files = append(files, f)
}
- f.Data = string(overlay.content.Data)
- f.Error = overlay.content.Error
- f.Hash = overlay.content.Hash
+ f.Data = string(overlay.data)
+ f.Error = nil
+ f.Hash = overlay.hash
}
sort.Slice(files, func(i int, j int) bool {
return files[i].URI < files[j].URI
@@ -301,13 +297,13 @@
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
for _, overlay := range s.overlays {
- if overlay.content.Hash == hash {
+ if overlay.hash == hash {
return &debug.File{
Session: s,
URI: overlay.uri,
- Data: string(overlay.content.Data),
- Error: overlay.content.Error,
- Hash: overlay.content.Hash,
+ Data: string(overlay.data),
+ Error: nil,
+ Hash: overlay.hash,
}
}
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index a2e7a81..06c3c2a 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -233,7 +233,7 @@
if f.pkg != nil {
f.view.remove(f.pkg.pkgPath, map[string]struct{}{})
}
- f.fc = nil
+ f.handle = nil
}
// remove invalidates a package and its reverse dependencies in the view's
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 0105601..6ae073a 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -202,8 +202,8 @@
v.Session().Logger().Errorf(ctx, "Could not find tokens for diagnostic: %v", spn.URI())
return spn
}
- fc := diagFile.Content(ctx)
- if fc.Error != nil {
+ data, _, err := diagFile.Handle(ctx).Read(ctx)
+ if err != nil {
v.Session().Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI())
return spn
}
@@ -216,7 +216,7 @@
}
start := s.Start()
offset := start.Offset()
- width := bytes.IndexAny(fc.Data[offset:], " \n,():;[]")
+ width := bytes.IndexAny(data[offset:], " \n,():;[]")
if width <= 0 {
return spn
}
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index d2971f0..56be5e0 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -56,15 +56,15 @@
// Imports formats a file using the goimports tool.
func Imports(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) {
- fc := f.Content(ctx)
- if fc.Error != nil {
- return nil, fc.Error
+ data, _, err := f.Handle(ctx).Read(ctx)
+ if err != nil {
+ return nil, err
}
tok := f.GetToken(ctx)
if tok == nil {
return nil, fmt.Errorf("no token file for %s", f.URI())
}
- formatted, err := imports.Process(tok.Name(), fc.Data, nil)
+ formatted, err := imports.Process(tok.Name(), data, nil)
if err != nil {
return nil, err
}
@@ -72,12 +72,12 @@
}
func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
- fc := file.Content(ctx)
- if fc.Error != nil {
- file.View().Session().Logger().Errorf(ctx, "Cannot compute text edits: %v", fc.Error)
+ data, _, err := file.Handle(ctx).Read(ctx)
+ if err != nil {
+ file.View().Session().Logger().Errorf(ctx, "Cannot compute text edits: %v", err)
return nil
}
- u := diff.SplitLines(string(fc.Data))
+ u := diff.SplitLines(string(data))
f := diff.SplitLines(formatted)
return DiffToEdits(file.URI(), diff.Operations(u, f))
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 28e767c..1837783 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -296,12 +296,12 @@
continue
}
ops := source.EditsToDiff(edits)
- fc := f.Content(ctx)
- if fc.Error != nil {
+ data, _, err := f.Handle(ctx).Read(ctx)
+ if err != nil {
t.Error(err)
continue
}
- got := strings.Join(diff.ApplyEdits(diff.SplitLines(string(fc.Data)), ops), "")
+ got := strings.Join(diff.ApplyEdits(diff.SplitLines(string(data)), ops), "")
if gofmted != got {
t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", filename, gofmted, got)
}
@@ -337,12 +337,12 @@
continue
}
ops := source.EditsToDiff(edits)
- fc := f.Content(ctx)
- if fc.Error != nil {
+ data, _, err := f.Handle(ctx).Read(ctx)
+ if err != nil {
t.Error(err)
continue
}
- got := strings.Join(diff.ApplyEdits(diff.SplitLines(string(fc.Data)), ops), "")
+ got := strings.Join(diff.ApplyEdits(diff.SplitLines(string(data)), ops), "")
if goimported != got {
t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", filename, goimported, got)
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index ca529cc..25d436a 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -18,14 +18,6 @@
"golang.org/x/tools/internal/span"
)
-// FileContent is returned from FileSystem implementation to represent the
-// contents of a file.
-type FileContent struct {
- Data []byte
- Error error
- Hash string
-}
-
// FileIdentity uniquely identifies a file at a version from a FileSystem.
type FileIdentity struct {
URI span.URI
@@ -35,14 +27,14 @@
// FileHandle represents a handle to a specific version of a single file from
// a specific file system.
type FileHandle interface {
- // FileSystem returns the file system this handle was aquired from.
+ // FileSystem returns the file system this handle was acquired from.
FileSystem() FileSystem
// Return the Identity for the file.
Identity() FileIdentity
- // Read reads the contents of a file and returns it.
- // If the file is not available, the returned FileContent will have no
- // data and an error.
- Read(ctx context.Context) *FileContent
+ // Read reads the contents of a file and returns it along with its hash
+ // value.
+ // If the file is not available, retruns a nil slice and an error.
+ Read(ctx context.Context) ([]byte, string, error)
}
// FileSystem is the interface to something that provides file contents.
@@ -161,7 +153,7 @@
type File interface {
URI() span.URI
View() View
- Content(ctx context.Context) *FileContent
+ Handle(ctx context.Context) FileHandle
FileSet() *token.FileSet
GetToken(ctx context.Context) *token.File
}
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 73d7451..edf7dfa 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -66,11 +66,10 @@
}
uri := span.NewURI(params.TextDocument.URI)
- fc := s.session.GetFile(uri).Read(ctx)
- if fc.Error != nil {
+ content, _, err := s.session.GetFile(uri).Read(ctx)
+ if err != nil {
return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "file not found")
}
- content := fc.Data
fset := s.session.Cache().FileSet()
filename, err := uri.Filename()
if err != nil {
diff --git a/internal/lsp/util.go b/internal/lsp/util.go
index 427ff07..33ed865 100644
--- a/internal/lsp/util.go
+++ b/internal/lsp/util.go
@@ -22,11 +22,11 @@
if err != nil {
return nil, nil, err
}
- fc := f.Content(ctx)
- if fc.Error != nil {
- return nil, nil, fc.Error
+ data, _, err := f.Handle(ctx).Read(ctx)
+ if err != nil {
+ return nil, nil, err
}
- m := protocol.NewColumnMapper(f.URI(), filename, f.FileSet(), f.GetToken(ctx), fc.Data)
+ m := protocol.NewColumnMapper(f.URI(), filename, f.FileSet(), f.GetToken(ctx), data)
return f, m, nil
}
