internal/lsp: consolidate completion sorting

Move completion candidate sorting into internal/lsp/source so source_test.go and internal/lsp don't have to duplicate the logic. Change-Id: Ifbe7ca5ad6a5b74020fd1260b4d4f775709968cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/215137 Reviewed-by: Rebecca Stambler <[email protected]> Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
benkrig · Jan 17, 2020 · a209551 · a209551
1 parent 7ad9cd8
commit a209551
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 16 deletions.
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
@@ -7,7 +7,6 @@ package lsp
 import (
 	"context"
 	"fmt"
-	"sort"
 	"strings"
 
 	"golang.org/x/tools/internal/lsp/protocol"
@@ -50,13 +49,6 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	if err != nil {
 		return nil, err
 	}
-	// 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
 	// client fetches updated completions after every key stroke.

diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
@@ -12,6 +12,7 @@ import (
 	"go/token"
 	"go/types"
 	"math"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -484,6 +485,8 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto
 
 	c.expectedType = expectedType(c)
 
+	defer c.sortItems()
+
 	// If we're inside a comment return comment completions
 	for _, comment := range file.Comments {
 		if comment.Pos() <= rng.Start && rng.Start <= comment.End() {
@@ -566,6 +569,19 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto
 	return c.items, c.getSurrounding(), nil
 }
 
+func (c *completer) sortItems() {
+	sort.SliceStable(c.items, func(i, j int) bool {
+		// Sort by score first.
+		if c.items[i].Score != c.items[j].Score {
+			return c.items[i].Score > c.items[j].Score
+		}
+
+		// Then sort by label so order stays consistent. This also has the
+		// effect of prefering shorter candidates.
+		return c.items[i].Label < c.items[j].Label
+	})
+}
+
 // populateCommentCompletions yields completions for an exported
 // variable immediately preceding comment.
 func (c *completer) populateCommentCompletions(comment *ast.CommentGroup) {

diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
@@ -245,14 +245,7 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options func(*sourc
 	if surrounding != nil {
 		prefix = strings.ToLower(surrounding.Prefix())
 	}
-	// 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
 	// Apply deep completion filtering.