From 43ed94694c2bc8f0fb9d61264e673fdce49be8a0 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 16 Jul 2020 01:15:38 -0400 Subject: [PATCH] internal/lsp: don't keep track of closed overlays getFile still returns a FileHandle, even if the file doesn't exist on disk. Work-around this by checking if the file exists before adding the file handle to the map. Fixes golang/go#38498 Change-Id: Ie02679068b37bf4f3d19966c6cfcf2361086b1de Reviewed-on: https://go-review.googlesource.com/c/tools/+/242924 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/cache.go | 4 ++++ internal/lsp/regtest/diagnostics_test.go | 30 ++++++++++++++++++++++++ internal/lsp/source/view.go | 3 +++ internal/lsp/text_synchronization.go | 5 +++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go index be6cd91695..64757519ca 100644 --- a/internal/lsp/cache/cache.go +++ b/internal/lsp/cache/cache.go @@ -60,6 +60,10 @@ type fileHandle struct { err error } +func (c *Cache) GetFile(ctx context.Context, uri span.URI) (source.FileHandle, error) { + return c.getFile(ctx, uri) +} + func (c *Cache) getFile(ctx context.Context, uri span.URI) (*fileHandle, error) { var modTime time.Time if fi, err := os.Stat(uri.Filename()); err == nil { diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go index 13499fb2b1..fafee46c1b 100644 --- a/internal/lsp/regtest/diagnostics_test.go +++ b/internal/lsp/regtest/diagnostics_test.go @@ -211,6 +211,7 @@ func TestRmTest38878Close(t *testing.T) { env.Await(EmptyDiagnostics("a_test.go")) }) } + func TestRmTest38878(t *testing.T) { log.SetFlags(log.Lshortfile) runner.Run(t, test38878, func(t *testing.T, env *Env) { @@ -1009,3 +1010,32 @@ func main() { ) }) } + +func TestClosingBuffer(t *testing.T) { + const basic = ` +-- go.mod -- +module mod.com + +go 1.14 +-- main.go -- +package main + +func main() {} +` + runner.Run(t, basic, func(t *testing.T, env *Env) { + env.Await( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromInitialWorkspaceLoad), 1), + ) + env.Editor.OpenFileWithContent(env.Ctx, "foo.go", `package main`) + env.Await( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1), + ) + env.CloseBuffer("foo.go") + env.Await( + OnceMet( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidClose), 1), + NoLogMatching(protocol.Info, "packages=0"), + ), + ) + }) +} diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index fb6e3f5b38..0057fda7f0 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -285,6 +285,9 @@ type Cache interface { // FileSet returns the shared fileset used by all files in the system. FileSet() *token.FileSet + // GetFile returns a file handle for the given URI. + GetFile(ctx context.Context, uri span.URI) (FileHandle, error) + // ParseGoHandle returns a ParseGoHandle for the given file handle. ParseGoHandle(ctx context.Context, fh FileHandle, mode ParseMode) ParseGoHandle } diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 2c7318fcbd..7fc4161a97 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -237,7 +237,10 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu if snapshot == nil { return errors.Errorf("no snapshot for %s", uri) } - fh, err := snapshot.GetFile(ctx, uri) + // Check if the file exists on disk after it has been closed. Calling + // snapshot.GetFile will add it back to the snapshot even if it doesn't + // exist, which will cause problems. + fh, err := snapshot.View().Session().Cache().GetFile(ctx, uri) if err != nil { return err }