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>
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index cb80f19..8f1158f 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -169,18 +169,21 @@
// 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 @@
// 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 @@
}
}
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 @@
}
// 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
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index cf66505..8db4b93 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -21,7 +21,10 @@
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
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 72e8647..5075e90 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -17,7 +17,10 @@
}
// 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
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 949a433..4b1e390 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -18,7 +18,10 @@
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 {
diff --git a/internal/lsp/completion_test.go b/internal/lsp/completion_test.go
index 0ee2e06..6fdffa1 100644
--- a/internal/lsp/completion_test.go
+++ b/internal/lsp/completion_test.go
@@ -120,12 +120,15 @@
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
diff --git a/internal/lsp/definition.go b/internal/lsp/definition.go
index f8f18df..6e3cae2 100644
--- a/internal/lsp/definition.go
+++ b/internal/lsp/definition.go
@@ -14,7 +14,10 @@
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) 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
diff --git a/internal/lsp/folding_range.go b/internal/lsp/folding_range.go
index d0eecc4..5d88e74 100644
--- a/internal/lsp/folding_range.go
+++ b/internal/lsp/folding_range.go
@@ -10,7 +10,10 @@
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
diff --git a/internal/lsp/format.go b/internal/lsp/format.go
index 33e9b8d..b16bfb6 100644
--- a/internal/lsp/format.go
+++ b/internal/lsp/format.go
@@ -14,7 +14,10 @@
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
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index a18fdd2..caab70b 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -166,11 +166,23 @@
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
diff --git a/internal/lsp/highlight.go b/internal/lsp/highlight.go
index 8a579a6..c873127 100644
--- a/internal/lsp/highlight.go
+++ b/internal/lsp/highlight.go
@@ -16,7 +16,10 @@
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))
diff --git a/internal/lsp/hover.go b/internal/lsp/hover.go
index ddf1a32..4377e67 100644
--- a/internal/lsp/hover.go
+++ b/internal/lsp/hover.go
@@ -17,7 +17,10 @@
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
diff --git a/internal/lsp/implementation.go b/internal/lsp/implementation.go
index daca99f..09f18e4 100644
--- a/internal/lsp/implementation.go
+++ b/internal/lsp/implementation.go
@@ -14,7 +14,10 @@
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
diff --git a/internal/lsp/link.go b/internal/lsp/link.go
index 92777fb..c9f89f2 100644
--- a/internal/lsp/link.go
+++ b/internal/lsp/link.go
@@ -22,7 +22,10 @@
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
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index c4daa4b..e2ee9af 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -98,13 +98,16 @@
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) 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)
diff --git a/internal/lsp/references.go b/internal/lsp/references.go
index 6930a6c..a292035 100644
--- a/internal/lsp/references.go
+++ b/internal/lsp/references.go
@@ -16,7 +16,10 @@
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
diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go
index 9664b5c..cc9f1a1 100644
--- a/internal/lsp/rename.go
+++ b/internal/lsp/rename.go
@@ -14,7 +14,10 @@
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) 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
diff --git a/internal/lsp/signature_help.go b/internal/lsp/signature_help.go
index 5771ef1..f65a7a3 100644
--- a/internal/lsp/signature_help.go
+++ b/internal/lsp/signature_help.go
@@ -16,7 +16,10 @@
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
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 16907b5..2e433fd 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -165,7 +165,7 @@
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
diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go
index 66f1654..b4d3a3f 100644
--- a/internal/lsp/symbols.go
+++ b/internal/lsp/symbols.go
@@ -18,7 +18,10 @@
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
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index fbb67bf..680d46b 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -29,7 +29,10 @@
// 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 @@
}
}
// 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 @@
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
}
