lsp/completion: fix ranking of *types.PkgName candidates
In Go 1.18 types.AssignableTo() started reporting that an invalid type
is assignable to any interface. *types.PkgName (i.e. an import at the
top of the file) has an invalid type for its Type(), so we started
thinking all in scope imports were great candidates when the expected
type was an interface.
Fix by wrapping the AssignableTo (and AssertableTo) to explicitly
return false if either operand is invalid.
Updates golang/go#53595
Change-Id: Ie5a84b7f410ff5c73c6b7870e052bafaf3e21e99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415595
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go
index 0c1ff3f..a2dfae6 100644
--- a/internal/lsp/source/completion/completion.go
+++ b/internal/lsp/source/completion/completion.go
@@ -2314,7 +2314,7 @@
// matchesVariadic returns true if we are completing a variadic
// parameter and candType is a compatible slice type.
func (ci candidateInference) matchesVariadic(candType types.Type) bool {
- return ci.variadic && ci.objType != nil && types.AssignableTo(candType, types.NewSlice(ci.objType))
+ return ci.variadic && ci.objType != nil && assignableTo(candType, types.NewSlice(ci.objType))
}
// findSwitchStmt returns an *ast.CaseClause's corresponding *ast.SwitchStmt or
@@ -2640,7 +2640,7 @@
return false
}
- if ci.convertibleTo != nil && types.ConvertibleTo(candType, ci.convertibleTo) {
+ if ci.convertibleTo != nil && convertibleTo(candType, ci.convertibleTo) {
return true
}
@@ -2728,7 +2728,7 @@
return false
}
- if !types.ConvertibleTo(from, to) {
+ if !convertibleTo(from, to) {
return false
}
@@ -2777,7 +2777,7 @@
// AssignableTo covers the case where the types are equal, but also handles
// cases like assigning a concrete type to an interface type.
- return types.AssignableTo(candType, expType)
+ return assignableTo(candType, expType)
}
// kindMatches reports whether candType's kind matches our expected
@@ -2840,7 +2840,7 @@
assignee = ci.assignees[i]
}
- if assignee == nil {
+ if assignee == nil || assignee == types.Typ[types.Invalid] {
continue
}
@@ -2894,7 +2894,7 @@
//
// Where our expected type is "[]int", and we expect a type name.
if c.inference.objType != nil {
- return types.AssignableTo(candType, c.inference.objType)
+ return assignableTo(candType, c.inference.objType)
}
// Default to saying any type name is a match.
diff --git a/internal/lsp/source/completion/util.go b/internal/lsp/source/completion/util.go
index cd7849a..e6d3bfd 100644
--- a/internal/lsp/source/completion/util.go
+++ b/internal/lsp/source/completion/util.go
@@ -321,3 +321,23 @@
NewText: newText,
}})
}
+
+// assignableTo is like types.AssignableTo, but returns false if
+// either type is invalid.
+func assignableTo(x, to types.Type) bool {
+ if x == types.Typ[types.Invalid] || to == types.Typ[types.Invalid] {
+ return false
+ }
+
+ return types.AssignableTo(x, to)
+}
+
+// convertibleTo is like types.ConvertibleTo, but returns false if
+// either type is invalid.
+func convertibleTo(x, to types.Type) bool {
+ if x == types.Typ[types.Invalid] || to == types.Typ[types.Invalid] {
+ return false
+ }
+
+ return types.ConvertibleTo(x, to)
+}
diff --git a/internal/lsp/testdata/deep/deep.go b/internal/lsp/testdata/deep/deep.go
index 6ed5ff8..6908824 100644
--- a/internal/lsp/testdata/deep/deep.go
+++ b/internal/lsp/testdata/deep/deep.go
@@ -29,6 +29,13 @@
}
func _() {
+ var cork struct{ err error }
+ cork.err //@item(deepCorkErr, "cork.err", "error", "field")
+ context //@item(deepContextPkg, "context", "\"context\"", "package")
+ var _ error = co //@rank(" //", deepCorkErr, deepContextPkg)
+}
+
+func _() {
// deepCircle is circular.
type deepCircle struct {
*deepCircle
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 0247551..b6c6c07 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -6,7 +6,7 @@
UnimportedCompletionsCount = 5
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
-RankedCompletionsCount = 163
+RankedCompletionsCount = 164
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 37
FoldingRangesCount = 2
diff --git a/internal/lsp/testdata/summary_go1.18.txt.golden b/internal/lsp/testdata/summary_go1.18.txt.golden
index 7e8da12..9fadf63 100644
--- a/internal/lsp/testdata/summary_go1.18.txt.golden
+++ b/internal/lsp/testdata/summary_go1.18.txt.golden
@@ -6,7 +6,7 @@
UnimportedCompletionsCount = 5
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
-RankedCompletionsCount = 173
+RankedCompletionsCount = 174
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 37
FoldingRangesCount = 2
