internal/lsp: fix panic in bestView

Rather than panicking when we have not created any views for the packages,
we should show a reasonable error to the user. This change propagates the
errors to the user.

Updates golang/go#35599

Change-Id: I49789d8ce18e154f111bc3584488f468a129e30c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207344
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>

internal/lsp/cache/session.go

@@ -169,18 +169,21 @@ func (s *session) View(name string) source.View {
 
 // ViewOf returns a view corresponding to the given URI.
 // If the file is not already associated with a view, pick one using some heuristics.
-func (s *session) ViewOf(uri span.URI) source.View {
+func (s *session) ViewOf(uri span.URI) (source.View, error) {
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
 
 	// Check if we already know this file.
 	if v, found := s.viewMap[uri]; found {
-		return v
+		return v, nil
 	}
 	// Pick the best view for this file and memoize the result.
-	v := s.bestView(uri)
+	v, err := s.bestView(uri)
+	if err != nil {
+		return nil, err
+	}
 	s.viewMap[uri] = v
-	return v
+	return v, nil
 }
 
 func (s *session) viewsOf(uri span.URI) []*view {
@@ -208,7 +211,10 @@ func (s *session) Views() []source.View {
 
 // bestView finds the best view to associate a given URI with.
 // viewMu must be held when calling this method.
-func (s *session) bestView(uri span.URI) source.View {
+func (s *session) bestView(uri span.URI) (source.View, error) {
+	if len(s.views) == 0 {
+		return nil, errors.Errorf("no views in the session")
+	}
 	// we need to find the best view for this file
 	var longest source.View
 	for _, view := range s.views {
@@ -220,10 +226,10 @@ func (s *session) bestView(uri span.URI) source.View {
 		}
 	}
 	if longest != nil {
-		return longest
+		return longest, nil
 	}
 	// TODO: are there any more heuristics we can use?
-	return s.views[0]
+	return s.views[0], nil
 }
 
 func (s *session) removeView(ctx context.Context, view *view) error {
@@ -291,7 +297,10 @@ func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKin
 	}
 
 	// Make sure that the file gets added to the session's file watch map.
-	view := s.bestView(uri)
+	view, err := s.bestView(uri)
+	if err != nil {
+		return err
+	}
 	if _, err := view.GetFile(ctx, uri); err != nil {
 		return err
 	}

internal/lsp/code_action.go

@@ -21,7 +21,10 @@ import (
 
 func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/command.go

@@ -17,7 +17,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 		}
 		// Confirm that this action is being taken on a go.mod file.
 		uri := span.NewURI(params.Arguments[0].(string))
-		view := s.session.ViewOf(uri)
+		view, err := s.session.ViewOf(uri)
+		if err != nil {
+			return nil, err
+		}
 		f, err := view.GetFile(ctx, uri)
 		if err != nil {
 			return nil, err

internal/lsp/completion.go

@@ -18,7 +18,10 @@ import (
 
 func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	options := view.Options()
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {

internal/lsp/completion_test.go

@@ -120,12 +120,15 @@ func expected(t *testing.T, test tests.Completion, items tests.CompletionItems)
 func (r *runner) callCompletion(t *testing.T, src span.Span, options source.CompletionOptions) []protocol.CompletionItem {
 	t.Helper()
 
-	view := r.server.session.ViewOf(src.URI())
+	view, err := r.server.session.ViewOf(src.URI())
+	if err != nil {
+		t.Fatal(err)
+	}
 	original := view.Options()
 	modified := original
 	modified.InsertTextFormat = protocol.SnippetTextFormat
 	modified.Completion = options
-	view, err := view.SetOptions(r.ctx, modified)
+	view, err = view.SetOptions(r.ctx, modified)
 	if err != nil {
 		t.Error(err)
 		return nil

internal/lsp/definition.go

@@ -14,7 +14,10 @@ import (
 
 func (s *Server) definition(ctx context.Context, params *protocol.DefinitionParams) ([]protocol.Location, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err
@@ -37,7 +40,10 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara
 
 func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefinitionParams) ([]protocol.Location, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/folding_range.go

@@ -10,7 +10,10 @@ import (
 
 func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRangeParams) ([]protocol.FoldingRange, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/format.go

@@ -14,7 +14,10 @@ import (
 
 func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormattingParams) ([]protocol.TextEdit, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/general.go

@@ -166,11 +166,23 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 	debug.PrintVersionInfo(buf, true, debug.PlainText)
 	log.Print(ctx, buf.String())
 
+	viewErrors := make(map[span.URI]error)
 	for _, folder := range s.pendingFolders {
+		uri := span.NewURI(folder.URI)
 		if _, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
-			return err
+			viewErrors[uri] = err
 		}
 	}
+	if len(viewErrors) > 0 {
+		errMsg := fmt.Sprintf("Error loading workspace folders (expected %v, got %v)\n", len(s.pendingFolders), len(s.session.Views()))
+		for uri, err := range viewErrors {
+			errMsg += fmt.Sprintf("failed to load view for %s: %v\n", uri, err)
+		}
+		s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+			Type:    protocol.Error,
+			Message: errMsg,
+		})
+	}
 	s.pendingFolders = nil
 
 	return nil

internal/lsp/highlight.go

@@ -16,7 +16,10 @@ import (
 
 func (s *Server) documentHighlight(ctx context.Context, params *protocol.DocumentHighlightParams) ([]protocol.DocumentHighlight, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	rngs, err := source.Highlight(ctx, view, uri, params.Position)
 	if err != nil {
 		log.Error(ctx, "no highlight", err, telemetry.URI.Of(uri))

internal/lsp/hover.go

@@ -17,7 +17,10 @@ import (
 
 func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*protocol.Hover, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/implementation.go

@@ -14,7 +14,10 @@ import (
 
 func (s *Server) implementation(ctx context.Context, params *protocol.ImplementationParams) ([]protocol.Location, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/link.go

@@ -22,7 +22,10 @@ import (
 
 func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/lsp_test.go

@@ -98,13 +98,16 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
 
 func (r *runner) FoldingRanges(t *testing.T, spn span.Span) {
 	uri := spn.URI()
-	view := r.server.session.ViewOf(uri)
+	view, err := r.server.session.ViewOf(uri)
+	if err != nil {
+		t.Fatal(err)
+	}
 	original := view.Options()
 	modified := original
 
 	// Test all folding ranges.
 	modified.LineFoldingOnly = false
-	view, err := view.SetOptions(r.ctx, modified)
+	view, err = view.SetOptions(r.ctx, modified)
 	if err != nil {
 		t.Error(err)
 		return
@@ -326,7 +329,10 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
 func (r *runner) SuggestedFix(t *testing.T, spn span.Span) {
 	uri := spn.URI()
 	filename := uri.Filename()
-	view := r.server.session.ViewOf(uri)
+	view, err := r.server.session.ViewOf(uri)
+	if err != nil {
+		t.Fatal(err)
+	}
 	f, err := view.GetFile(r.ctx, uri)
 	if err != nil {
 		t.Fatal(err)

internal/lsp/references.go

@@ -16,7 +16,10 @@ import (
 
 func (s *Server) references(ctx context.Context, params *protocol.ReferenceParams) ([]protocol.Location, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/rename.go

@@ -14,7 +14,10 @@ import (
 
 func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err
@@ -43,7 +46,10 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr
 
 func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRenameParams) (*protocol.Range, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/signature_help.go

@@ -16,7 +16,10 @@ import (
 
 func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHelpParams) (*protocol.SignatureHelp, error) {
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/source/view.go

@@ -165,7 +165,7 @@ type Session interface {
 	View(name string) View
 
 	// ViewOf returns a view corresponding to the given URI.
-	ViewOf(uri span.URI) View
+	ViewOf(uri span.URI) (View, error)
 
 	// Views returns the set of active views built by this session.
 	Views() []View

internal/lsp/symbols.go

@@ -18,7 +18,10 @@ func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSy
 	defer done()
 
 	uri := span.NewURI(params.TextDocument.URI)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return nil, err
+	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		return nil, err

internal/lsp/text_synchronization.go

@@ -29,7 +29,10 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume
 	// Open the file.
 	s.session.DidOpen(ctx, uri, fileKind, version, text)
 
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return err
+	}
 
 	// Run diagnostics on the newly-changed file.
 	go s.diagnostics(view, uri)
@@ -64,7 +67,10 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
 		}
 	}
 	// Cache the new file content and send fresh diagnostics.
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return err
+	}
 	wasFirstChange, err := view.SetContent(ctx, uri, params.TextDocument.Version, []byte(text))
 	if err != nil {
 		return err
@@ -139,7 +145,10 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu
 	uri := span.NewURI(params.TextDocument.URI)
 	ctx = telemetry.URI.With(ctx, uri)
 	s.session.DidClose(uri)
-	view := s.session.ViewOf(uri)
+	view, err := s.session.ViewOf(uri)
+	if err != nil {
+		return err
+	}
 	if _, err := view.SetContent(ctx, uri, -1, nil); err != nil {
 		return err
 	}