gopls/internal/lsp/source: hovering over broken packages is not an error
Fixes golang/go#64237
Change-Id: I5b326e084c31a7835684fb08491bd71af606fb1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/549117
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index 9e9f5dd..4dc747d 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -273,7 +273,7 @@
{
linkMeta = findFileInDeps(snapshot, pkg.Metadata(), declPGF.URI)
if linkMeta == nil {
- return protocol.Range{}, nil, bug.Errorf("no metadata for %s", declPGF.URI)
+ return protocol.Range{}, nil, bug.Errorf("no package data for %s", declPGF.URI)
}
// For package names, we simply link to their imported package.
@@ -283,7 +283,10 @@
impID := linkMeta.DepsByPkgPath[PackagePath(pkgName.Imported().Path())]
linkMeta = snapshot.Metadata(impID)
if linkMeta == nil {
- return protocol.Range{}, nil, bug.Errorf("no metadata for %s", declPGF.URI)
+ // Broken imports have fake package paths, so it is not a bug if we
+ // don't have metadata. As of writing, there is no way to distinguish
+ // broken imports from a true bug where expected metadata is missing.
+ return protocol.Range{}, nil, fmt.Errorf("no package data for %s", declPGF.URI)
}
} else {
// For all others, check whether the object is in the package scope, or
diff --git a/gopls/internal/test/marker/doc.go b/gopls/internal/test/marker/doc.go
index 88ed89d..755812f 100644
--- a/gopls/internal/test/marker/doc.go
+++ b/gopls/internal/test/marker/doc.go
@@ -151,18 +151,18 @@
TODO(adonovan): in the older marker framework, the annotation asserted
two additional fields (source="compiler", kind="error"). Restore them?
- - def(src, dst location): perform a textDocument/definition request at
+ - def(src, dst location): performs a textDocument/definition request at
the src location, and check the result points to the dst location.
- documentLink(golden): asserts that textDocument/documentLink returns
links as described by the golden file.
- - foldingrange(golden): perform a textDocument/foldingRange for the
+ - foldingrange(golden): performs a textDocument/foldingRange for the
current document, and compare with the golden content, which is the
original source annotated with numbered tags delimiting the resulting
ranges (e.g. <1 kind="..."> ... </1>).
- - format(golden): perform a textDocument/format request for the enclosing
+ - format(golden): performs a textDocument/format request for the enclosing
file, and compare against the named golden file. If the formatting
request succeeds, the golden file must contain the resulting formatted
source. If the formatting request fails, the golden file must contain
@@ -172,9 +172,12 @@
textDocument/highlight request at the given src location, which should
highlight the provided dst locations.
- - hover(src, dst location, sm stringMatcher): perform a
- textDocument/hover at the src location, and checks that the result is
- the dst location, with matching hover content.
+ - hover(src, dst location, sm stringMatcher): performs a textDocument/hover
+ at the src location, and checks that the result is the dst location, with
+ matching hover content.
+
+ - hovererr(src, sm stringMatcher): performs a textDocument/hover at the src
+ location, and checks that the error matches the given stringMatcher.
- implementations(src location, want ...location): makes a
textDocument/implementation query at the src location and
diff --git a/gopls/internal/test/marker/marker_test.go b/gopls/internal/test/marker/marker_test.go
index 10aba7e..2fa4197 100644
--- a/gopls/internal/test/marker/marker_test.go
+++ b/gopls/internal/test/marker/marker_test.go
@@ -232,6 +232,9 @@
func (m marker) T() testing.TB { return m.run.env.T }
// server returns the LSP server for the marker test run.
+func (m marker) editor() *fake.Editor { return m.run.env.Editor }
+
+// server returns the LSP server for the marker test run.
func (m marker) server() protocol.Server { return m.run.env.Editor.Server }
// uri returns the URI of the file containing the marker.
@@ -251,7 +254,7 @@
// mapper returns a *protocol.Mapper for the current file.
func (mark marker) mapper() *protocol.Mapper {
- mapper, err := mark.run.env.Editor.Mapper(mark.path())
+ mapper, err := mark.editor().Mapper(mark.path())
if err != nil {
mark.T().Fatalf("failed to get mapper for current mark: %v", err)
}
@@ -418,6 +421,7 @@
"format": actionMarkerFunc(formatMarker),
"highlight": actionMarkerFunc(highlightMarker),
"hover": actionMarkerFunc(hoverMarker),
+ "hovererr": actionMarkerFunc(hoverErrMarker),
"implementation": actionMarkerFunc(implementationMarker),
"incomingcalls": actionMarkerFunc(incomingCallsMarker),
"inlayhints": actionMarkerFunc(inlayhintsMarker),
@@ -1456,10 +1460,6 @@
}
}
-// hoverMarker implements the @hover marker, running textDocument/hover at the
-// given src location and asserting that the resulting hover is over the dst
-// location (typically a span surrounding src), and that the markdown content
-// matches the golden content.
func hoverMarker(mark marker, src, dst protocol.Location, sc stringMatcher) {
content, gotDst := mark.run.env.Hover(src)
if gotDst != dst {
@@ -1472,6 +1472,11 @@
sc.check(mark, gotMD)
}
+func hoverErrMarker(mark marker, src protocol.Location, em stringMatcher) {
+ _, _, err := mark.editor().Hover(mark.ctx(), src)
+ em.checkErr(mark, err)
+}
+
// locMarker implements the @loc marker. It is executed before other
// markers, so that locations are available.
func locMarker(mark marker, loc protocol.Location) protocol.Location { return loc }
diff --git a/gopls/internal/test/marker/testdata/hover/issue64239.txt b/gopls/internal/test/marker/testdata/hover/issue64239.txt
deleted file mode 100644
index 493942d..0000000
--- a/gopls/internal/test/marker/testdata/hover/issue64239.txt
+++ /dev/null
@@ -1,9 +0,0 @@
-This test verifies the fix for issue #64239: hover fails for objects in the
-unsafe package.
-
--- p.go --
-package p
-
-import "unsafe"
-
-var _ = unsafe.Sizeof(struct{}{}) //@hover("Sizeof", "Sizeof", "`Sizeof` on pkg.go.dev")
diff --git a/gopls/internal/test/marker/testdata/hover/issues.txt b/gopls/internal/test/marker/testdata/hover/issues.txt
new file mode 100644
index 0000000..6212964
--- /dev/null
+++ b/gopls/internal/test/marker/testdata/hover/issues.txt
@@ -0,0 +1,22 @@
+This test verifies fixes for various issues reported for hover.
+
+-- go.mod --
+module golang.org/lsptests
+
+-- issue64239/p.go --
+package issue64239
+
+// golang/go#64239: hover fails for objects in the unsafe package.
+
+import "unsafe"
+
+var _ = unsafe.Sizeof(struct{}{}) //@hover("Sizeof", "Sizeof", "`Sizeof` on pkg.go.dev")
+
+-- issue64237/p.go --
+package issue64237
+
+// golang/go#64237: hover panics for broken imports.
+
+import "golang.org/lsptests/nonexistant" //@diag("\"golang", re"could not import")
+
+var _ = nonexistant.Value //@hovererr("nonexistant", "no package data")
