From cf22ef03859dcea3e5d191a604baec82f00b8cd5 Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Thu, 14 Mar 2019 17:19:01 -0400
Subject: [PATCH] internal/lsp: handle undelivered diagnostics

This change adds a cache of undelivered diagnostics on the server-side.
If we fail to send a diagnostic once, we will retry the next time that
the server sends diagnostics.

Change-Id: I161dfad8ea1d2cfdcee933baed2d6872dc03b0c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/167737
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
---
 internal/lsp/diagnostics.go | 38 +++++++++++++++++++++++++++----------
 internal/lsp/server.go      | 12 ++++++++++--
 internal/lsp/source/view.go |  2 +-
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 6cdb71a423..7ec6b965e4 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -14,7 +14,7 @@ import (
 )
 
 func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content string) error {
-	if err := s.setContent(ctx, uri, []byte(content)); err != nil {
+	if err := s.view.SetContent(ctx, uri, []byte(content)); err != nil {
 		return err
 	}
 	go func() {
@@ -26,22 +26,40 @@ func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content str
 		if err != nil {
 			return // handle error?
 		}
+
+		s.undeliveredMu.Lock()
+		defer s.undeliveredMu.Unlock()
+
 		for uri, diagnostics := range reports {
-			protocolDiagnostics, err := toProtocolDiagnostics(ctx, s.view, diagnostics)
-			if err != nil {
-				continue // handle errors?
+			if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil {
+				s.undelivered[uri] = diagnostics
+				continue
 			}
-			s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
-				Diagnostics: protocolDiagnostics,
-				URI:         protocol.NewURI(uri),
-			})
+			// In case we had old, undelivered diagnostics.
+			delete(s.undelivered, uri)
+		}
+		// Anytime we compute diagnostics, make sure to also send along any
+		// undelivered ones (only for remaining URIs).
+		for uri, diagnostics := range s.undelivered {
+			s.publishDiagnostics(ctx, uri, diagnostics)
+
+			// If we fail to deliver the same diagnostics twice, just give up.
+			delete(s.undelivered, uri)
 		}
 	}()
 	return nil
 }
 
-func (s *Server) setContent(ctx context.Context, uri span.URI, content []byte) error {
-	return s.view.SetContent(ctx, uri, content)
+func (s *Server) publishDiagnostics(ctx context.Context, uri span.URI, diagnostics []source.Diagnostic) error {
+	protocolDiagnostics, err := toProtocolDiagnostics(ctx, s.view, diagnostics)
+	if err != nil {
+		return err
+	}
+	s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
+		Diagnostics: protocolDiagnostics,
+		URI:         protocol.NewURI(uri),
+	})
+	return nil
 }
 
 func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []source.Diagnostic) ([]protocol.Diagnostic, error) {
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index 3a5ea3ea56..f18debecfa 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -80,7 +80,13 @@ type Server struct {
 
 	textDocumentSyncKind protocol.TextDocumentSyncKind
 
-	view *cache.View
+	viewMu sync.Mutex
+	view   *cache.View
+
+	// undelivered is a cache of any diagnostics that the server
+	// failed to deliver for some reason.
+	undeliveredMu sync.Mutex
+	undelivered   map[span.URI][]source.Diagnostic
 }
 
 func (s *Server) Run(ctx context.Context) error {
@@ -302,7 +308,9 @@ func (s *Server) DidSave(context.Context, *protocol.DidSaveTextDocumentParams) e
 }
 
 func (s *Server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error {
-	s.setContent(ctx, span.NewURI(params.TextDocument.URI), nil)
+	if err := s.view.SetContent(ctx, span.NewURI(params.TextDocument.URI), nil); err != nil {
+		return err
+	}
 	return nil
 }
 
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 2b7d2b1804..243857f5f8 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -21,9 +21,9 @@ import (
 // package does not directly access the file system.
 type View interface {
 	Logger() xlog.Logger
+	FileSet() *token.FileSet
 	GetFile(ctx context.Context, uri span.URI) (File, error)
 	SetContent(ctx context.Context, uri span.URI, content []byte) error
-	FileSet() *token.FileSet
 }
 
 // File represents a Go source file that has been type-checked. It is the input