internal/lsp/source: fix bug in deep completion score tracking
We keep track of the N highest seen scores so we can quickly skip deep
completions not in the top N. Our logic for maintaining the top N list
wasn't quite right, resulting in certain cases where we would let
non-high scoring candidates through. I don't think the bug impacted
correctness.
Change-Id: Ic105617523c82f0e71e4f95ef0ee216182a84252
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247418
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go
index 2b57782..2afff8b 100644
--- a/internal/lsp/source/deep_completion.go
+++ b/internal/lsp/source/deep_completion.go
@@ -73,22 +73,19 @@
// Invariant: s.highScores is sorted with highest score first. Unclaimed
// positions are trailing zeros.
- // First check for an unclaimed spot and claim if available.
+ // If we beat an existing score then take its spot.
for i, deepScore := range s.highScores {
- if deepScore == 0 {
- s.highScores[i] = score
- return true
+ if score <= deepScore {
+ continue
}
- }
- // Otherwise, if we beat an existing score then take its spot and scoot
- // all lower scores down one position.
- for i, deepScore := range s.highScores {
- if score > deepScore {
+ if deepScore != 0 && i != len(s.highScores)-1 {
+ // If this wasn't an empty slot then we need to scooch everyone
+ // down one spot.
copy(s.highScores[i+1:], s.highScores[i:])
- s.highScores[i] = score
- return true
}
+ s.highScores[i] = score
+ return true
}
return false
diff --git a/internal/lsp/source/deep_completion_test.go b/internal/lsp/source/deep_completion_test.go
new file mode 100644
index 0000000..47d5179
--- /dev/null
+++ b/internal/lsp/source/deep_completion_test.go
@@ -0,0 +1,33 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package source
+
+import (
+ "testing"
+)
+
+func TestDeepCompletionIsHighScore(t *testing.T) {
+ // Test that deepCompletionState.isHighScore properly tracks the top
+ // N=MaxDeepCompletions scores.
+
+ var s deepCompletionState
+
+ if !s.isHighScore(1) {
+ // No other scores yet, anything is a winner.
+ t.Error("1 should be high score")
+ }
+
+ // Fill up with higher scores.
+ for i := 0; i < MaxDeepCompletions; i++ {
+ if !s.isHighScore(10) {
+ t.Error("10 should be high score")
+ }
+ }
+
+ // High scores should be filled with 10s so 2 is not a high score.
+ if s.isHighScore(2) {
+ t.Error("2 shouldn't be high score")
+ }
+}
