Skip to content

Commit 80313e1

Browse files
stamblerrestamblerre
committed
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 #35599 Change-Id: I49789d8ce18e154f111bc3584488f468a129e30c Reviewed-on: https://go-review.googlesource.com/c/tools/+/207344 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent 3a792d9 commit 80313e1

20 files changed

+121
-34
lines changed

‎internal/lsp/cache/session.go

+17-8
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,21 @@ func (s *session) View(name string) source.View {
169169

170170
// ViewOf returns a view corresponding to the given URI.
171171
// If the file is not already associated with a view, pick one using some heuristics.
172-
func (s *session) ViewOf(uri span.URI) source.View {
172+
func (s *session) ViewOf(uri span.URI) (source.View, error) {
173173
s.viewMu.Lock()
174174
defer s.viewMu.Unlock()
175175

176176
// Check if we already know this file.
177177
if v, found := s.viewMap[uri]; found {
178-
return v
178+
return v, nil
179179
}
180180
// Pick the best view for this file and memoize the result.
181-
v := s.bestView(uri)
181+
v, err := s.bestView(uri)
182+
if err != nil {
183+
return nil, err
184+
}
182185
s.viewMap[uri] = v
183-
return v
186+
return v, nil
184187
}
185188

186189
func (s *session) viewsOf(uri span.URI) []*view {
@@ -208,7 +211,10 @@ func (s *session) Views() []source.View {
208211

209212
// bestView finds the best view to associate a given URI with.
210213
// viewMu must be held when calling this method.
211-
func (s *session) bestView(uri span.URI) source.View {
214+
func (s *session) bestView(uri span.URI) (source.View, error) {
215+
if len(s.views) == 0 {
216+
return nil, errors.Errorf("no views in the session")
217+
}
212218
// we need to find the best view for this file
213219
var longest source.View
214220
for _, view := range s.views {
@@ -220,10 +226,10 @@ func (s *session) bestView(uri span.URI) source.View {
220226
}
221227
}
222228
if longest != nil {
223-
return longest
229+
return longest, nil
224230
}
225231
// TODO: are there any more heuristics we can use?
226-
return s.views[0]
232+
return s.views[0], nil
227233
}
228234

229235
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
291297
}
292298

293299
// Make sure that the file gets added to the session's file watch map.
294-
view := s.bestView(uri)
300+
view, err := s.bestView(uri)
301+
if err != nil {
302+
return err
303+
}
295304
if _, err := view.GetFile(ctx, uri); err != nil {
296305
return err
297306
}

‎internal/lsp/code_action.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import (
2121

2222
func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
2323
uri := span.NewURI(params.TextDocument.URI)
24-
view := s.session.ViewOf(uri)
24+
view, err := s.session.ViewOf(uri)
25+
if err != nil {
26+
return nil, err
27+
}
2528
f, err := view.GetFile(ctx, uri)
2629
if err != nil {
2730
return nil, err

‎internal/lsp/command.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
1717
}
1818
// Confirm that this action is being taken on a go.mod file.
1919
uri := span.NewURI(params.Arguments[0].(string))
20-
view := s.session.ViewOf(uri)
20+
view, err := s.session.ViewOf(uri)
21+
if err != nil {
22+
return nil, err
23+
}
2124
f, err := view.GetFile(ctx, uri)
2225
if err != nil {
2326
return nil, err

‎internal/lsp/completion.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import (
1818

1919
func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) {
2020
uri := span.NewURI(params.TextDocument.URI)
21-
view := s.session.ViewOf(uri)
21+
view, err := s.session.ViewOf(uri)
22+
if err != nil {
23+
return nil, err
24+
}
2225
options := view.Options()
2326
f, err := view.GetFile(ctx, uri)
2427
if err != nil {

‎internal/lsp/completion_test.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,15 @@ func expected(t *testing.T, test tests.Completion, items tests.CompletionItems)
120120
func (r *runner) callCompletion(t *testing.T, src span.Span, options source.CompletionOptions) []protocol.CompletionItem {
121121
t.Helper()
122122

123-
view := r.server.session.ViewOf(src.URI())
123+
view, err := r.server.session.ViewOf(src.URI())
124+
if err != nil {
125+
t.Fatal(err)
126+
}
124127
original := view.Options()
125128
modified := original
126129
modified.InsertTextFormat = protocol.SnippetTextFormat
127130
modified.Completion = options
128-
view, err := view.SetOptions(r.ctx, modified)
131+
view, err = view.SetOptions(r.ctx, modified)
129132
if err != nil {
130133
t.Error(err)
131134
return nil

‎internal/lsp/definition.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import (
1414

1515
func (s *Server) definition(ctx context.Context, params *protocol.DefinitionParams) ([]protocol.Location, error) {
1616
uri := span.NewURI(params.TextDocument.URI)
17-
view := s.session.ViewOf(uri)
17+
view, err := s.session.ViewOf(uri)
18+
if err != nil {
19+
return nil, err
20+
}
1821
f, err := view.GetFile(ctx, uri)
1922
if err != nil {
2023
return nil, err
@@ -37,7 +40,10 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara
3740

3841
func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefinitionParams) ([]protocol.Location, error) {
3942
uri := span.NewURI(params.TextDocument.URI)
40-
view := s.session.ViewOf(uri)
43+
view, err := s.session.ViewOf(uri)
44+
if err != nil {
45+
return nil, err
46+
}
4147
f, err := view.GetFile(ctx, uri)
4248
if err != nil {
4349
return nil, err

‎internal/lsp/folding_range.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import (
1010

1111
func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRangeParams) ([]protocol.FoldingRange, error) {
1212
uri := span.NewURI(params.TextDocument.URI)
13-
view := s.session.ViewOf(uri)
13+
view, err := s.session.ViewOf(uri)
14+
if err != nil {
15+
return nil, err
16+
}
1417
f, err := view.GetFile(ctx, uri)
1518
if err != nil {
1619
return nil, err

‎internal/lsp/format.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import (
1414

1515
func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormattingParams) ([]protocol.TextEdit, error) {
1616
uri := span.NewURI(params.TextDocument.URI)
17-
view := s.session.ViewOf(uri)
17+
view, err := s.session.ViewOf(uri)
18+
if err != nil {
19+
return nil, err
20+
}
1821
f, err := view.GetFile(ctx, uri)
1922
if err != nil {
2023
return nil, err

‎internal/lsp/general.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,23 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
166166
debug.PrintVersionInfo(buf, true, debug.PlainText)
167167
log.Print(ctx, buf.String())
168168

169+
viewErrors := make(map[span.URI]error)
169170
for _, folder := range s.pendingFolders {
171+
uri := span.NewURI(folder.URI)
170172
if _, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
171-
return err
173+
viewErrors[uri] = err
172174
}
173175
}
176+
if len(viewErrors) > 0 {
177+
errMsg := fmt.Sprintf("Error loading workspace folders (expected %v, got %v)\n", len(s.pendingFolders), len(s.session.Views()))
178+
for uri, err := range viewErrors {
179+
errMsg += fmt.Sprintf("failed to load view for %s: %v\n", uri, err)
180+
}
181+
s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
182+
Type: protocol.Error,
183+
Message: errMsg,
184+
})
185+
}
174186
s.pendingFolders = nil
175187

176188
return nil

‎internal/lsp/highlight.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import (
1616

1717
func (s *Server) documentHighlight(ctx context.Context, params *protocol.DocumentHighlightParams) ([]protocol.DocumentHighlight, error) {
1818
uri := span.NewURI(params.TextDocument.URI)
19-
view := s.session.ViewOf(uri)
19+
view, err := s.session.ViewOf(uri)
20+
if err != nil {
21+
return nil, err
22+
}
2023
rngs, err := source.Highlight(ctx, view, uri, params.Position)
2124
if err != nil {
2225
log.Error(ctx, "no highlight", err, telemetry.URI.Of(uri))

‎internal/lsp/hover.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import (
1717

1818
func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*protocol.Hover, error) {
1919
uri := span.NewURI(params.TextDocument.URI)
20-
view := s.session.ViewOf(uri)
20+
view, err := s.session.ViewOf(uri)
21+
if err != nil {
22+
return nil, err
23+
}
2124
f, err := view.GetFile(ctx, uri)
2225
if err != nil {
2326
return nil, err

‎internal/lsp/implementation.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import (
1414

1515
func (s *Server) implementation(ctx context.Context, params *protocol.ImplementationParams) ([]protocol.Location, error) {
1616
uri := span.NewURI(params.TextDocument.URI)
17-
view := s.session.ViewOf(uri)
17+
view, err := s.session.ViewOf(uri)
18+
if err != nil {
19+
return nil, err
20+
}
1821
f, err := view.GetFile(ctx, uri)
1922
if err != nil {
2023
return nil, err

‎internal/lsp/link.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import (
2222

2323
func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) {
2424
uri := span.NewURI(params.TextDocument.URI)
25-
view := s.session.ViewOf(uri)
25+
view, err := s.session.ViewOf(uri)
26+
if err != nil {
27+
return nil, err
28+
}
2629
f, err := view.GetFile(ctx, uri)
2730
if err != nil {
2831
return nil, err

‎internal/lsp/lsp_test.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,16 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
9898

9999
func (r *runner) FoldingRanges(t *testing.T, spn span.Span) {
100100
uri := spn.URI()
101-
view := r.server.session.ViewOf(uri)
101+
view, err := r.server.session.ViewOf(uri)
102+
if err != nil {
103+
t.Fatal(err)
104+
}
102105
original := view.Options()
103106
modified := original
104107

105108
// Test all folding ranges.
106109
modified.LineFoldingOnly = false
107-
view, err := view.SetOptions(r.ctx, modified)
110+
view, err = view.SetOptions(r.ctx, modified)
108111
if err != nil {
109112
t.Error(err)
110113
return
@@ -326,7 +329,10 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
326329
func (r *runner) SuggestedFix(t *testing.T, spn span.Span) {
327330
uri := spn.URI()
328331
filename := uri.Filename()
329-
view := r.server.session.ViewOf(uri)
332+
view, err := r.server.session.ViewOf(uri)
333+
if err != nil {
334+
t.Fatal(err)
335+
}
330336
f, err := view.GetFile(r.ctx, uri)
331337
if err != nil {
332338
t.Fatal(err)

‎internal/lsp/references.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import (
1616

1717
func (s *Server) references(ctx context.Context, params *protocol.ReferenceParams) ([]protocol.Location, error) {
1818
uri := span.NewURI(params.TextDocument.URI)
19-
view := s.session.ViewOf(uri)
19+
view, err := s.session.ViewOf(uri)
20+
if err != nil {
21+
return nil, err
22+
}
2023
f, err := view.GetFile(ctx, uri)
2124
if err != nil {
2225
return nil, err

‎internal/lsp/rename.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import (
1414

1515
func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) {
1616
uri := span.NewURI(params.TextDocument.URI)
17-
view := s.session.ViewOf(uri)
17+
view, err := s.session.ViewOf(uri)
18+
if err != nil {
19+
return nil, err
20+
}
1821
f, err := view.GetFile(ctx, uri)
1922
if err != nil {
2023
return nil, err
@@ -43,7 +46,10 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr
4346

4447
func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRenameParams) (*protocol.Range, error) {
4548
uri := span.NewURI(params.TextDocument.URI)
46-
view := s.session.ViewOf(uri)
49+
view, err := s.session.ViewOf(uri)
50+
if err != nil {
51+
return nil, err
52+
}
4753
f, err := view.GetFile(ctx, uri)
4854
if err != nil {
4955
return nil, err

‎internal/lsp/signature_help.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import (
1616

1717
func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHelpParams) (*protocol.SignatureHelp, error) {
1818
uri := span.NewURI(params.TextDocument.URI)
19-
view := s.session.ViewOf(uri)
19+
view, err := s.session.ViewOf(uri)
20+
if err != nil {
21+
return nil, err
22+
}
2023
f, err := view.GetFile(ctx, uri)
2124
if err != nil {
2225
return nil, err

‎internal/lsp/source/view.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ type Session interface {
165165
View(name string) View
166166

167167
// ViewOf returns a view corresponding to the given URI.
168-
ViewOf(uri span.URI) View
168+
ViewOf(uri span.URI) (View, error)
169169

170170
// Views returns the set of active views built by this session.
171171
Views() []View

‎internal/lsp/symbols.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSy
1818
defer done()
1919

2020
uri := span.NewURI(params.TextDocument.URI)
21-
view := s.session.ViewOf(uri)
21+
view, err := s.session.ViewOf(uri)
22+
if err != nil {
23+
return nil, err
24+
}
2225
f, err := view.GetFile(ctx, uri)
2326
if err != nil {
2427
return nil, err

‎internal/lsp/text_synchronization.go

+12-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume
2929
// Open the file.
3030
s.session.DidOpen(ctx, uri, fileKind, version, text)
3131

32-
view := s.session.ViewOf(uri)
32+
view, err := s.session.ViewOf(uri)
33+
if err != nil {
34+
return err
35+
}
3336

3437
// Run diagnostics on the newly-changed file.
3538
go s.diagnostics(view, uri)
@@ -64,7 +67,10 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
6467
}
6568
}
6669
// Cache the new file content and send fresh diagnostics.
67-
view := s.session.ViewOf(uri)
70+
view, err := s.session.ViewOf(uri)
71+
if err != nil {
72+
return err
73+
}
6874
wasFirstChange, err := view.SetContent(ctx, uri, params.TextDocument.Version, []byte(text))
6975
if err != nil {
7076
return err
@@ -139,7 +145,10 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu
139145
uri := span.NewURI(params.TextDocument.URI)
140146
ctx = telemetry.URI.With(ctx, uri)
141147
s.session.DidClose(uri)
142-
view := s.session.ViewOf(uri)
148+
view, err := s.session.ViewOf(uri)
149+
if err != nil {
150+
return err
151+
}
143152
if _, err := view.SetContent(ctx, uri, -1, nil); err != nil {
144153
return err
145154
}

0 commit comments

Comments
 (0)