internal/lsp: enable deep completion and fuzzy matching by default

Invert "useDeepCompletions" config flag to "disableDeepCompletion" and separate out "disableFuzzyMatching" which reverts to the previous prefix matching behavior. I separated fuzzy matching tests out to a separate file so they aren't entangled with deep completion tests. In coming up with representative test cases I found a couple issues which I fixed: - We were treating a fuzzy matcher score of 0 as no match, but the matcher returns 0 for candidates that match but have no bonuses. I changed the matcher interface so that a score of 0 counts as a match. For example, this was preventing a pattern of "o" from matching "foo". - When we lower a candidate's score based on its depth, we were subtracting a static multiplier which could result in the score going negative. A negative score messes up future score weighting because multiplying it by a value in the range [0, 1) makes it bigger instead of smaller. Fix by scaling a candidate's score based on its depth rather than subtracting a constant factor. Updates golang/go#32754 Change-Id: Ie6f9111f1696b0d067d08f7eed7b0a338ad9cd67 Reviewed-on: https://go-review.googlesource.com/c/tools/+/192137 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
golang · Aug 30, 2019 · 6afc7fc · 6afc7fc
1 parent c17b040
commit 6afc7fc
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 33 deletions.
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
@@ -24,7 +24,8 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 		return nil, err
 	}
 	candidates, surrounding, err := source.Completion(ctx, view, f, params.Position, source.CompletionOptions{
-		DeepComplete:          s.useDeepCompletions,
+		WantDeepCompletion:    !s.disableDeepCompletion,
+		WantFuzzyMatching:     !s.disableFuzzyMatching,
 		WantDocumentaton:      s.wantCompletionDocumentation,
 		WantFullDocumentation: s.hoverKind == fullDocumentation,
 		WantUnimported:        s.wantUnimportedCompletions,
@@ -49,7 +50,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	return &protocol.CompletionList{
 		// When using deep completions/fuzzy matching, report results as incomplete so
 		// client fetches updated completions after every key stroke.
-		IsIncomplete: s.useDeepCompletions,
+		IsIncomplete: !s.disableDeepCompletion,
 		Items:        s.toProtocolCompletionItems(candidates, rng),
 	}, nil
 }
@@ -63,7 +64,7 @@ func (s *Server) toProtocolCompletionItems(candidates []source.CompletionItem, r
 		// Limit the number of deep completions to not overwhelm the user in cases
 		// with dozens of deep completion matches.
 		if candidate.Depth > 0 {
-			if !s.useDeepCompletions {
+			if s.disableDeepCompletion {
 				continue
 			}
 			if numDeepCompletionsSeen >= source.MaxDeepCompletions {

diff --git a/internal/lsp/general.go b/internal/lsp/general.go
@@ -312,10 +312,8 @@ func (s *Server) processConfig(ctx context.Context, view source.View, config int
 		}
 	}
 
-	// Check if deep completions are enabled.
-	if useDeepCompletions, ok := c["useDeepCompletions"].(bool); ok {
-		s.useDeepCompletions = useDeepCompletions
-	}
+	s.disableDeepCompletion, _ = c["disableDeepCompletion"].(bool)
+	s.disableFuzzyMatching, _ = c["disableFuzzyMatching"].(bool)
 
 	// Check if want unimported package completions.
 	if wantUnimportedCompletions, ok := c["wantUnimportedCompletions"].(bool); ok {

diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
@@ -68,7 +68,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 					protocol.QuickFix:              true,
 				},
 				source.Mod: {},
-				source.Sum: {}},
+				source.Sum: {},
+			},
 			hoverKind: synopsisDocumentation,
 		},
 		data: data,
@@ -109,7 +110,8 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) {
 
 func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests.CompletionSnippets, items tests.CompletionItems) {
 	defer func() {
-		r.server.useDeepCompletions = false
+		r.server.disableDeepCompletion = true
+		r.server.disableFuzzyMatching = true
 		r.server.wantUnimportedCompletions = false
 		r.server.wantCompletionDocumentation = false
 	}()
@@ -122,7 +124,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			want = append(want, *items[pos])
 		}
 
-		r.server.useDeepCompletions = strings.Contains(string(src.URI()), "deepcomplete")
+		r.server.disableDeepCompletion = !strings.Contains(string(src.URI()), "deepcomplete")
+		r.server.disableFuzzyMatching = !strings.Contains(string(src.URI()), "fuzzymatch")
 		r.server.wantUnimportedCompletions = strings.Contains(string(src.URI()), "unimported")
 
 		list := r.runCompletion(t, src)
@@ -152,7 +155,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 		r.server.usePlaceholders = usePlaceholders
 
 		for src, want := range snippets {
-			r.server.useDeepCompletions = strings.Contains(string(src.URI()), "deepcomplete")
+			r.server.disableDeepCompletion = !strings.Contains(string(src.URI()), "deepcomplete")
+			r.server.disableFuzzyMatching = !strings.Contains(string(src.URI()), "fuzzymatch")
 			r.server.wantUnimportedCompletions = strings.Contains(string(src.URI()), "unimported")
 
 			list := r.runCompletion(t, src)

diff --git a/internal/lsp/server.go b/internal/lsp/server.go
@@ -80,7 +80,8 @@ type Server struct {
 	// TODO(rstambler): Separate these into their own struct?
 	usePlaceholders               bool
 	hoverKind                     hoverKind
-	useDeepCompletions            bool
+	disableDeepCompletion         bool
+	disableFuzzyMatching          bool
 	watchFileChanges              bool
 	wantCompletionDocumentation   bool
 	wantUnimportedCompletions     bool

diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
@@ -131,23 +131,21 @@ const (
 	completionBudget = 100 * time.Millisecond
 )
 
-// matcher matches a candidate's label against the user input.
-// The returned score reflects the quality of the match. A score
-// less than or equal to zero indicates no match, and a score of
-// one means a perfect match.
+// matcher matches a candidate's label against the user input.  The
+// returned score reflects the quality of the match. A score less than
+// zero indicates no match, and a score of one means a perfect match.
 type matcher interface {
 	Score(candidateLabel string) (score float32)
 }
 
-// prefixMatcher implements the original case insensitive prefix matching.
-// This matcher should go away once fuzzy matching is released.
+// prefixMatcher implements case insensitive prefix matching.
 type prefixMatcher string
 
 func (pm prefixMatcher) Score(candidateLabel string) float32 {
 	if strings.HasPrefix(strings.ToLower(candidateLabel), string(pm)) {
 		return 1
 	}
-	return 0
+	return -1
 }
 
 // completer contains the necessary information for a single completion request.
@@ -268,9 +266,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) {
 		},
 	}
 
-	// Fuzzy matching shares the "useDeepCompletions" config flag, so if deep completions
-	// are enabled then also enable fuzzy matching.
-	if c.deepState.maxDepth != 0 {
+	if c.opts.WantFuzzyMatching {
 		c.matcher = fuzzy.NewMatcher(c.surrounding.Prefix(), fuzzy.Symbol)
 	} else {
 		c.matcher = prefixMatcher(strings.ToLower(c.surrounding.Prefix()))
@@ -334,11 +330,19 @@ func (c *completer) found(obj types.Object, score float64, imp *imports.ImportIn
 	}
 
 	// Favor shallow matches by lowering weight according to depth.
-	cand.score -= stdScore * float64(len(c.deepState.chain))
+	cand.score -= cand.score * float64(len(c.deepState.chain)) / 10
+	if cand.score < 0 {
+		cand.score = 0
+	}
 
 	cand.name = c.deepState.chainString(obj.Name())
 	matchScore := c.matcher.Score(cand.name)
-	if matchScore > 0 {
+	if matchScore >= 0 {
+		// Avoid a score of zero since that homogenizes all candidates.
+		if matchScore == 0 {
+			matchScore = 0.001
+		}
+
 		cand.score *= float64(matchScore)
 
 		// Avoid calling c.item() for deep candidates that wouldn't be in the top
@@ -376,10 +380,11 @@ type candidate struct {
 }
 
 type CompletionOptions struct {
-	DeepComplete          bool
+	WantDeepCompletion    bool
 	WantDocumentaton      bool
 	WantFullDocumentation bool
 	WantUnimported        bool
+	WantFuzzyMatching     bool
 }
 
 // Completion returns a list of possible candidates for completion, given a
@@ -465,7 +470,8 @@ func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position,
 		startTime:      startTime,
 	}
 
-	if opts.DeepComplete {
+	if opts.WantDeepCompletion {
+		// Initialize max search depth to unlimited.
 		c.deepState.maxDepth = -1
 	}
 

diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
@@ -89,14 +89,16 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			t.Fatalf("failed for %v: %v", src, err)
 		}
 		deepComplete := strings.Contains(string(src.URI()), "deepcomplete")
+		fuzzyMatch := strings.Contains(string(src.URI()), "fuzzymatch")
 		unimported := strings.Contains(string(src.URI()), "unimported")
 		list, surrounding, err := source.Completion(ctx, r.view, f.(source.GoFile), protocol.Position{
 			Line:      float64(src.Start().Line() - 1),
 			Character: float64(src.Start().Column() - 1),
 		}, source.CompletionOptions{
-			DeepComplete:     deepComplete,
-			WantDocumentaton: true,
-			WantUnimported:   unimported,
+			WantDeepCompletion: deepComplete,
+			WantFuzzyMatching:  fuzzyMatch,
+			WantDocumentaton:   true,
+			WantUnimported:     unimported,
 		})
 		if err != nil {
 			t.Fatalf("failed for %v: %v", src, err)
@@ -121,7 +123,7 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			// If deep completion is enabled, we need to use the fuzzy matcher to match
 			// the code's behvaior.
 			if deepComplete {
-				if fuzzyMatcher != nil && fuzzyMatcher.Score(item.Label) <= 0 {
+				if fuzzyMatcher != nil && fuzzyMatcher.Score(item.Label) < 0 {
 					continue
 				}
 			} else {
@@ -143,11 +145,13 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			if err != nil {
 				t.Fatalf("failed for %v: %v", src, err)
 			}
+
 			list, _, err := source.Completion(ctx, r.view, f.(source.GoFile), protocol.Position{
 				Line:      float64(src.Start().Line() - 1),
 				Character: float64(src.Start().Column() - 1),
 			}, source.CompletionOptions{
-				DeepComplete: strings.Contains(string(src.URI()), "deepcomplete"),
+				WantDeepCompletion: strings.Contains(string(src.URI()), "deepcomplete"),
+				WantFuzzyMatching:  strings.Contains(string(src.URI()), "fuzzymatch"),
 			})
 			if err != nil {
 				t.Fatalf("failed for %v: %v", src, err)

diff --git a/internal/lsp/testdata/deepcomplete/deep_complete.go b/internal/lsp/testdata/deepcomplete/deep_complete.go
@@ -39,7 +39,7 @@ func _() {
 	}
 	var circle deepCircle   //@item(deepCircle, "circle", "deepCircle", "var")
 	circle.deepCircle       //@item(deepCircleField, "circle.deepCircle", "*deepCircle", "field")
-	var _ deepCircle = circ //@complete(" //", deepCircle, deepCircleStruct, deepCircleField)
+	var _ deepCircle = circ //@complete(" //", deepCircle, deepCircleField)
 }
 
 func _() {
@@ -70,3 +70,21 @@ func _() {
 		a: 123, //@complete(" //", deepNestedField)
 	}
 }
+
+func _() {
+	var a struct {
+		b struct {
+			c int
+		}
+		d int
+	}
+
+	a.d   //@item(deepAD, "a.d", "int", "field")
+	a.b.c //@item(deepABC, "a.b.c", "int", "field")
+	a.b   //@item(deepAB, "a.b", "struct{...}", "field")
+	a     //@item(deepA, "a", "struct{...}", "var")
+
+	// "a.d" should be ranked above the deeper "a.b.c"
+	var i int
+	i = a //@complete(" //", deepAD, deepABC, deepA, deepAB)
+}
diff --git a/internal/lsp/testdata/deepcomplete/fuzzymatch/deep_fuzzy.go b/internal/lsp/testdata/deepcomplete/fuzzymatch/deep_fuzzy.go
@@ -0,0 +1,50 @@
+// Copyright 2019 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 fuzzymatch
+
+func _() {
+	var a struct {
+		fabar  int
+		fooBar string
+	}
+
+	a.fabar  //@item(fuzzFabarField, "a.fabar", "int", "field")
+	a.fooBar //@item(fuzzFooBarField, "a.fooBar", "string", "field")
+
+	afa //@complete(" //", fuzzFabarField, fuzzFooBarField)
+	afb //@complete(" //", fuzzFooBarField, fuzzFabarField)
+
+	fab //@complete(" //", fuzzFabarField)
+
+	o //@complete(" //", fuzzFooBarField)
+
+	var myString string
+	myString = ar //@complete(" //", fuzzFooBarField, fuzzFabarField)
+
+	var b struct {
+		c struct {
+			d struct {
+				e struct {
+					abc string
+				}
+				abc float32
+			}
+			abc bool
+		}
+		abc int
+	}
+
+	b.abc       //@item(fuzzABCInt, "b.abc", "int", "field")
+	b.c.abc     //@item(fuzzABCbool, "b.c.abc", "bool", "field")
+	b.c.d.abc   //@item(fuzzABCfloat, "b.c.d.abc", "float32", "field")
+	b.c.d.e.abc //@item(fuzzABCstring, "b.c.d.e.abc", "string", "field")
+
+	// in depth order by default
+	abc //@complete(" //", fuzzABCInt, fuzzABCbool, fuzzABCfloat)
+
+	// deep candidate that matches expected type should still ranked first
+	var s string
+	s = abc //@complete(" //", fuzzABCstring, fuzzABCInt, fuzzABCbool)
+}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
@@ -29,7 +29,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       = 146
+	ExpectedCompletionsCount       = 154
 	ExpectedCompletionSnippetCount = 15
 	ExpectedDiagnosticsCount       = 21
 	ExpectedFormatCount            = 6