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