From 9d59f9e855658d0b9a057f2b52549e14946d84c1 Mon Sep 17 00:00:00 2001
From: Muir Manders <muir@mnd.rs>
Date: Mon, 1 Jul 2019 21:28:46 +0000
Subject: [PATCH] internal/lsp: improve completion support for untyped
 constants

Add some extra smarts when evaluating untyped constants as completion
candidates. Previously we called types.Default() on the expected type
and candidate type, but this loses the untypedness of an untyped
constant which prevents it from being assignable to any type or named
type other than the untyped constant's default type.

Note that the added logic does not take into account the untyped
constant's value, so you will still get some false positive
completions (e.g. suggesting an untyped negative integer constant when
only a uint would do). Unfortunately go/types doesn't provide a way of
answering the question "is this *types.Const assignable to this
types.Type" since types.AssignableTo only considers a constant's type,
not its value.

Change-Id: If7075642e928f712b127256ae7706a5190e2f42c
GitHub-Last-Rev: 124d2f05b0aec09c9d7004d9da0d900524185b92
GitHub-Pull-Request: golang/tools#128
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184477
Reviewed-by: Suzy Mueller <suzmue@golang.org>
---
 internal/lsp/source/completion.go             | 20 +++++++++++---
 internal/lsp/testdata/rank/convert_rank.go.in | 27 +++++++++++++++++++
 internal/lsp/tests/tests.go                   |  2 +-
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index e060c2b1..805c441a 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -1028,14 +1028,28 @@ func (c *completer) matchingType(cand *candidate) bool {
 	// are invoked by default.
 	cand.expandFuncCall = isFunc(cand.obj)
 
-	typeMatches := func(actual types.Type) bool {
+	typeMatches := func(candType types.Type) bool {
 		// Take into account any type modifiers on the expected type.
-		actual = c.expectedType.applyTypeModifiers(actual)
+		candType = c.expectedType.applyTypeModifiers(candType)
 
 		if c.expectedType.objType != nil {
+			wantType := types.Default(c.expectedType.objType)
+
+			// Handle untyped values specially since AssignableTo gives false negatives
+			// for them (see https://golang.org/issue/32146).
+			if candBasic, ok := candType.(*types.Basic); ok && candBasic.Info()&types.IsUntyped > 0 {
+				if wantBasic, ok := wantType.Underlying().(*types.Basic); ok {
+					// Check that their constant kind (bool|int|float|complex|string) matches.
+					// This doesn't take into account the constant value, so there will be some
+					// false positives due to integer sign and overflow.
+					return candBasic.Info()&types.IsConstType == wantBasic.Info()&types.IsConstType
+				}
+				return false
+			}
+
 			// 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(types.Default(actual), types.Default(c.expectedType.objType))
+			return types.AssignableTo(candType, wantType)
 		}
 
 		return false
diff --git a/internal/lsp/testdata/rank/convert_rank.go.in b/internal/lsp/testdata/rank/convert_rank.go.in
index dc0a3a5e..fe6b7831 100644
--- a/internal/lsp/testdata/rank/convert_rank.go.in
+++ b/internal/lsp/testdata/rank/convert_rank.go.in
@@ -10,3 +10,30 @@ func _() {
 	)
 	wantsStrList(strList(conv)) //@complete("))", convertB, convertA)
 }
+
+func _() {
+	const (
+		convC        = "hi"    //@item(convertC, "convC", "string", "const")
+		convD        = 123     //@item(convertD, "convD", "int", "const")
+		convE int    = 123     //@item(convertE, "convE", "int", "const")
+		convF string = "there" //@item(convertF, "convF", "string", "const")
+	)
+
+	var foo int
+	foo = conv //@complete(" //", convertD, convertE, convertC, convertF)
+
+	type myInt int
+	var mi myInt
+	mi = conv //@complete(" //", convertD, convertC, convertE, convertF)
+	mi + conv //@complete(" //", convertD, convertC, convertE, convertF)
+
+	1 + conv //@complete(" //", convertD, convertE, convertC, convertF)
+
+	type myString string
+	var ms myString
+	ms = conv //@complete(" //", convertC, convertD, convertE, convertF)
+
+	type myUint uint32
+	var mu myUint
+	mu = conv //@complete(" //", convertD, convertC, convertE, convertF)
+}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 6b6c39f2..65a29f39 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -25,7 +25,7 @@ import (
 // We hardcode the expected number of test cases to ensure that all tests
 // are being executed. If a test is added, this number must be changed.
 const (
-	ExpectedCompletionsCount       = 138
+	ExpectedCompletionsCount       = 144
 	ExpectedCompletionSnippetCount = 15
 	ExpectedDiagnosticsCount       = 17
 	ExpectedFormatCount            = 5