internal/lsp: don't resend diagnostics if they are unchanged

Cache delivered diagnostics on the server so that we can determine if
they should be resent. To be careful about this, we only reuse cached
diagnostics if they are for a greater version, or if we don't know
the file's version and it is unchanged.

Fixes golang/go#32443

Change-Id: I4ba22d85e5b21a8ad6cc62f74cd83c07d3c220cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208261
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/diagnostics.go

@@ -90,6 +90,9 @@ func (s *Server) publishReports(ctx context.Context, reports map[source.FileIden
 		return
 	}
 
+	s.deliveredMu.Lock()
+	defer s.deliveredMu.Unlock()
+
 	for fileID, diagnostics := range reports {
 		// Don't deliver diagnostics if the context has already been canceled.
 		if ctx.Err() != nil {
@@ -99,6 +102,26 @@ func (s *Server) publishReports(ctx context.Context, reports map[source.FileIden
 		if len(diagnostics) == 0 && !publishEmpty {
 			continue
 		}
+		// Pre-sort diagnostics to avoid extra work when we compare them.
+		source.SortDiagnostics(diagnostics)
+		toSend := sentDiagnostics{
+			version:    fileID.Version,
+			identifier: fileID.Identifier,
+			sorted:     diagnostics,
+		}
+
+		if delivered, ok := s.delivered[fileID.URI]; ok {
+			// We only reuse cached diagnostics in two cases:
+			//   1. This file is at a greater version than that of the previously sent diagnostics.
+			//   2. There are no known versions for the file.
+			greaterVersion := fileID.Version > delivered.version && delivered.version > 0
+			noVersions := (fileID.Version == 0 && delivered.version == 0) && delivered.identifier == fileID.Identifier
+			if (greaterVersion || noVersions) && equalDiagnostics(delivered.sorted, diagnostics) {
+				// Update the delivered map even if we reuse cached diagnostics.
+				s.delivered[fileID.URI] = toSend
+				continue
+			}
+		}
 		if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
 			Diagnostics: toProtocolDiagnostics(ctx, diagnostics),
 			URI:         protocol.NewURI(fileID.URI),
@@ -107,9 +130,25 @@ func (s *Server) publishReports(ctx context.Context, reports map[source.FileIden
 			log.Error(ctx, "failed to deliver diagnostic", err, telemetry.File)
 			continue
 		}
+		// Update the delivered map.
+		s.delivered[fileID.URI] = toSend
 	}
 }
 
+// equalDiagnostics returns true if the 2 lists of diagnostics are equal.
+// It assumes that both a and b are already sorted.
+func equalDiagnostics(a, b []source.Diagnostic) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := 0; i < len(a); i++ {
+		if source.CompareDiagnostic(a[i], b[i]) != 0 {
+			return false
+		}
+	}
+	return true
+}
+
 func toProtocolDiagnostics(ctx context.Context, diagnostics []source.Diagnostic) []protocol.Diagnostic {
 	reports := []protocol.Diagnostic{}
 	for _, diag := range diagnostics {

internal/lsp/lsp_test.go

@@ -73,7 +73,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	}
 	r := &runner{
 		server: &Server{
-			session: session,
+			session:   session,
+			delivered: map[span.URI]sentDiagnostics{},
 		},
 		data: data,
 		ctx:  ctx,

internal/lsp/server.go

@@ -21,15 +21,18 @@ import (
 func NewClientServer(ctx context.Context, cache source.Cache, client protocol.Client) (context.Context, *Server) {
 	ctx = protocol.WithClient(ctx, client)
 	return ctx, &Server{
-		client:  client,
-		session: cache.NewSession(ctx),
+		client:    client,
+		session:   cache.NewSession(ctx),
+		delivered: make(map[span.URI]sentDiagnostics),
 	}
 }
 
 // NewServer starts an LSP server on the supplied stream, and waits until the
 // stream is closed.
 func NewServer(ctx context.Context, cache source.Cache, stream jsonrpc2.Stream) (context.Context, *Server) {
-	s := &Server{}
+	s := &Server{
+		delivered: make(map[span.URI]sentDiagnostics),
+	}
 	ctx, s.Conn, s.client = protocol.NewServer(ctx, stream, s)
 	s.session = cache.NewSession(ctx)
 	return ctx, s
@@ -85,6 +88,17 @@ type Server struct {
 	// folders is only valid between initialize and initialized, and holds the
 	// set of folders to build views for when we are ready
 	pendingFolders []protocol.WorkspaceFolder
+
+	// delivered is a cache of the diagnostics that the server has sent.
+	deliveredMu sync.Mutex
+	delivered   map[span.URI]sentDiagnostics
+}
+
+// sentDiagnostics is used to cache diagnostics that have been sent for a given file.
+type sentDiagnostics struct {
+	version    float64
+	identifier string
+	sorted     []source.Diagnostic
 }
 
 // General

internal/lsp/source/util.go

@@ -13,6 +13,7 @@ import (
 	"go/types"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strings"
 
 	"golang.org/x/tools/internal/lsp/protocol"
@@ -590,3 +591,22 @@ func formatFunction(params []string, results []string, writeResultParens bool) s
 
 	return detail.String()
 }
+
+func SortDiagnostics(d []Diagnostic) {
+	sort.Slice(d, func(i int, j int) bool {
+		return CompareDiagnostic(d[i], d[j]) < 0
+	})
+}
+
+func CompareDiagnostic(a, b Diagnostic) int {
+	if r := protocol.CompareRange(a.Range, b.Range); r != 0 {
+		return r
+	}
+	if a.Message < b.Message {
+		return -1
+	}
+	if a.Message == b.Message {
+		return 0
+	}
+	return 1
+}

internal/lsp/tests/diagnostics.go

@@ -3,7 +3,6 @@ package tests
 import (
 	"bytes"
 	"fmt"
-	"sort"
 	"strings"
 
 	"golang.org/x/tools/internal/lsp/protocol"
@@ -14,8 +13,8 @@ import (
 // DiffDiagnostics prints the diff between expected and actual diagnostics test
 // results.
 func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string {
-	sortDiagnostics(want)
-	sortDiagnostics(got)
+	source.SortDiagnostics(want)
+	source.SortDiagnostics(got)
 
 	if len(got) != len(want) {
 		return summarizeDiagnostics(-1, uri, want, got, "different lengths got %v want %v", len(got), len(want))
@@ -47,26 +46,6 @@ func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string {
 	return ""
 }
 
-func sortDiagnostics(d []source.Diagnostic) {
-	sort.Slice(d, func(i int, j int) bool {
-		return compareDiagnostic(d[i], d[j]) < 0
-	})
-}
-
-func compareDiagnostic(a, b source.Diagnostic) int {
-	if r := protocol.CompareRange(a.Range, b.Range); r != 0 {
-		return r
-	}
-	if a.Message < b.Message {
-		return -1
-	}
-	if a.Message == b.Message {
-		return 0
-	} else {
-		return 1
-	}
-}
-
 func summarizeDiagnostics(i int, uri span.URI, want []source.Diagnostic, got []source.Diagnostic, reason string, args ...interface{}) string {
 	msg := &bytes.Buffer{}
 	fmt.Fprint(msg, "diagnostics failed")