From 5228f4b59c095453fbb2971e741aadd2089a8734 Mon Sep 17 00:00:00 2001
From: Muir Manders <muir@mnd.rs>
Date: Tue, 17 Sep 2019 14:59:27 -0700
Subject: [PATCH] internal/lsp: improve completions in *ast.MapType

We now expect a type name when in the key or value of a *ast.MapType.
I also added an extra filter to expect a comparable type for the key.

Change-Id: I647cf4d791b2c0960ad3b12702b91b9bc168599b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197439
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
---
 internal/lsp/source/completion.go        | 19 +++++++++++++++++++
 internal/lsp/source/source_test.go       |  9 +++------
 internal/lsp/testdata/maps/maps.go.in    | 18 ++++++++++++++++++
 internal/lsp/testdata/summary.txt.golden |  4 ++--
 4 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 internal/lsp/testdata/maps/maps.go.in

diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index e578ebc32b..9cf05b8957 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -989,6 +989,9 @@ type typeNameInference struct {
 
 	// assertableFrom is a type that must be assertable to our candidate type.
 	assertableFrom types.Type
+
+	// wantComparable is true if we want a comparable type.
+	wantComparable bool
 }
 
 // expectedType returns information about the expected type for an expression at
@@ -1205,6 +1208,7 @@ func breaksExpectedTypeInference(n ast.Node) bool {
 func expectTypeName(c *completer) typeNameInference {
 	var (
 		wantTypeName   bool
+		wantComparable bool
 		modifiers      []typeModifier
 		assertableFrom types.Type
 	)
@@ -1280,6 +1284,16 @@ Nodes:
 
 				break Nodes
 			}
+		case *ast.MapType:
+			wantTypeName = true
+			if n.Key != nil {
+				wantComparable = n.Key.Pos() <= c.pos && c.pos <= n.Key.End()
+			} else {
+				// If the key is empty, assume we are completing the key if
+				// pos is directly after the "map[".
+				wantComparable = c.pos == n.Pos()+token.Pos(len("map["))
+			}
+			break Nodes
 		default:
 			if breaksExpectedTypeInference(p) {
 				return typeNameInference{}
@@ -1289,6 +1303,7 @@ Nodes:
 
 	return typeNameInference{
 		wantTypeName:   wantTypeName,
+		wantComparable: wantComparable,
 		modifiers:      modifiers,
 		assertableFrom: assertableFrom,
 	}
@@ -1389,6 +1404,10 @@ func (c *completer) matchingTypeName(cand *candidate) bool {
 		}
 	}
 
+	if c.expectedType.typeName.wantComparable && !types.Comparable(actual) {
+		return false
+	}
+
 	// We can expect a type name and have an expected type in cases like:
 	//
 	//   var foo []int
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index d3716b4273..8c0d19e7dc 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -157,7 +157,7 @@ func (r *runner) DeepCompletion(t *testing.T, src span.Span, test tests.Completi
 	fuzzyMatcher := fuzzy.NewMatcher(prefix)
 	var got []protocol.CompletionItem
 	for _, item := range list {
-		if fuzzyMatcher.Score(item.Label) < 0 {
+		if fuzzyMatcher.Score(item.Label) <= 0 {
 			continue
 		}
 		got = append(got, item)
@@ -186,7 +186,7 @@ func (r *runner) FuzzyCompletion(t *testing.T, src span.Span, test tests.Complet
 	}
 	var got []protocol.CompletionItem
 	for _, item := range list {
-		if fuzzyMatcher != nil && fuzzyMatcher.Score(item.Label) < 0 {
+		if fuzzyMatcher != nil && fuzzyMatcher.Score(item.Label) <= 0 {
 			continue
 		}
 		got = append(got, item)
@@ -222,13 +222,10 @@ func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completi
 		Deep:          true,
 		Budget:        5 * time.Second,
 	})
-	if !strings.Contains(string(src.URI()), "builtins") {
-		list = tests.FilterBuiltins(list)
-	}
 	fuzzyMatcher := fuzzy.NewMatcher(prefix)
 	var got []protocol.CompletionItem
 	for _, item := range list {
-		if fuzzyMatcher.Score(item.Label) < 0 {
+		if fuzzyMatcher.Score(item.Label) <= 0 {
 			continue
 		}
 		got = append(got, item)
diff --git a/internal/lsp/testdata/maps/maps.go.in b/internal/lsp/testdata/maps/maps.go.in
new file mode 100644
index 0000000000..36451eade1
--- /dev/null
+++ b/internal/lsp/testdata/maps/maps.go.in
@@ -0,0 +1,18 @@
+package maps
+
+func _() {
+	var aVar int          //@item(mapVar, "aVar", "int", "var")
+
+	// not comparabale
+	type aSlice []int     //@item(mapSliceType, "aSlice", "[]int", "type")
+
+	// comparable
+	type aStruct struct{} //@item(mapStructType, "aStruct", "struct{...}", "struct")
+
+	map[]a{} //@complete("]", mapStructType, mapSliceType, mapVar)
+
+	map[a]a{} //@complete("]", mapStructType, mapSliceType, mapVar)
+	map[a]a{} //@complete("{", mapSliceType, mapStructType, mapVar)
+
+	map[]a{} //@rank("]", int, mapSliceType)
+}
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 8975e58ce5..9127199146 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -1,10 +1,10 @@
 -- summary --
-CompletionsCount = 216
+CompletionsCount = 219
 CompletionSnippetCount = 39
 UnimportedCompletionsCount = 1
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 7
-RankedCompletionsCount = 1
+RankedCompletionsCount = 2
 CaseSensitiveCompletionsCount = 4
 DiagnosticsCount = 22
 FoldingRangesCount = 2