internal/lsp/cache: return concrete types where possible

For testability, and to allow the exchange of debug information when
forwarding the LSP, it will be necessary to access debug information
stored in cache objects. This is cumbersome to do if our constructors
return source interfaces rather than concrete types.

This CL changes cache.New and (*Cache).NewSession to return concrete
types. This required removing NewSession from source.Cache. I would
argue that this makes sense from a philosophical perspective: everything
in the source package operates in a context where the Session and Cache
already exist.

Updates golang/go#34111

Change-Id: I01721db827d51117f9479f1544b15cedae0c5921
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220077
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Rob Findley 2020-02-18 20:59:37 -05:00 committed by Robert Findley
parent 33153249e7
commit c4d4ea9c79
7 changed files with 38 additions and 41 deletions

View File

@ -19,9 +19,9 @@ import (
"golang.org/x/tools/internal/span"
)
func New(options func(*source.Options)) source.Cache {
func New(options func(*source.Options)) *Cache {
index := atomic.AddInt64(&cacheIndex, 1)
c := &cache{
c := &Cache{
fs: &nativeFileSystem{},
id: strconv.FormatInt(index, 10),
fset: token.NewFileSet(),
@ -31,7 +31,7 @@ func New(options func(*source.Options)) source.Cache {
return c
}
type cache struct {
type Cache struct {
fs source.FileSystem
id string
fset *token.FileSet
@ -45,7 +45,7 @@ type fileKey struct {
}
type fileHandle struct {
cache *cache
cache *Cache
underlying source.FileHandle
handle *memoize.Handle
}
@ -57,7 +57,7 @@ type fileData struct {
err error
}
func (c *cache) GetFile(uri span.URI) source.FileHandle {
func (c *Cache) GetFile(uri span.URI) source.FileHandle {
underlying := c.fs.GetFile(uri)
key := fileKey{
identity: underlying.Identity(),
@ -74,9 +74,9 @@ func (c *cache) GetFile(uri span.URI) source.FileHandle {
}
}
func (c *cache) NewSession() source.Session {
func (c *Cache) NewSession() *Session {
index := atomic.AddInt64(&sessionIndex, 1)
s := &session{
s := &Session{
cache: c,
id: strconv.FormatInt(index, 10),
options: source.DefaultOptions(),
@ -86,7 +86,7 @@ func (c *cache) NewSession() source.Session {
return s
}
func (c *cache) FileSet() *token.FileSet {
func (c *Cache) FileSet() *token.FileSet {
return c.fset
}
@ -115,8 +115,8 @@ func hashContents(contents []byte) string {
var cacheIndex, sessionIndex, viewIndex int64
type debugCache struct{ *cache }
type debugCache struct{ *Cache }
func (c *cache) ID() string { return c.id }
func (c *Cache) ID() string { return c.id }
func (c debugCache) FileSet() *token.FileSet { return c.fset }
func (c debugCache) MemStats() map[reflect.Type]int { return c.store.Stats() }

View File

@ -17,7 +17,7 @@ func (v debugView) ID() string { return v.id }
func (v debugView) Session() debug.Session { return debugSession{v.session} }
func (v debugView) Env() []string { return v.Options().Env }
type debugSession struct{ *session }
type debugSession struct{ *Session }
func (s debugSession) ID() string { return s.id }
func (s debugSession) Cache() debug.Cache { return debugCache{s.cache} }

View File

@ -47,7 +47,7 @@ type parseGoData struct {
err error
}
func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) source.ParseGoHandle {
func (c *Cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) source.ParseGoHandle {
key := parseKey{
file: fh.Identity(),
mode: mode,

View File

@ -19,8 +19,8 @@ import (
errors "golang.org/x/xerrors"
)
type session struct {
cache *cache
type Session struct {
cache *Cache
id string
options source.Options
@ -34,7 +34,7 @@ type session struct {
}
type overlay struct {
session *session
session *Session
uri span.URI
text []byte
hash string
@ -62,15 +62,15 @@ func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
return o.text, o.hash, nil
}
func (s *session) Options() source.Options {
func (s *Session) Options() source.Options {
return s.options
}
func (s *session) SetOptions(options source.Options) {
func (s *Session) SetOptions(options source.Options) {
s.options = options
}
func (s *session) Shutdown(ctx context.Context) {
func (s *Session) Shutdown(ctx context.Context) {
s.viewMu.Lock()
defer s.viewMu.Unlock()
for _, view := range s.views {
@ -81,11 +81,11 @@ func (s *session) Shutdown(ctx context.Context) {
debug.DropSession(debugSession{s})
}
func (s *session) Cache() source.Cache {
func (s *Session) Cache() source.Cache {
return s.cache
}
func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) (source.View, source.Snapshot, error) {
func (s *Session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) (source.View, source.Snapshot, error) {
s.viewMu.Lock()
defer s.viewMu.Unlock()
v, snapshot, err := s.createView(ctx, name, folder, options)
@ -98,7 +98,7 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt
return v, snapshot, nil
}
func (s *session) createView(ctx context.Context, name string, folder span.URI, options source.Options) (*view, *snapshot, error) {
func (s *Session) createView(ctx context.Context, name string, folder span.URI, options source.Options) (*view, *snapshot, error) {
index := atomic.AddInt64(&viewIndex, 1)
// We want a true background context and not a detached context here
// the spans need to be unrelated and no tag values should pollute it.
@ -147,7 +147,7 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
}
// View returns the view by name.
func (s *session) View(name string) source.View {
func (s *Session) View(name string) source.View {
s.viewMu.Lock()
defer s.viewMu.Unlock()
for _, view := range s.views {
@ -160,11 +160,11 @@ func (s *session) View(name string) source.View {
// ViewOf returns a view corresponding to the given URI.
// If the file is not already associated with a view, pick one using some heuristics.
func (s *session) ViewOf(uri span.URI) (source.View, error) {
func (s *Session) ViewOf(uri span.URI) (source.View, error) {
return s.viewOf(uri)
}
func (s *session) viewOf(uri span.URI) (*view, error) {
func (s *Session) viewOf(uri span.URI) (*view, error) {
s.viewMu.Lock()
defer s.viewMu.Unlock()
@ -181,7 +181,7 @@ func (s *session) viewOf(uri span.URI) (*view, error) {
return v, nil
}
func (s *session) viewsOf(uri span.URI) []*view {
func (s *Session) viewsOf(uri span.URI) []*view {
s.viewMu.Lock()
defer s.viewMu.Unlock()
@ -194,7 +194,7 @@ func (s *session) viewsOf(uri span.URI) []*view {
return views
}
func (s *session) Views() []source.View {
func (s *Session) Views() []source.View {
s.viewMu.Lock()
defer s.viewMu.Unlock()
result := make([]source.View, len(s.views))
@ -206,7 +206,7 @@ func (s *session) Views() []source.View {
// bestView finds the best view to associate a given URI with.
// viewMu must be held when calling this method.
func (s *session) bestView(uri span.URI) (*view, error) {
func (s *Session) bestView(uri span.URI) (*view, error) {
if len(s.views) == 0 {
return nil, errors.Errorf("no views in the session")
}
@ -227,7 +227,7 @@ func (s *session) bestView(uri span.URI) (*view, error) {
return s.views[0], nil
}
func (s *session) removeView(ctx context.Context, view *view) error {
func (s *Session) removeView(ctx context.Context, view *view) error {
s.viewMu.Lock()
defer s.viewMu.Unlock()
i, err := s.dropView(ctx, view)
@ -242,7 +242,7 @@ func (s *session) removeView(ctx context.Context, view *view) error {
return nil
}
func (s *session) updateView(ctx context.Context, view *view, options source.Options) (*view, *snapshot, error) {
func (s *Session) updateView(ctx context.Context, view *view, options source.Options) (*view, *snapshot, error) {
s.viewMu.Lock()
defer s.viewMu.Unlock()
i, err := s.dropView(ctx, view)
@ -263,7 +263,7 @@ func (s *session) updateView(ctx context.Context, view *view, options source.Opt
return v, snapshot, nil
}
func (s *session) dropView(ctx context.Context, v *view) (int, error) {
func (s *Session) dropView(ctx context.Context, v *view) (int, error) {
// we always need to drop the view map
s.viewMap = make(map[span.URI]*view)
for i := range s.views {
@ -277,7 +277,7 @@ func (s *session) dropView(ctx context.Context, v *view) (int, error) {
return -1, errors.Errorf("view %s for %v not found", v.Name(), v.Folder())
}
func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, error) {
func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, error) {
views := make(map[*view]map[span.URI]source.FileHandle)
overlays, err := s.updateOverlays(ctx, changes)
@ -322,7 +322,7 @@ func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModif
return snapshots, nil
}
func (s *session) isOpen(uri span.URI) bool {
func (s *Session) isOpen(uri span.URI) bool {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
@ -330,7 +330,7 @@ func (s *session) isOpen(uri span.URI) bool {
return open
}
func (s *session) updateOverlays(ctx context.Context, changes []source.FileModification) (map[span.URI]*overlay, error) {
func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModification) (map[span.URI]*overlay, error) {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
@ -406,7 +406,7 @@ func (s *session) updateOverlays(ctx context.Context, changes []source.FileModif
return overlays, nil
}
func (s *session) GetFile(uri span.URI) source.FileHandle {
func (s *Session) GetFile(uri span.URI) source.FileHandle {
if overlay := s.readOverlay(uri); overlay != nil {
return overlay
}
@ -414,7 +414,7 @@ func (s *session) GetFile(uri span.URI) source.FileHandle {
return s.cache.GetFile(uri)
}
func (s *session) readOverlay(uri span.URI) *overlay {
func (s *Session) readOverlay(uri span.URI) *overlay {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()

View File

@ -34,7 +34,7 @@ import (
)
type view struct {
session *session
session *Session
id string
options source.Options

View File

@ -17,8 +17,8 @@ import (
"golang.org/x/sync/errgroup"
"golang.org/x/tools/internal/jsonrpc2"
"golang.org/x/tools/internal/lsp"
"golang.org/x/tools/internal/lsp/cache"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
)
// The StreamServer type is a jsonrpc2.StreamServer that handles incoming
@ -33,7 +33,7 @@ type StreamServer struct {
// NewStreamServer creates a StreamServer using the shared cache. If
// withTelemetry is true, each session is instrumented with telemetry that
// records RPC statistics.
func NewStreamServer(cache source.Cache, withTelemetry bool) *StreamServer {
func NewStreamServer(cache *cache.Cache, withTelemetry bool) *StreamServer {
s := &StreamServer{
withTelemetry: withTelemetry,
}

View File

@ -228,9 +228,6 @@ type Cache interface {
// A FileSystem that reads file contents from external storage.
FileSystem
// NewSession creates a new Session manager and returns it.
NewSession() Session
// FileSet returns the shared fileset used by all files in the system.
FileSet() *token.FileSet