internal/lsp: don't run full workspace diagnostics on mod file change

Minimize the issues at master by not running workspace-level diagnostics
on mod file changes. Once the initial workspace load stabilizes we will
be able to go back to that approach.

Also, a couple of minor changes along the way while debugging.

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

internal/lsp/cache/analysis.go

@@ -206,7 +206,7 @@ func runAnalysis(ctx context.Context, fset *token.FileSet, analyzer *analysis.An
 	defer func() {
 		if r := recover(); r != nil {
 			log.Print(ctx, fmt.Sprintf("analysis panicked: %s", r), telemetry.Package.Of(pkg.PkgPath))
-			data.err = errors.Errorf("analysis %s for package %s panicked: %v", analyzer.Name, pkg.PkgPath())
+			data.err = errors.Errorf("analysis %s for package %s panicked: %v", analyzer.Name, pkg.PkgPath(), r)
 		}
 	}()
 

internal/lsp/cache/snapshot.go

@@ -524,10 +524,7 @@ func (s *snapshot) getFileURIs() []span.URI {
 
 // FindFile returns the file if the given URI is already a part of the view.
 func (s *snapshot) FindFile(ctx context.Context, uri span.URI) source.FileHandle {
-	s.view.mu.Lock()
-	defer s.view.mu.Unlock()
-
-	f, err := s.view.findFile(uri)
+	f, err := s.view.findFileLocked(ctx, uri)
 	if f == nil || err != nil {
 		return nil
 	}
@@ -537,10 +534,7 @@ func (s *snapshot) FindFile(ctx context.Context, uri span.URI) source.FileHandle
 // GetFile returns a File for the given URI. It will always succeed because it
 // adds the file to the managed set if needed.
 func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.FileHandle, error) {
-	s.view.mu.Lock()
-	defer s.view.mu.Unlock()
-
-	f, err := s.view.getFile(ctx, uri)
+	f, err := s.view.getFileLocked(ctx, uri)
 	if err != nil {
 		return nil, err
 	}

internal/lsp/cache/view.go

@@ -344,15 +344,11 @@ func basename(filename string) string {
 }
 
 // FindFile returns the file if the given URI is already a part of the view.
-func (v *view) findFileLocked(ctx context.Context, uri span.URI) *fileBase {
+func (v *view) findFileLocked(ctx context.Context, uri span.URI) (*fileBase, error) {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	f, err := v.findFile(uri)
-	if err != nil {
-		return nil
-	}
-	return f
+	return v.findFile(uri)
 }
 
 // getFileLocked returns a File for the given URI. It will always succeed because it

internal/lsp/diagnostics.go

@@ -16,17 +16,6 @@ import (
 	"golang.org/x/tools/internal/telemetry/trace"
 )
 
-func (s *Server) diagnose(snapshot source.Snapshot, fh source.FileHandle) error {
-	switch fh.Identity().Kind {
-	case source.Go:
-		go s.diagnoseFile(snapshot, fh)
-	case source.Mod:
-		ctx := snapshot.View().BackgroundContext()
-		go s.diagnoseSnapshot(ctx, snapshot)
-	}
-	return nil
-}
-
 func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot) {
 	ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
 	defer done()

internal/lsp/text_synchronization.go

@@ -37,8 +37,12 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume
 	if err != nil {
 		return err
 	}
-	// Always run diagnostics when a file is opened.
-	return s.diagnose(snapshot, fh)
+	// Only run diagnostics on file open for Go files.
+	switch fh.Identity().Kind {
+	case source.Go:
+		go s.diagnoseFile(snapshot, fh)
+	}
+	return nil
 }
 
 func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error {
@@ -72,8 +76,15 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
 	if err != nil {
 		return err
 	}
-	// Always update diagnostics after a file change.
-	return s.diagnose(snapshot, fh)
+	// Run diagnostics for Go files and for mod files.
+	switch fh.Identity().Kind {
+	case source.Go:
+		go s.diagnoseFile(snapshot, fh)
+	case source.Mod:
+		// TODO(rstambler): Modifying the go.mod file should trigger workspace-level diagnostics.
+		go s.diagnoseModfile(snapshot)
+	}
+	return nil
 }
 
 func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error {

internal/lsp/watched_files.go

@@ -36,7 +36,14 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 					if err != nil {
 						return err
 					}
-					return s.diagnose(snapshot, fh)
+					switch fh.Identity().Kind {
+					case source.Go:
+						go s.diagnoseFile(snapshot, fh)
+					case source.Mod:
+						go s.diagnoseModfile(snapshot)
+					}
+
+					return nil
 				}
 			case source.Delete:
 				snapshot := view.Snapshot()