gopls: allow 'any' and 'comparable' in completion results
Remove long-since unnecessary filters in lexical completion results
excluding 'any' and 'comparable', and update tests accordingly.
Also improve diff output for completion results, using cmp.Diff.
Fixes golang/go#47669
Updates golang/go#54845
Change-Id: I5ea542d09b4dffb0dd701bc5f813c7bbdcd846f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463139
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/lsp/completion_test.go b/gopls/internal/lsp/completion_test.go
index 8211cc5..9a72f12 100644
--- a/gopls/internal/lsp/completion_test.go
+++ b/gopls/internal/lsp/completion_test.go
@@ -28,7 +28,7 @@
got = tests.FilterBuiltins(src, got)
want := expected(t, test, items)
if diff := tests.DiffCompletionItems(want, got); diff != "" {
- t.Errorf("%s", diff)
+ t.Errorf("mismatching completion items (-want +got):\n%s", diff)
}
}
@@ -66,8 +66,8 @@
})
got = tests.FilterBuiltins(src, got)
want := expected(t, test, items)
- if msg := tests.DiffCompletionItems(want, got); msg != "" {
- t.Errorf("%s", msg)
+ if diff := tests.DiffCompletionItems(want, got); diff != "" {
+ t.Errorf("mismatching completion items (-want +got):\n%s", diff)
}
}
@@ -79,8 +79,8 @@
})
got = tests.FilterBuiltins(src, got)
want := expected(t, test, items)
- if msg := tests.DiffCompletionItems(want, got); msg != "" {
- t.Errorf("%s", msg)
+ if diff := tests.DiffCompletionItems(want, got); diff != "" {
+ t.Errorf("mismatching completion items (-want +got):\n%s", diff)
}
}
@@ -91,8 +91,8 @@
})
got = tests.FilterBuiltins(src, got)
want := expected(t, test, items)
- if msg := tests.DiffCompletionItems(want, got); msg != "" {
- t.Errorf("%s", msg)
+ if diff := tests.DiffCompletionItems(want, got); diff != "" {
+ t.Errorf("mismatching completion items (-want +got):\n%s", diff)
}
}
diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
index 8aace97..056b289 100644
--- a/gopls/internal/lsp/source/completion/completion.go
+++ b/gopls/internal/lsp/source/completion/completion.go
@@ -1287,12 +1287,10 @@
var (
builtinIota = types.Universe.Lookup("iota")
builtinNil = types.Universe.Lookup("nil")
- // comparable is an interface that exists on the dev.typeparams Go branch.
- // Filter it out from completion results to stabilize tests.
- // TODO(rFindley) update (or remove) our handling for comparable once the
- // type parameter API has stabilized.
- builtinAny = types.Universe.Lookup("any")
- builtinComparable = types.Universe.Lookup("comparable")
+
+ // TODO(rfindley): only allow "comparable" where it is valid (in constraint
+ // position or embedded in interface declarations).
+ // builtinComparable = types.Universe.Lookup("comparable")
)
// Track seen variables to avoid showing completions for shadowed variables.
@@ -1311,9 +1309,6 @@
if declScope != scope {
continue // Name was declared in some enclosing scope, or not at all.
}
- if obj == builtinComparable || obj == builtinAny {
- continue
- }
// If obj's type is invalid, find the AST node that defines the lexical block
// containing the declaration of obj. Don't resolve types for packages.
diff --git a/gopls/internal/lsp/testdata/builtins/builtin_go117.go b/gopls/internal/lsp/testdata/builtins/builtin_go117.go
new file mode 100644
index 0000000..57abcde
--- /dev/null
+++ b/gopls/internal/lsp/testdata/builtins/builtin_go117.go
@@ -0,0 +1,8 @@
+//go:build !go1.18
+// +build !go1.18
+
+package builtins
+
+func _() {
+ //@complete("", append, bool, byte, cap, close, complex, complex128, complex64, copy, delete, error, _false, float32, float64, imag, int, int16, int32, int64, int8, len, make, new, panic, print, println, real, recover, rune, string, _true, uint, uint16, uint32, uint64, uint8, uintptr, _nil)
+}
diff --git a/gopls/internal/lsp/testdata/builtins/builtin_go118.go b/gopls/internal/lsp/testdata/builtins/builtin_go118.go
new file mode 100644
index 0000000..1fa0b14
--- /dev/null
+++ b/gopls/internal/lsp/testdata/builtins/builtin_go118.go
@@ -0,0 +1,8 @@
+//go:build go1.18
+// +build go1.18
+
+package builtins
+
+func _() {
+ //@complete("", any, append, bool, byte, cap, close, comparable, complex, complex128, complex64, copy, delete, error, _false, float32, float64, imag, int, int16, int32, int64, int8, len, make, new, panic, print, println, real, recover, rune, string, _true, uint, uint16, uint32, uint64, uint8, uintptr, _nil)
+}
diff --git a/gopls/internal/lsp/testdata/builtins/builtins.go b/gopls/internal/lsp/testdata/builtins/builtins.go
index 25c29f2..d30176c 100644
--- a/gopls/internal/lsp/testdata/builtins/builtins.go
+++ b/gopls/internal/lsp/testdata/builtins/builtins.go
@@ -1,15 +1,15 @@
package builtins
-func _() {
- //@complete("", append, bool, byte, cap, close, complex, complex128, complex64, copy, delete, error, _false, float32, float64, imag, int, int16, int32, int64, int8, len, make, new, panic, print, println, real, recover, rune, string, _true, uint, uint16, uint32, uint64, uint8, uintptr, _nil)
-}
+// Definitions of builtin completion items.
+/* any */ //@item(any, "any", "", "interface")
/* Create markers for builtin types. Only for use by this test.
/* append(slice []Type, elems ...Type) []Type */ //@item(append, "append", "func(slice []Type, elems ...Type) []Type", "func")
/* bool */ //@item(bool, "bool", "", "type")
/* byte */ //@item(byte, "byte", "", "type")
/* cap(v Type) int */ //@item(cap, "cap", "func(v Type) int", "func")
/* close(c chan<- Type) */ //@item(close, "close", "func(c chan<- Type)", "func")
+/* comparable */ //@item(comparable, "comparable", "", "interface")
/* complex(r float64, i float64) */ //@item(complex, "complex", "func(r float64, i float64) complex128", "func")
/* complex128 */ //@item(complex128, "complex128", "", "type")
/* complex64 */ //@item(complex64, "complex64", "", "type")
diff --git a/gopls/internal/lsp/tests/util.go b/gopls/internal/lsp/tests/util.go
index cdbd12e..344a768 100644
--- a/gopls/internal/lsp/tests/util.go
+++ b/gopls/internal/lsp/tests/util.go
@@ -17,6 +17,8 @@
"strings"
"testing"
+ "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/completion"
@@ -24,6 +26,29 @@
"golang.org/x/tools/gopls/internal/span"
)
+var builtins = map[string]bool{
+ "append": true,
+ "cap": true,
+ "close": true,
+ "complex": true,
+ "copy": true,
+ "delete": true,
+ "error": true,
+ "false": true,
+ "imag": true,
+ "iota": true,
+ "len": true,
+ "make": true,
+ "new": true,
+ "nil": true,
+ "panic": true,
+ "print": true,
+ "println": true,
+ "real": true,
+ "recover": true,
+ "true": true,
+}
+
// DiffLinks takes the links we got and checks if they are located within the source or a Note.
// If the link is within a Note, the link is removed.
// Returns an diff comment if there are differences and empty string if no diffs.
@@ -304,13 +329,7 @@
if i := strings.Index(trimmed, "("); i >= 0 {
trimmed = trimmed[:i]
}
- switch trimmed {
- case "append", "cap", "close", "complex", "copy", "delete",
- "error", "false", "imag", "iota", "len", "make", "new",
- "nil", "panic", "print", "println", "real", "recover", "true":
- return true
- }
- return false
+ return builtins[trimmed]
}
func CheckCompletionOrder(want, got []protocol.CompletionItem, strictScores bool) string {
@@ -390,28 +409,26 @@
// DiffCompletionItems prints the diff between expected and actual completion
// test results.
+//
+// The diff will be formatted using '-' and '+' for want and got, respectively.
func DiffCompletionItems(want, got []protocol.CompletionItem) string {
- if len(got) != len(want) {
- return summarizeCompletionItems(-1, want, got, "different lengths got %v want %v", len(got), len(want))
+ // Many fields are not set in the "want" slice.
+ irrelevantFields := []string{
+ "AdditionalTextEdits",
+ "Documentation",
+ "TextEdit",
+ "SortText",
+ "Preselect",
+ "FilterText",
+ "InsertText",
+ "InsertTextFormat",
}
- for i, w := range want {
- g := got[i]
- if w.Label != g.Label {
- return summarizeCompletionItems(i, want, got, "incorrect Label got %v want %v", g.Label, w.Label)
- }
- if NormalizeAny(w.Detail) != NormalizeAny(g.Detail) {
- return summarizeCompletionItems(i, want, got, "incorrect Detail got %v want %v", g.Detail, w.Detail)
- }
- if w.Documentation != "" && !strings.HasPrefix(w.Documentation, "@") {
- if w.Documentation != g.Documentation {
- return summarizeCompletionItems(i, want, got, "incorrect Documentation got %v want %v", g.Documentation, w.Documentation)
- }
- }
- if w.Kind != g.Kind {
- return summarizeCompletionItems(i, want, got, "incorrect Kind got %v want %v", g.Kind, w.Kind)
- }
- }
- return ""
+ ignore := cmpopts.IgnoreFields(protocol.CompletionItem{}, irrelevantFields...)
+ normalizeAny := cmpopts.AcyclicTransformer("NormalizeAny", func(item protocol.CompletionItem) protocol.CompletionItem {
+ item.Detail = NormalizeAny(item.Detail)
+ return item
+ })
+ return cmp.Diff(want, got, ignore, normalizeAny)
}
func summarizeCompletionItems(i int, want, got []protocol.CompletionItem, reason string, args ...interface{}) string {
diff --git a/gopls/internal/lsp/tests/util_go118.go b/gopls/internal/lsp/tests/util_go118.go
new file mode 100644
index 0000000..6115342
--- /dev/null
+++ b/gopls/internal/lsp/tests/util_go118.go
@@ -0,0 +1,13 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.18
+// +build go1.18
+
+package tests
+
+func init() {
+ builtins["any"] = true
+ builtins["comparable"] = true
+}
