internal/lsp: return errors from diagnostics

Also, we were repeating the same code for diagnostics in 4 places.
Extract into a function.

Change-Id: I18d00e512a67679b915347a61126970cf02a7a4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196019
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

@@ -13,24 +13,28 @@ import (
 	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
+	"golang.org/x/tools/internal/telemetry/trace"
+	errors "golang.org/x/xerrors"
 )
 
-func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI) {
+func (s *Server) diagnostics(view source.View, uri span.URI) error {
+	ctx := view.BackgroundContext()
+	ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
+	defer done()
+
 	ctx = telemetry.File.With(ctx, uri)
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
-		log.Error(ctx, "no file", err, telemetry.File)
+		return err
-		return
 	}
 	// For non-Go files, don't return any diagnostics.
 	gof, ok := f.(source.GoFile)
 	if !ok {
-		return
+		return errors.Errorf("%s is not a Go file", f.URI())
 	}
 	reports, err := source.Diagnostics(ctx, view, gof, view.Options().DisabledAnalyses)
 	if err != nil {
-		log.Error(ctx, "failed to compute diagnostics", err, telemetry.File)
+		return err
-		return
 	}
 
 	s.undeliveredMu.Lock()
@@ -57,6 +61,7 @@ func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI
 		// If we fail to deliver the same diagnostics twice, just give up.
 		delete(s.undelivered, uri)
 	}
+	return nil
 }
 
 func (s *Server) publishDiagnostics(ctx context.Context, uri span.URI, diagnostics []source.Diagnostic) error {

internal/lsp/text_synchronization.go

@@ -15,7 +15,6 @@ import (
 	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
-	"golang.org/x/tools/internal/telemetry/trace"
 	errors "golang.org/x/xerrors"
 )
 
@@ -32,12 +31,8 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume
 	view := s.session.ViewOf(uri)
 
 	// Run diagnostics on the newly-changed file.
-	go func() {
+	go s.diagnostics(view, uri)
-		ctx := view.BackgroundContext()
+
-		ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
-		defer done()
-		s.Diagnostics(ctx, view, uri)
-	}()
 	return nil
 }
 
@@ -84,12 +79,8 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
 	}
 
 	// Run diagnostics on the newly-changed file.
-	go func() {
+	go s.diagnostics(view, uri)
-		ctx := view.BackgroundContext()
+
-		ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
-		defer done()
-		s.Diagnostics(ctx, view, uri)
-	}()
 	return nil
 }
 

internal/lsp/watched_files.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
-	"golang.org/x/tools/internal/telemetry/trace"
 )
 
 func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
@@ -48,12 +47,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
 
 				// Refresh diagnostics to reflect updated file contents.
-				go func(view source.View) {
+				go s.diagnostics(view, uri)
-					ctx := view.BackgroundContext()
-					ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
-					defer done()
-					s.Diagnostics(ctx, view, uri)
-				}(view)
 			case protocol.Created:
 				log.Print(ctx, "watched file created", telemetry.File)
 			case protocol.Deleted:
@@ -85,18 +79,14 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 				}
 				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
 
-				if otherFile != nil {
+				// If this was the only file in the package, clear its diagnostics.
-					// Refresh diagnostics to reflect updated file contents.
+				if otherFile == nil {
-					go func(view source.View) {
+					if err := s.publishDiagnostics(ctx, uri, []source.Diagnostic{}); err != nil {
-						ctx := view.BackgroundContext()
+						log.Error(ctx, "failed to clear diagnostics", err, telemetry.URI.Of(uri))
-						ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
+					}
-						defer done()
+					return nil
-						s.Diagnostics(ctx, view, otherFile.URI())
-					}(view)
-				} else {
-					// TODO: Handle case when there is no other file (i.e. deleted
-					//       file was the only file in the package).
 				}
+				go s.diagnostics(view, uri)
 			}
 		}
 	}