gopls/internal/lsp: remove now redundant debouncing logic

Now that we cancel the debouncing operation on cancellation of the
background context, debouncing is actually unnecessary. The cancellation of
earlier background contexts occurs whenever a new snapshot is created
for the view, has the same effect as debouncing (but better, because
cancellation occurs immediately and not when we reach the call to
debounce).

Also, simplify the control flow of diagnoseSnapshot.

Change-Id: I04e076d759e8a0a15238a219eb683c0d9a239677
Reviewed-on: https://go-review.googlesource.com/c/tools/+/487138
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: findleyr

gopls/internal/lsp/debounce.go

@@ -181,24 +181,17 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot, changedURIs []span.U
 		// does not analyze) packages directly affected by
 		// file modifications.
 		//
-		// The second phase runs analysis on the entire snapshot,
-		// and is debounced by the configured delay.
+		// The second phase runs after the delay, and does everything.
 		s.diagnoseChangedFiles(ctx, snapshot, changedURIs, onDisk)
 		s.publishDiagnostics(ctx, false, snapshot)
 
-		// We debounce diagnostics separately for each view, using the snapshot
-		// local ID as logical ordering.
-		//
-		// TODO(rfindley): it would be cleaner to simply put the diagnostic
-		// debouncer on the view, and remove the "key" argument to debouncing.
-		if ok := <-s.diagDebouncer.debounce(ctx, snapshot.View().ID(), uint64(snapshot.GlobalID()), time.After(delay)); ok {
-			s.diagnose(ctx, snapshot, analyzeOpenPackages)
-			s.publishDiagnostics(ctx, true, snapshot)
+		select {
+		case <-time.After(delay):
+		case <-ctx.Done():
+			return
 		}
-		return
 	}
 
-	// Ignore possible workspace configuration warnings in the normal flow.
 	s.diagnose(ctx, snapshot, analyzeOpenPackages)
 	s.publishDiagnostics(ctx, true, snapshot)
 }

gopls/internal/lsp/debounce_test.go

@@ -35,7 +35,6 @@ func NewServer(session *cache.Session, client protocol.ClientCloser) *Server {
 		client:                client,
 		diagnosticsSema:       make(chan struct{}, concurrentAnalyses),
 		progress:              progress.NewTracker(client),
-		diagDebouncer:         newDebouncer(),
 	}
 }
 
@@ -105,9 +104,6 @@ type Server struct {
 
 	progress *progress.Tracker
 
-	// diagDebouncer is used for debouncing diagnostics.
-	diagDebouncer *debouncer
-
 	// When the workspace fails to load, we show its status through a progress
 	// report with an error message.
 	criticalErrorStatusMu sync.Mutex

gopls/internal/lsp/diagnostics.go

@@ -647,7 +647,7 @@ use (
 		}
 
 		// This fails if guarded with a OnceMet(DoneWithSave(), ...), because it is
-		// debounced (and therefore not synchronous with the change).
+		// delayed (and therefore not synchronous with the change).
 		env.Await(NoDiagnostics(ForFile("modb/go.mod")))
 
 		// Test Formatting.