From 83d82311fd1f3a51bb8d03f6f8197c33e0b762a4 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 22 Oct 2019 09:28:04 -0700 Subject: [PATCH] internal/lsp: fix fuzzy matcher inconsistency Originally the fuzzy matcher required a match in the final candidate segment. For example, to match the candidate "foo.bar", the input had to have at least one character that matched "bar". I previously removed this requirement as it is too restrictive for deep completions to be useful. However, there was still some lingering final-segment favoritism in the matching algorithm. In particular, there were penalties for not matching the final segment's first character and for not matching the final segment's word initial characters. However, these penalties only made sense when we also required a final segment match. Consider this example: User input: "U" Candidate "ErrUnexpectedEOF" - with only a single segment, we got big penalties for not matching the leading "E" (since it is the final segment). Candidate "ErrUnexpectedEOF.Error" - "ErrUnexpectedEOF" is no longer the final segment, so we didn't get penalties. And we didn't get penalties for the final segment "Error" because we finished matching after the first "U". As a result, this candidate slips through with a higher score. Fix by simplifying the skip penalty. Now we only penalize for skipping the first character of the first or final segment (and the penalty is lower). For deep completions, the first and final segment are both "important" segments, so I think it makes sense to focus on both of them. We don't want to penalize all segment starts because that makes it harder to match deeper candidates where you often "ignore" intermediate segments. I had to adjust a few scores in the tests, but I don't think the impact will be too big other than fixing the bug. Fixes golang/go#35062. Change-Id: Id17a5c80bf0f80ce252fe990ccfbd51c1bac1c72 Reviewed-on: https://go-review.googlesource.com/c/tools/+/202638 Reviewed-by: Rebecca Stambler Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/fuzzy/matcher.go | 10 +++------ internal/lsp/fuzzy/matcher_test.go | 36 +++++++++++++++++++++++------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/internal/lsp/fuzzy/matcher.go b/internal/lsp/fuzzy/matcher.go index 17968536c4..0fbe5b3146 100644 --- a/internal/lsp/fuzzy/matcher.go +++ b/internal/lsp/fuzzy/matcher.go @@ -225,13 +225,9 @@ func (m *Matcher) computeScore(candidate string, candidateLower []byte) int { } var skipPenalty int - if segmentsLeft == 1 && isHead { - // Skipping a word. - skipPenalty++ - } - if i-1 == lastSegStart { - // Skipping the start of the last segment. - skipPenalty += 3 + if i == 1 || (i-1) == lastSegStart { + // Skipping the start of first or last segment. + skipPenalty += 1 } for j := 0; j <= pattLen; j++ { diff --git a/internal/lsp/fuzzy/matcher_test.go b/internal/lsp/fuzzy/matcher_test.go index 1472123357..8be25b4085 100644 --- a/internal/lsp/fuzzy/matcher_test.go +++ b/internal/lsp/fuzzy/matcher_test.go @@ -35,6 +35,12 @@ var ( }, descr: ">=", } + gt = comparator{ + f: func(val, ref float32) bool { + return val > ref + }, + descr: ">", + } ) func (c comparator) eval(val, ref float32) bool { @@ -82,6 +88,13 @@ var matcherTests = []struct { {"Ab stuff c", ge, 0}, }, }, + { + pattern: "U", + tests: []scoreTest{ + {"ErrUnexpectedEOF", gt, 0}, + {"ErrUnexpectedEOF.Error", eq, 0}, + }, + }, } func TestScore(t *testing.T) { @@ -110,10 +123,10 @@ var compareCandidatesTestCases = []struct { pattern: "Foo", orderedCandidates: []string{ "Barfoo", - "FaoFooa", - "BarFoo", "Faoo", "F_o_o", + "FaoFooa", + "BarFoo", "F__oo", "F_oo", "FooA", @@ -121,6 +134,13 @@ var compareCandidatesTestCases = []struct { "Foo", }, }, + { + pattern: "U", + orderedCandidates: []string{ + "ErrUnexpectedEOF.Error", + "ErrUnexpectedEOF", + }, + }, } func TestCompareCandidateScores(t *testing.T) { @@ -211,15 +231,15 @@ var scoreTestCases = []struct { {p: "strc", str: "StrCat", want: 1}, {p: "abc_def", str: "abc_def_xyz", want: 1}, {p: "abcdef", str: "abc_def_xyz", want: 0.91667}, - {p: "abcxyz", str: "abc_def_xyz", want: 0.875}, + {p: "abcxyz", str: "abc_def_xyz", want: 0.91667}, {p: "sc", str: "StrCat", want: 0.75}, - {p: "abc", str: "AbstrBasicCtor", want: 0.75}, - {p: "foo", str: "abc::foo", want: 1}, + {p: "abc", str: "AbstrBasicCtor", want: 0.83333}, + {p: "foo", str: "abc::foo", want: 0.91667}, {p: "afoo", str: "abc::foo", want: 0.9375}, {p: "abr", str: "abc::bar", want: 0.5}, - {p: "br", str: "abc::bar", want: 0.375}, - {p: "aar", str: "abc::bar", want: 0.16667}, - {p: "edin", str: "foo.TexteditNum", want: 0}, + {p: "br", str: "abc::bar", want: 0.25}, + {p: "aar", str: "abc::bar", want: 0.41667}, + {p: "edin", str: "foo.TexteditNum", want: 0.125}, {p: "ediu", str: "foo.TexteditNum", want: 0}, // We want the next two items to have roughly similar scores. {p: "up", str: "unique_ptr", want: 0.75},