From f986e617de62cf1fa310e3138a4664499bb3089a Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Mon, 13 Jan 2020 20:41:03 -0500
Subject: [PATCH] internal/lsp: make sure diagnostics with analyses are not
 overwritten

The initial workspace load does not send analyses with diagnostics, but
subsequent diagnostic requests do. If we've already sent diagnostics
with analysis for a file at a given version and snapshot, do not resend
diagnostics for the same file version with analyses.

Change-Id: I6979aa308e8846ff0cb66bd23e0dac488eae5446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214587
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 | 25 +++++++++++++++++--------
 internal/lsp/server.go      |  8 +++++---
 internal/lsp/source/view.go |  2 ++
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 07f05ee9ba..c3ce41cb30 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -32,7 +32,7 @@ func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot)
 				log.Error(ctx, "diagnoseSnapshot: no diagnostics", err, telemetry.Package.Of(ph.ID()))
 				return
 			}
-			s.publishReports(ctx, snapshot, reports, false)
+			s.publishReports(ctx, snapshot, reports, false, false)
 		}(ph)
 	}
 	// Run diagnostics on the go.mod file.
@@ -61,7 +61,7 @@ func (s *Server) diagnoseFile(snapshot source.Snapshot, fh source.FileHandle) {
 		return
 	}
 	// Publish empty diagnostics for files.
-	s.publishReports(ctx, snapshot, reports, true)
+	s.publishReports(ctx, snapshot, reports, true, true)
 }
 
 func (s *Server) diagnoseModfile(snapshot source.Snapshot) {
@@ -77,10 +77,10 @@ func (s *Server) diagnoseModfile(snapshot source.Snapshot) {
 		return
 	}
 	// Publish empty diagnostics for files.
-	s.publishReports(ctx, snapshot, reports, true)
+	s.publishReports(ctx, snapshot, reports, true, true)
 }
 
-func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[source.FileIdentity][]source.Diagnostic, publishEmpty bool) {
+func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[source.FileIdentity][]source.Diagnostic, publishEmpty, withAnalysis bool) {
 	// Check for context cancellation before publishing diagnostics.
 	if ctx.Err() != nil {
 		return
@@ -98,11 +98,14 @@ func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, r
 		// Pre-sort diagnostics to avoid extra work when we compare them.
 		source.SortDiagnostics(diagnostics)
 		toSend := sentDiagnostics{
-			version:    fileID.Version,
-			identifier: fileID.Identifier,
-			sorted:     diagnostics,
+			version:      fileID.Version,
+			identifier:   fileID.Identifier,
+			sorted:       diagnostics,
+			withAnalysis: withAnalysis,
+			snapshotID:   snapshot.ID(),
 		}
 		delivered, ok := s.delivered[fileID.URI]
+
 		// If diagnostics are empty and not previously delivered,
 		// only send them if we are publishing empty diagnostics.
 		if !ok && len(diagnostics) == 0 && !publishEmpty {
@@ -115,11 +118,17 @@ func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, r
 		if ok {
 			geqVersion := fileID.Version >= delivered.version && delivered.version > 0
 			noVersions := (fileID.Version == 0 && delivered.version == 0) && delivered.identifier == fileID.Identifier
-			if (geqVersion || noVersions) && equalDiagnostics(delivered.sorted, diagnostics) {
+			if (geqVersion || noVersions) && equalDiagnostics(delivered.sorted, toSend.sorted) {
 				// Update the delivered map even if we reuse cached diagnostics.
 				s.delivered[fileID.URI] = toSend
 				continue
 			}
+			// If we've already delivered diagnostics with analyses for this file, for this snapshot,
+			// at this version, do not send diagnostics without analyses.
+			if delivered.snapshotID == toSend.snapshotID && delivered.version == toSend.version &&
+				delivered.withAnalysis && !toSend.withAnalysis {
+				continue
+			}
 		}
 		if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
 			Diagnostics: toProtocolDiagnostics(ctx, diagnostics),
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index ab8f208123..7c94c45a72 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -96,9 +96,11 @@ type Server struct {
 
 // sentDiagnostics is used to cache diagnostics that have been sent for a given file.
 type sentDiagnostics struct {
-	version    float64
-	identifier string
-	sorted     []source.Diagnostic
+	version      float64
+	identifier   string
+	sorted       []source.Diagnostic
+	withAnalysis bool
+	snapshotID   uint64
 }
 
 // General
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index cb6995b46c..789ac9980f 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -21,6 +21,8 @@ import (
 
 // Snapshot represents the current state for the given view.
 type Snapshot interface {
+	ID() uint64
+
 	// View returns the View associated with this snapshot.
 	View() View