From 7f7817c0f927cae3d4542fc6bd2104faca6d9356 Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Tue, 19 Nov 2019 16:59:25 -0500
Subject: [PATCH] internal/lsp: handle breakage caused by CL 207598

textDocument/didChange events need to indicate if the change includes
the full file content or just a diff. Previously, the
contentChange.Range field was a pointer, so if it was nil, then we would
conclude that the file change was for the whole file. Now, the best we
can do is compare it to an empty range, but this still doesn't work if
you are at the beginning of a file. I think that the range needs to be a
pointer for this to work correctly.

Also, some minor changes that came up along the way while debugging:
(1) Don't close over the *cache variable for fear of pinning anything in
    memory
(2) Improve the error message when the token.File is nil
(3) Check for a nil token.File earlier

Change-Id: If9f310e92b7fb740b45e6cd3f9ca678a6fb52ff6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207906
Reviewed-by: Heschi Kreinick <heschi@google.com>
---
 internal/lsp/cache/parse.go          | 22 +++++++++++-----------
 internal/lsp/text_synchronization.go |  4 +++-
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 77a4e16870..417d3acc21 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -52,9 +52,10 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc
 		file: fh.Identity(),
 		mode: mode,
 	}
+	fset := c.fset
 	h := c.store.Bind(key, func(ctx context.Context) interface{} {
 		data := &parseGoData{}
-		data.ast, data.mapper, data.parseError, data.err = parseGo(ctx, c, fh, mode)
+		data.ast, data.mapper, data.parseError, data.err = parseGo(ctx, fset, fh, mode)
 		return data
 	})
 	return &parseGoHandle{
@@ -105,7 +106,7 @@ func hashParseKeys(phs []source.ParseGoHandle) string {
 	return hashContents(b.Bytes())
 }
 
-func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (file *ast.File, mapper *protocol.ColumnMapper, parseError error, err error) {
+func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mode source.ParseMode) (file *ast.File, mapper *protocol.ColumnMapper, parseError error, err error) {
 	ctx, done := trace.StartSpan(ctx, "cache.parseGo", telemetry.File.Of(fh.Identity().URI.Filename()))
 	defer done()
 
@@ -119,18 +120,21 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa
 	if mode == source.ParseHeader {
 		parserMode = parser.ImportsOnly | parser.ParseComments
 	}
-	file, parseError = parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode)
+	file, parseError = parser.ParseFile(fset, fh.Identity().URI.Filename(), buf, parserMode)
+	var tok *token.File
 	if file != nil {
+		// Fix any badly parsed parts of the AST.
+		tok = fset.File(file.Pos())
+		if tok == nil {
+			return nil, nil, nil, errors.Errorf("successfully parsed but no token.File for %s (%v)", fh.Identity().URI, parseError)
+		}
 		if mode == source.ParseExported {
 			trimAST(file)
 		}
-		// Fix any badly parsed parts of the AST.
-		tok := c.fset.File(file.Pos())
 		if err := fix(ctx, file, tok, buf); err != nil {
 			log.Error(ctx, "failed to fix AST", err)
 		}
 	}
-
 	if file == nil {
 		// If the file is nil only due to parse errors,
 		// the parse errors are the actual errors.
@@ -140,10 +144,6 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa
 		}
 		return nil, nil, parseError, err
 	}
-	tok := c.FileSet().File(file.Pos())
-	if tok == nil {
-		return nil, nil, parseError, errors.Errorf("no token.File for %s", fh.Identity().URI)
-	}
 	uri := fh.Identity().URI
 	content, _, err := fh.Read(ctx)
 	if err != nil {
@@ -151,7 +151,7 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa
 	}
 	m := &protocol.ColumnMapper{
 		URI:       uri,
-		Converter: span.NewTokenConverter(c.FileSet(), tok),
+		Converter: span.NewTokenConverter(fset, tok),
 		Content:   content,
 	}
 	return file, m, parseError, nil
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index b3b085900f..0a31b78e64 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -96,9 +96,11 @@ func fullChange(changes []protocol.TextDocumentContentChangeEvent) (string, bool
 		return "", false
 	}
 	// The length of the changes must be 1 at this point.
-	if changes[0].RangeLength == 0 { // used to check changes[0].Range == nil
+	// TODO: This breaks if you insert a character at the beginning of the file.
+	if (changes[0].Range == protocol.Range{} && changes[0].RangeLength == 0) {
 		return changes[0].Text, true
 	}
+
 	return "", false
 }