internal/lsp: return an error from {Narrowest,Widest}CheckPackageHandle
This will prevent us from panicking in cases with errors.
Fixes golang/go#34824
Change-Id: I02c20655f6926ec00c1591a905ff5a107cc44192
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200300
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index 1435ffa..3eaded2 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -110,7 +110,10 @@
if err != nil {
continue
}
- cph := source.WidestCheckPackageHandle(cphs)
+ cph, err := source.WidestCheckPackageHandle(cphs)
+ if err != nil {
+ continue
+ }
for _, ph := range cph.Files() {
seen[ph.File().Identity().URI] = struct{}{}
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index ea7bff6..dadb9e8 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -213,7 +213,10 @@
}
// We get the package that source.Diagnostics would've used. This is hack.
// TODO(golang/go#32443): The correct solution will be to cache diagnostics per-file per-snapshot.
- cph := source.WidestCheckPackageHandle(cphs)
+ cph, err := source.WidestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Cached(ctx)
if err != nil {
return nil, err
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 4e47a6c..9f45283 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -393,7 +393,10 @@
if err != nil {
return nil, nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, nil, err
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index a528bac..a1f0e8d 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -45,7 +45,10 @@
if err != nil {
return nil, "", err
}
- cph := WidestCheckPackageHandle(cphs)
+ cph, err := WidestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, "", err
+ }
// If we are missing dependencies, it may because the user's workspace is
// not correctly configured. Report errors, if possible.
@@ -220,7 +223,10 @@
if err != nil {
return Diagnostic{}, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return Diagnostic{}, err
+ }
pkg, err := cph.Cached(ctx)
if err != nil {
return Diagnostic{}, err
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index 15185db..46f2ee2 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -28,7 +28,10 @@
if err != nil {
return nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
@@ -91,7 +94,10 @@
if err != nil {
return nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
@@ -154,7 +160,10 @@
if err != nil {
return nil, nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, nil, err
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index 2aa0613..baed832 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -53,7 +53,10 @@
if err != nil {
return nil, err
}
- cph := WidestCheckPackageHandle(cphs)
+ cph, err := WidestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index de6b1b3..0f87de4 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -35,7 +35,10 @@
if err != nil {
return nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go
index 5f249b4..4b765ad 100644
--- a/internal/lsp/source/symbols.go
+++ b/internal/lsp/source/symbols.go
@@ -22,7 +22,10 @@
if err != nil {
return nil, err
}
- cph := NarrowestCheckPackageHandle(cphs)
+ cph, err := NarrowestCheckPackageHandle(cphs)
+ if err != nil {
+ return nil, err
+ }
pkg, err := cph.Check(ctx)
if err != nil {
return nil, err
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index c65b870..304de90 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -56,9 +56,9 @@
// By "narrowest" package, we mean the package with the fewest number of files
// that includes the given file. This solves the problem of test variants,
// as the test will have more files than the non-test package.
-func NarrowestCheckPackageHandle(handles []CheckPackageHandle) CheckPackageHandle {
+func NarrowestCheckPackageHandle(handles []CheckPackageHandle) (CheckPackageHandle, error) {
if len(handles) < 1 {
- return nil
+ return nil, errors.Errorf("no CheckPackageHandles")
}
result := handles[0]
for _, handle := range handles[1:] {
@@ -66,16 +66,19 @@
result = handle
}
}
- return result
+ if result == nil {
+ return nil, errors.Errorf("nil CheckPackageHandles have been returned")
+ }
+ return result, nil
}
// WidestCheckPackageHandle returns the CheckPackageHandle containing the most files.
//
// This is useful for something like diagnostics, where we'd prefer to offer diagnostics
// for as many files as possible.
-func WidestCheckPackageHandle(handles []CheckPackageHandle) CheckPackageHandle {
+func WidestCheckPackageHandle(handles []CheckPackageHandle) (CheckPackageHandle, error) {
if len(handles) < 1 {
- return nil
+ return nil, errors.Errorf("no CheckPackageHandles")
}
result := handles[0]
for _, handle := range handles[1:] {
@@ -83,7 +86,10 @@
result = handle
}
}
- return result
+ if result == nil {
+ return nil, errors.Errorf("nil CheckPackageHandles have been returned")
+ }
+ return result, nil
}
func IsGenerated(ctx context.Context, view View, uri span.URI) bool {
