internal/lsp: stop requiring file kind when fetching a file

This change consolidates the FileKind into only the FileHandle.
Previously, it had been set in multiple places, which required users to
pass in a FileKind when fetching a file. This resulted in confusion,
particularly in places when users did not have access to the file kind.

Change-Id: I9e07d7320c46a21d453ffe108d1431a615706a71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213459
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

Author: stamblerre

internal/lsp/cache/builtin.go

@@ -44,7 +44,7 @@ func (v *view) buildBuiltinPackage(ctx context.Context) error {
 	pkg := pkgs[0]
 	files := make(map[string]*ast.File)
 	for _, filename := range pkg.GoFiles {
-		fh := v.session.GetFile(span.FileURI(filename), source.Go)
+		fh := v.session.GetFile(span.FileURI(filename))
 		ph := v.session.cache.ParseGoHandle(fh, source.ParseFull)
 		v.builtin.files = append(v.builtin.files, ph)
 		file, _, _, err := ph.Parse(ctx)

internal/lsp/cache/cache.go

@@ -56,8 +56,8 @@ type fileData struct {
 	err   error
 }
 
-func (c *cache) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
-	underlying := c.fs.GetFile(uri, kind)
+func (c *cache) GetFile(uri span.URI) source.FileHandle {
+	underlying := c.fs.GetFile(uri)
 	key := fileKey{
 		identity: underlying.Identity(),
 	}

internal/lsp/cache/external.go

@@ -27,11 +27,12 @@ type nativeFileHandle struct {
 	identity source.FileIdentity
 }
 
-func (fs *nativeFileSystem) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
+func (fs *nativeFileSystem) GetFile(uri span.URI) source.FileHandle {
 	identifier := "DOES NOT EXIST"
 	if fi, err := os.Stat(uri.Filename()); err == nil {
 		identifier = fi.ModTime().String()
 	}
+	kind := source.DetectLanguage("", uri.Filename())
 	return &nativeFileHandle{
 		fs: fs,
 		identity: source.FileIdentity{

internal/lsp/cache/file.go

@@ -41,7 +41,7 @@ func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
 	return o.text, o.hash, nil
 }
 
-func (s *session) updateOverlay(ctx context.Context, c source.FileModification) error {
+func (s *session) updateOverlay(ctx context.Context, c source.FileModification) (source.FileKind, error) {
 	s.overlayMu.Lock()
 	defer s.overlayMu.Unlock()
 
@@ -54,18 +54,18 @@ func (s *session) updateOverlay(ctx context.Context, c source.FileModification)
 		kind = source.DetectLanguage(c.LanguageID, c.URI.Filename())
 	default:
 		if !ok {
-			return errors.Errorf("updateOverlay: modifying unopened overlay %v", c.URI)
+			return -1, errors.Errorf("updateOverlay: modifying unopened overlay %v", c.URI)
 		}
 		kind = o.kind
 	}
 	if kind == source.UnknownKind {
-		return errors.Errorf("updateOverlay: unknown file kind for %s", c.URI)
+		return -1, errors.Errorf("updateOverlay: unknown file kind for %s", c.URI)
 	}
 
 	// Closing a file just deletes its overlay.
 	if c.Action == source.Close {
 		delete(s.overlays, c.URI)
-		return nil
+		return kind, nil
 	}
 
 	// If the file is on disk, check if its content is the same as the overlay.
@@ -77,15 +77,15 @@ func (s *session) updateOverlay(ctx context.Context, c source.FileModification)
 	var sameContentOnDisk bool
 	switch c.Action {
 	case source.Open:
-		_, h, err := s.cache.GetFile(c.URI, kind).Read(ctx)
+		_, h, err := s.cache.GetFile(c.URI).Read(ctx)
 		sameContentOnDisk = (err == nil && h == hash)
 	case source.Save:
 		// Make sure the version and content (if present) is the same.
 		if o.version != c.Version {
-			return errors.Errorf("updateOverlay: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
+			return -1, errors.Errorf("updateOverlay: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
 		}
 		if c.Text != nil && o.hash != hash {
-			return errors.Errorf("updateOverlay: overlay %s changed on save", c.URI)
+			return -1, errors.Errorf("updateOverlay: overlay %s changed on save", c.URI)
 		}
 		sameContentOnDisk = true
 	}
@@ -98,7 +98,7 @@ func (s *session) updateOverlay(ctx context.Context, c source.FileModification)
 		hash:              hash,
 		sameContentOnDisk: sameContentOnDisk,
 	}
-	return nil
+	return kind, nil
 }
 
 func (s *session) readOverlay(uri span.URI) *overlay {

internal/lsp/cache/overlay.go

@@ -256,7 +256,8 @@ func (s *session) DidModifyFile(ctx context.Context, c source.FileModification)
 	ctx = telemetry.URI.With(ctx, c.URI)
 
 	// Perform the session-specific updates.
-	if err := s.updateOverlay(ctx, c); err != nil {
+	kind, err := s.updateOverlay(ctx, c)
+	if err != nil {
 		return nil, err
 	}
 
@@ -265,12 +266,11 @@ func (s *session) DidModifyFile(ctx context.Context, c source.FileModification)
 		if view.Ignore(c.URI) {
 			return nil, errors.Errorf("ignored file %v", c.URI)
 		}
-		// Set the content for the file, only for didChange and didClose events.
-		f, err := view.getFileLocked(ctx, c.URI)
-		if err != nil {
+		// Make sure to add the file to the view.
+		if _, err := view.getFileLocked(ctx, c.URI); err != nil {
 			return nil, err
 		}
-		snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, f.kind, c.Action))
+		snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, kind, c.Action))
 	}
 	return snapshots, nil
 }
@@ -283,23 +283,25 @@ func (s *session) IsOpen(uri span.URI) bool {
 	return open
 }
 
-func (s *session) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
+func (s *session) GetFile(uri span.URI) source.FileHandle {
 	if overlay := s.readOverlay(uri); overlay != nil {
 		return overlay
 	}
 	// Fall back to the cache-level file system.
-	return s.cache.GetFile(uri, kind)
+	return s.cache.GetFile(uri)
 }
 
 func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action source.FileAction) bool {
 	view, err := s.viewOf(uri)
 	if err != nil {
 		return false
 	}
-	f, err := view.getFileLocked(ctx, uri)
-	if err != nil {
+	// Make sure that the file is part of the view.
+	if _, err := view.getFileLocked(ctx, uri); err != nil {
 		return false
 	}
-	view.invalidateContent(ctx, f.URI(), f.kind, action)
+	// TODO(golang/go#31553): Remove this when this issue has been resolved.
+	kind := source.DetectLanguage("", uri.Filename())
+	view.invalidateContent(ctx, uri, kind, action)
 	return true
 }

internal/lsp/cache/session.go

@@ -8,6 +8,8 @@ import (
 	"context"
 	"io"
 	"os"
+	"path/filepath"
+	"strings"
 	"sync"
 
 	"golang.org/x/tools/go/analysis"
@@ -92,7 +94,7 @@ func (s *snapshot) ModFiles(ctx context.Context) (source.FileHandle, source.File
 		return nil, nil, err
 	}
 	// Go directly to disk to get the correct FileHandle, since we just copied the file without invalidating the snapshot.
-	tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp), source.Mod)
+	tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp))
 	if tempfh == nil {
 		return nil, nil, errors.Errorf("temporary go.mod filehandle is nil")
 	}
@@ -533,9 +535,7 @@ func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.FileHandle
 	s.view.mu.Lock()
 	defer s.view.mu.Unlock()
 
-	// TODO(rstambler): Should there be a version that provides a kind explicitly?
-	kind := source.DetectLanguage("", uri.Filename())
-	f, err := s.view.getFile(ctx, uri, kind)
+	f, err := s.view.getFile(ctx, uri)
 	if err != nil {
 		return nil, err
 	}
@@ -547,7 +547,7 @@ func (s *snapshot) getFileHandle(ctx context.Context, f *fileBase) source.FileHa
 	defer s.mu.Unlock()
 
 	if _, ok := s.files[f.URI()]; !ok {
-		s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.kind)
+		s.files[f.URI()] = s.view.session.GetFile(f.URI())
 	}
 	return s.files[f.URI()]
 }
@@ -633,7 +633,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKi
 		result.files[k] = v
 	}
 	// Handle the invalidated file; it may have new contents or not exist.
-	currentFH := s.view.session.GetFile(withoutURI, withoutFileKind)
+	currentFH := s.view.session.GetFile(withoutURI)
 	if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
 		delete(result.files, withoutURI)
 	} else {
@@ -680,6 +680,10 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKi
 	return result
 }
 
+func dir(filename string) string {
+	return strings.ToLower(filepath.Dir(filename))
+}
+
 func (s *snapshot) ID() uint64 {
 	return s.id
 }