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>
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index cdad7e4..90bcba9 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -52,9 +52,12 @@
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 {
- return candidates[i].Score > candidates[j].Score
+ 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
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 930eb09..767181d 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -11,6 +11,7 @@
"go/constant"
"go/token"
"go/types"
+ "math"
"strconv"
"strings"
"time"
@@ -696,7 +697,7 @@
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 @@
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 {
diff --git a/internal/lsp/source/completion_keywords.go b/internal/lsp/source/completion_keywords.go
index 526de65..dcb5c95 100644
--- a/internal/lsp/source/completion_keywords.go
+++ b/internal/lsp/source/completion_keywords.go
@@ -38,6 +38,7 @@
// 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 @@
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 @@
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[DEFAULT] = stdScore + lowScore
+ valid[CASE] = keywordScore + lowScore
+ valid[DEFAULT] = keywordScore + lowScore
case *ast.ForStmt:
- valid[BREAK] = stdScore
- valid[CONTINUE] = stdScore
+ valid[BREAK] = keywordScore
+ 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[RETURN] = stdScore - lowScore
- valid[FOR] = stdScore - lowScore
- valid[GO] = stdScore - lowScore
- valid[SWITCH] = stdScore - lowScore
- valid[SELECT] = stdScore - lowScore
- valid[IF] = stdScore - lowScore
- valid[ELSE] = stdScore - lowScore
- valid[VAR] = stdScore - lowScore
- valid[CONST] = stdScore - lowScore
+ valid[DEFER] = keywordScore - lowScore
+ valid[RETURN] = keywordScore - lowScore
+ valid[FOR] = keywordScore - lowScore
+ valid[GO] = keywordScore - lowScore
+ valid[SWITCH] = keywordScore - lowScore
+ valid[SELECT] = keywordScore - lowScore
+ valid[IF] = keywordScore - lowScore
+ valid[ELSE] = keywordScore - lowScore
+ valid[VAR] = keywordScore - lowScore
+ valid[CONST] = keywordScore - lowScore
}
}
}
diff --git a/internal/lsp/source/completion_labels.go b/internal/lsp/source/completion_labels.go
index ebef101..0a23aa8 100644
--- a/internal/lsp/source/completion_labels.go
+++ b/internal/lsp/source/completion_labels.go
@@ -7,6 +7,7 @@
import (
"go/ast"
"go/token"
+ "math"
)
type labelType int
@@ -53,10 +54,10 @@
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 @@
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 @@
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 @@
}
return false
case *ast.LabeledStmt:
- addLabel(n)
+ addLabel(highScore, n)
}
return true
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index c53a787..c77cab4 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -251,7 +251,10 @@
// 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 {
- return list[i].Score > list[j].Score
+ 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
diff --git a/internal/lsp/testdata/deep/deep.go b/internal/lsp/testdata/deep/deep.go
index a7c659a..c49f001 100644
--- a/internal/lsp/testdata/deep/deep.go
+++ b/internal/lsp/testdata/deep/deep.go
@@ -113,9 +113,9 @@
f foo
)
- 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")
+ f.b.ptrReceiver() //@item(deepBPtr, "f.b.ptrReceiver", "func() int", "method")
+ f.bar().valueReceiver //@item(deepBarValue, "f.bar().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)
}
diff --git a/internal/lsp/testdata/selector/selector.go.in b/internal/lsp/testdata/selector/selector.go.in
index 77df003..6c6e1fd 100644
--- a/internal/lsp/testdata/selector/selector.go.in
+++ b/internal/lsp/testdata/selector/selector.go.in
@@ -11,7 +11,7 @@
}
func _() {
- _ = S{}.; //@complete(";", Bf, Af, Cf)
+ _ = S{}.; //@complete(";", Af, Bf, Cf)
}
type bob struct { a int } //@item(a, "a", "int", "field")
