internal/lsp: sort by label after score

I want to stop sorting unimported completions. We still want to show
users something reasonable, so use label as a tiebreaker for score in
the higher level completion function.

To maintain the current sorting, we need to adjust scores by search
depth (height?) for lexical completions. A few tests are really ties,
and need sorting in the test case.

Change-Id: Ie2d09fdcbebf6fda4ab33a2f16c579d12b0f26ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212633
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/completion.go

@@ -52,9 +52,12 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	if err != nil {
 		return nil, err
 	}
-	// Sort the candidates by score, since that is not supported by LSP yet.
+	// Sort the candidates by score, then label, since that is not supported by LSP yet.
 	sort.SliceStable(candidates, func(i, j int) bool {
+		if candidates[i].Score != candidates[j].Score {
 			return candidates[i].Score > candidates[j].Score
+		}
+		return candidates[i].Label < candidates[j].Label
 	})
 
 	// When using deep completions/fuzzy matching, report results as incomplete so

internal/lsp/source/completion.go

@@ -11,6 +11,7 @@ import (
 	"go/constant"
 	"go/token"
 	"go/types"
+	"math"
 	"strconv"
 	"strings"
 	"time"
@@ -696,7 +697,7 @@ func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *impo
 	for _, f := range fieldSelections(typ) {
 		c.found(candidate{
 			obj:         f,
-			score:       stdScore,
+			score:       stdScore - 0.01,
 			imp:         imp,
 			addressable: addressable || isPointer(typ),
 		})
@@ -778,7 +779,8 @@ func (c *completer) lexical() error {
 				continue
 			}
 
-			score := stdScore
+			// Rank outer scopes lower than inner.
+			score := stdScore * math.Pow(.99, float64(i))
 
 			// Dowrank "nil" a bit so it is ranked below more interesting candidates.
 			if obj == builtinNil {

internal/lsp/source/completion_keywords.go

@@ -38,6 +38,7 @@ const (
 
 // keyword looks at the current scope of an *ast.Ident and recommends keywords
 func (c *completer) keyword() error {
+	keywordScore := float64(0.9)
 	if _, ok := c.path[0].(*ast.Ident); !ok {
 		// TODO(golang/go#34009): Support keyword completion in any context
 		return errors.Errorf("keywords are currently only recommended for identifiers")
@@ -61,7 +62,7 @@ func (c *completer) keyword() error {
 		case *ast.CaseClause:
 			// only recommend "fallthrough" and "break" within the bodies of a case clause
 			if c.pos > node.Colon {
-				valid[BREAK] = stdScore
+				valid[BREAK] = keywordScore
 				// "fallthrough" is only valid in switch statements.
 				// A case clause is always nested within a block statement in a switch statement,
 				// that block statement is nested within either a TypeSwitchStmt or a SwitchStmt.
@@ -69,32 +70,32 @@ func (c *completer) keyword() error {
 					continue
 				}
 				if _, ok := path[i+2].(*ast.SwitchStmt); ok {
-					valid[FALLTHROUGH] = stdScore
+					valid[FALLTHROUGH] = keywordScore
 				}
 			}
 		case *ast.CommClause:
 			if c.pos > node.Colon {
-				valid[BREAK] = stdScore
+				valid[BREAK] = keywordScore
 			}
 		case *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.SwitchStmt:
-			valid[CASE] = stdScore + lowScore
+			valid[CASE] = keywordScore + lowScore
-			valid[DEFAULT] = stdScore + lowScore
+			valid[DEFAULT] = keywordScore + lowScore
 		case *ast.ForStmt:
-			valid[BREAK] = stdScore
+			valid[BREAK] = keywordScore
-			valid[CONTINUE] = stdScore
+			valid[CONTINUE] = keywordScore
 		// This is a bit weak, functions allow for many keywords
 		case *ast.FuncDecl:
 			if node.Body != nil && c.pos > node.Body.Lbrace {
-				valid[DEFER] = stdScore - lowScore
+				valid[DEFER] = keywordScore - lowScore
-				valid[RETURN] = stdScore - lowScore
+				valid[RETURN] = keywordScore - lowScore
-				valid[FOR] = stdScore - lowScore
+				valid[FOR] = keywordScore - lowScore
-				valid[GO] = stdScore - lowScore
+				valid[GO] = keywordScore - lowScore
-				valid[SWITCH] = stdScore - lowScore
+				valid[SWITCH] = keywordScore - lowScore
-				valid[SELECT] = stdScore - lowScore
+				valid[SELECT] = keywordScore - lowScore
-				valid[IF] = stdScore - lowScore
+				valid[IF] = keywordScore - lowScore
-				valid[ELSE] = stdScore - lowScore
+				valid[ELSE] = keywordScore - lowScore
-				valid[VAR] = stdScore - lowScore
+				valid[VAR] = keywordScore - lowScore
-				valid[CONST] = stdScore - lowScore
+				valid[CONST] = keywordScore - lowScore
 			}
 		}
 	}

internal/lsp/source/completion_labels.go

@@ -7,6 +7,7 @@ package source
 import (
 	"go/ast"
 	"go/token"
+	"math"
 )
 
 type labelType int
@@ -53,10 +54,10 @@ func (c *completer) labels(lt labelType) {
 		return
 	}
 
-	addLabel := func(l *ast.LabeledStmt) {
+	addLabel := func(score float64, l *ast.LabeledStmt) {
 		labelObj := c.pkg.GetTypesInfo().ObjectOf(l.Label)
 		if labelObj != nil {
-			c.found(candidate{obj: labelObj, score: highScore})
+			c.found(candidate{obj: labelObj, score: score})
 		}
 	}
 
@@ -64,7 +65,7 @@ func (c *completer) labels(lt labelType) {
 	case labelBreak, labelContinue:
 		// "break" and "continue" only accept labels from enclosing statements.
 
-		for _, p := range c.path {
+		for i, p := range c.path {
 			switch p := p.(type) {
 			case *ast.FuncLit:
 				// Labels are function scoped, so don't continue out of functions.
@@ -73,11 +74,11 @@ func (c *completer) labels(lt labelType) {
 				switch p.Stmt.(type) {
 				case *ast.ForStmt, *ast.RangeStmt:
 					// Loop labels can be used for "break" or "continue".
-					addLabel(p)
+					addLabel(highScore*math.Pow(.99, float64(i)), p)
 				case *ast.SwitchStmt, *ast.SelectStmt, *ast.TypeSwitchStmt:
 					// Switch and select labels can be used only for "break".
 					if lt == labelBreak {
-						addLabel(p)
+						addLabel(highScore*math.Pow(.99, float64(i)), p)
 					}
 				}
 			}
@@ -102,7 +103,7 @@ func (c *completer) labels(lt labelType) {
 				}
 				return false
 			case *ast.LabeledStmt:
-				addLabel(n)
+				addLabel(highScore, n)
 			}
 
 			return true

internal/lsp/source/source_test.go

@@ -251,7 +251,10 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options source.Comp
 	// TODO(rstambler): In testing this out, I noticed that scores are equal,
 	// even when they shouldn't be. This needs more investigation.
 	sort.SliceStable(list, func(i, j int) bool {
+		if list[i].Score != list[j].Score {
 			return list[i].Score > list[j].Score
+		}
+		return list[i].Label < list[j].Label
 	})
 	var numDeepCompletionsSeen int
 	var items []source.CompletionItem

internal/lsp/testdata/deep/deep.go

@@ -113,9 +113,9 @@ func _() {
 		f foo
 	)
 
+	f.b.ptrReceiver()      //@item(deepBPtr, "f.b.ptrReceiver", "func() int", "method")
 	f.bar().valueReceiver  //@item(deepBarValue, "f.bar().valueReceiver", "func() int", "method")
-	f.barPtr().valueReceiver //@item(deepBarPtrValue, "f.barPtr().valueReceiver", "func() int", "method")
 	f.barPtr().ptrReceiver //@item(deepBarPtrPtr, "f.barPtr().ptrReceiver", "func() int", "method")
 
-	i = fb //@fuzzy(" //", deepBarValue, deepBarPtrPtr, deepBarPtrValue)
+	i = fb //@fuzzy(" //", deepBPtr, deepBarValue, deepBarPtrPtr)
 }

internal/lsp/testdata/selector/selector.go.in

@@ -11,7 +11,7 @@ type S struct {
 }
 
 func _() {
-	_ = S{}.; //@complete(";", Bf, Af, Cf)
+	_ = S{}.; //@complete(";", Af, Bf, Cf)
 }
 
 type bob struct { a int } //@item(a, "a", "int", "field")