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>
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 77a4e16..417d3ac 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -52,9 +52,10 @@
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 @@
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 @@
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 @@
}
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 @@
}
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 b3b0859..0a31b78 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -96,9 +96,11 @@
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
}
