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 +}