lsp/fscache: set version on token.File

Files can be loaded directly by different parts of the LSP, but also by
modpkgload. It is desirable to keep a file fully self-contained: once it
is loaded, it should ideally keep with it all its metadata. This avoids
going back to the fs to fetch further metadata later on, which could run
the risk of race conditions.

A while ago we added File.[Set]Content() so that we could easily get
from an AST back to the bytes that built it. Various messages from the
LSP server to the client require that version numbers of files are
included which helps to avoid race conditions, especially if the client
is sending async modification messages whilst a sync rpc message is in
progress.

To better support this, we add the version number to the token.File and
so once a file has been loaded (and probably parsed to an AST), we can
get back not just to its raw bytes, but also to its version number.

Also whilst we're here, improve the naming of fields in token.File.

Signed-off-by: Matthew Sackman <[email protected]>
Change-Id: Ic1736cb997d0c71375a5476ceabc85c42586293a
Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1225196
Reviewed-by: Daniel Martí <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: cuematthew

cue/token/position.go

@@ -98,7 +98,7 @@ func (p hiddenPos) Experiment() (x cueexperiment.File) {
 // NOTE: this is an internal API and may change at any time without notice.
 func (p hiddenPos) Priority() (pr layer.Priority, ok bool) {
 	if f := p.file; f != nil {
-		return f.p, f.isData
+		return f.priority, f.isData
 	}
 	return 0, false
 }
@@ -272,13 +272,14 @@ type File struct {
 	base index
 	size index // file size as provided to AddFile
 
-	// lines, infos, and content are protected by set.mutex
-	lines   []index // lines contains the offset of the first character for each line (the first entry is always 0)
-	infos   []lineInfo
-	content []byte
+	// lines, infos, content, and revision are protected by [File.mutex]
+	lines    []index // lines contains the offset of the first character for each line (the first entry is always 0)
+	infos    []lineInfo
+	content  []byte
+	revision int32
 
 	experiments *cueexperiment.File
-	p           layer.Priority
+	priority    layer.Priority
 	isData      bool
 }
 
@@ -326,7 +327,7 @@ func (f *hiddenFile) SetExperiments(experiments *cueexperiment.File) {
 // whether this file should be treated as containing data defaults, which
 // have different merging semantics from regular defaults.
 func (f *hiddenFile) SetLayer(priority int8, isData bool) {
-	f.p = layer.Priority(priority)
+	f.priority = layer.Priority(priority)
 	f.isData = isData
 }
 
@@ -466,6 +467,24 @@ func (f *hiddenFile) Content() []byte {
 	return f.content
 }
 
+// NOTE: this is an internal API and may change at any time without notice.
+//
+// SetRevision sets the file's version.
+func (f *hiddenFile) SetRevision(version int32) {
+	f.mutex.Lock()
+	f.revision = version
+	f.mutex.Unlock()
+}
+
+// NOTE: this is an internal API and may change at any time without notice.
+//
+// Revision retrieves the file's version.
+func (f *hiddenFile) Revision() int32 {
+	f.mutex.RLock()
+	defer f.mutex.RUnlock()
+	return f.revision
+}
+
 // A lineInfo object describes alternative file and line number
 // information (such as provided via a //line comment in a .go
 // file) for a given file offset.

internal/lsp/cache/rename.go

@@ -15,15 +15,11 @@
 package cache
 
 import (
-	"errors"
-	"io/fs"
-
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/literal"
 	"cuelang.org/go/cue/token"
 	"cuelang.org/go/internal/golangorgx/gopls/protocol"
 	"cuelang.org/go/internal/lsp/definitions"
-	"cuelang.org/go/internal/lsp/fscache"
 )
 
 // Rename implements the LSP Rename functionality.
@@ -41,7 +37,12 @@ func (w *Workspace) Rename(tokFile *token.File, fdfns *definitions.FileDefinitio
 		}
 	}
 
-	changes := make(map[protocol.DocumentURI][]protocol.TextEdit)
+	type versionedEdits struct {
+		edits   []protocol.TextEdit
+		version int32
+	}
+
+	changes := make(map[protocol.DocumentURI]*versionedEdits)
 	for _, target := range targets {
 		startPos := target.Pos().Position()
 		endPos := target.End().Position()
@@ -62,7 +63,14 @@ func (w *Workspace) Rename(tokFile *token.File, fdfns *definitions.FileDefinitio
 		if lit, ok := target.(*ast.BasicLit); (ok && lit.Kind == token.STRING) || !ast.IsValidIdent(name) {
 			name = literal.Label.Quote(name)
 		}
-		changes[uri] = append(changes[uri], protocol.TextEdit{
+		ve, found := changes[uri]
+		if !found {
+			ve = &versionedEdits{
+				version: targetFile.Revision(),
+			}
+			changes[uri] = ve
+		}
+		ve.edits = append(ve.edits, protocol.TextEdit{
 			Range:   r,
 			NewText: name,
 		})
@@ -73,25 +81,9 @@ func (w *Workspace) Rename(tokFile *token.File, fdfns *definitions.FileDefinitio
 	}
 
 	var docChanges []protocol.DocumentChanges
-	err := w.overlayFS.View(func(txn *fscache.ViewTxn) error {
-		for uri, edits := range changes {
-			handle, err := txn.Get(uri)
-			version := int32(0)
-			if errors.Is(err, fs.ErrNotExist) {
-				// noop
-			} else if err != nil {
-				return err
-			} else {
-				version = handle.Version()
-			}
-			docChanges = append(docChanges, protocol.TextEditsToDocumentChanges(uri, version, edits)...)
-		}
-		return nil
-	})
-	if err != nil {
-		w.debugLog(err.Error())
+	for uri, edits := range changes {
+		docChanges = append(docChanges, protocol.TextEditsToDocumentChanges(uri, edits.version, edits.edits)...)
 	}
-
 	return &protocol.WorkspaceEdit{DocumentChanges: docChanges}
 }
 

internal/lsp/fscache/fs_cache.go

@@ -149,7 +149,7 @@ func (p *cueFileParser) ReadCUE(config parser.Config) (syntax *ast.File, cfg par
 }
 
 // Version implements [FileHandle]
-func (entry *diskFileEntry) Version() int32 { return -1 }
+func (entry *diskFileEntry) Version() int32 { panic("Should never be called") }
 
 // Content implements [FileHandle]
 func (entry *diskFileEntry) Content() []byte { return slices.Clone(entry.content) }

internal/lsp/fscache/fs_overlay.go

@@ -78,6 +78,18 @@ func (entry *overlayFileEntry) open() *overlayFile {
 	}
 }
 
+// ReadCUE implements [FileHandle]
+func (entry *overlayFileEntry) ReadCUE(config parser.Config) (syntax *ast.File, cfg parser.Config, err error) {
+	syntax, cfg, err = entry.cueFileParser.ReadCUE(config)
+	if syntax != nil {
+		file := syntax.Pos().File()
+		if file != nil {
+			file.SetRevision(entry.version)
+		}
+	}
+	return
+}
+
 var _ iofs.File = (*overlayFile)(nil)
 
 type overlayFile struct {