internal/lsp/source: workspace symbol improvements for selectors
This CL adds various improvements for matching nested fields and
methods:
- Limit the symbols we produce to not show unqualified fields/methods,
and not show partial package paths.
- Handle embedded selectors, by trimming the package path.
- Improve the internal API used by symbolizers to operate on named
chunks.
Fixes golang/go#46997
Change-Id: I86cbe998adbb8e52549c937e330896134c375ed7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334531
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go
index c0aabf2..18583ae 100644
--- a/internal/lsp/source/workspace_symbol.go
+++ b/internal/lsp/source/workspace_symbol.go
@@ -57,79 +57,77 @@
// See the comment for symbolCollector for more information.
type matcherFunc func(name string) float64
-// A symbolizer returns the best symbol match for name with pkg, according to
-// some heuristic.
+// A symbolizer returns the best symbol match for a name with pkg, according to
+// some heuristic. The symbol name is passed as the slice nameParts of logical
+// name pieces. For example, for myType.field the caller can pass either
+// []string{"myType.field"} or []string{"myType.", "field"}.
//
// See the comment for symbolCollector for more information.
-type symbolizer func(name string, pkg Package, m matcherFunc) (string, float64)
+type symbolizer func(nameParts []string, pkg Package, m matcherFunc) (string, float64)
-func fullyQualifiedSymbolMatch(name string, pkg Package, matcher matcherFunc) (string, float64) {
- _, score := dynamicSymbolMatch(name, pkg, matcher)
+func fullyQualifiedSymbolMatch(nameParts []string, pkg Package, matcher matcherFunc) (string, float64) {
+ _, score := dynamicSymbolMatch(nameParts, pkg, matcher)
+ path := append([]string{pkg.PkgPath() + "."}, nameParts...)
if score > 0 {
- return pkg.PkgPath() + "." + name, score
+ return strings.Join(path, ""), score
}
return "", 0
}
-func dynamicSymbolMatch(name string, pkg Package, matcher matcherFunc) (string, float64) {
- // Prefer any package-qualified match.
- pkgQualified := pkg.Name() + "." + name
- if match, score := bestMatch(pkgQualified, matcher); match != "" {
- return match, score
+func dynamicSymbolMatch(nameParts []string, pkg Package, matcher matcherFunc) (string, float64) {
+ var best string
+ fullName := strings.Join(nameParts, "")
+ var score float64
+ var name string
+
+ // Compute the match score by finding the highest scoring suffix. In these
+ // cases the matched symbol is still the full name: it is confusing to match
+ // an unqualified nested field or method.
+ if match := bestMatch("", nameParts, matcher); match > score {
+ best = fullName
+ score = match
}
- fullyQualified := pkg.PkgPath() + "." + name
- if match, score := bestMatch(fullyQualified, matcher); match != "" {
- return match, score
+
+ // Next: try to match a package-qualified name.
+ name = pkg.Name() + "." + fullName
+ if match := matcher(name); match > score {
+ best = name
+ score = match
}
- return "", 0
+
+ // Finally: consider a fully qualified name.
+ prefix := pkg.PkgPath() + "."
+ fullyQualified := prefix + fullName
+ // As with field/method selectors, consider suffixes from right to left, but
+ // always return a fully-qualified symbol.
+ pathParts := strings.SplitAfter(prefix, "/")
+ if match := bestMatch(fullName, pathParts, matcher); match > score {
+ best = fullyQualified
+ score = match
+ }
+ return best, score
}
-func packageSymbolMatch(name string, pkg Package, matcher matcherFunc) (string, float64) {
- qualified := pkg.Name() + "." + name
+func bestMatch(name string, prefixParts []string, matcher matcherFunc) float64 {
+ var score float64
+ for i := len(prefixParts) - 1; i >= 0; i-- {
+ name = prefixParts[i] + name
+ if match := matcher(name); match > score {
+ score = match
+ }
+ }
+ return score
+}
+
+func packageSymbolMatch(components []string, pkg Package, matcher matcherFunc) (string, float64) {
+ path := append([]string{pkg.Name() + "."}, components...)
+ qualified := strings.Join(path, "")
if matcher(qualified) > 0 {
return qualified, 1
}
return "", 0
}
-// bestMatch returns the highest scoring symbol suffix of fullPath, starting
-// from the right and splitting on selectors and path components.
-//
-// e.g. given a symbol path of the form 'host.com/dir/pkg.type.field', we
-// check the match quality of the following:
-// - field
-// - type.field
-// - pkg.type.field
-// - dir/pkg.type.field
-// - host.com/dir/pkg.type.field
-//
-// and return the best match, along with its score.
-//
-// This is used to implement the 'dynamic' symbol style.
-func bestMatch(fullPath string, matcher matcherFunc) (string, float64) {
- pathParts := strings.Split(fullPath, "/")
- dottedParts := strings.Split(pathParts[len(pathParts)-1], ".")
-
- var best string
- var score float64
-
- for i := 0; i < len(dottedParts); i++ {
- path := strings.Join(dottedParts[len(dottedParts)-1-i:], ".")
- if match := matcher(path); match > score {
- best = path
- score = match
- }
- }
- for i := 0; i < len(pathParts); i++ {
- path := strings.Join(pathParts[len(pathParts)-1-i:], "/")
- if match := matcher(path); match > score {
- best = path
- score = match
- }
- }
- return best, score
-}
-
// symbolCollector holds context as we walk Packages, gathering symbols that
// match a given query.
//
@@ -420,7 +418,13 @@
// or named. path is the path of nested identifiers containing the field.
func (sc *symbolCollector) walkField(field *ast.Field, unnamedKind, namedKind protocol.SymbolKind, path ...*ast.Ident) {
if len(field.Names) == 0 {
- sc.match(types.ExprString(field.Type), unnamedKind, field, path...)
+ switch typ := field.Type.(type) {
+ case *ast.SelectorExpr:
+ // embedded qualified type
+ sc.match(typ.Sel.Name, unnamedKind, field, path...)
+ default:
+ sc.match(types.ExprString(field.Type), unnamedKind, field, path...)
+ }
}
for _, name := range field.Names {
sc.match(name.Name, namedKind, name, path...)
@@ -466,18 +470,14 @@
}
isExported := isExported(name)
- if len(path) > 0 {
- var nameBuilder strings.Builder
- for _, ident := range path {
- nameBuilder.WriteString(ident.Name)
- nameBuilder.WriteString(".")
- if !ident.IsExported() {
- isExported = false
- }
+ var names []string
+ for _, ident := range path {
+ names = append(names, ident.Name+".")
+ if !ident.IsExported() {
+ isExported = false
}
- nameBuilder.WriteString(name)
- name = nameBuilder.String()
}
+ names = append(names, name)
// Factors to apply to the match score for the purpose of downranking
// results.
@@ -501,7 +501,7 @@
// can be noisy.
fieldFactor = 0.5
)
- symbol, score := sc.symbolizer(name, sc.current.pkg, sc.matcher)
+ symbol, score := sc.symbolizer(names, sc.current.pkg, sc.matcher)
// Downrank symbols outside of the workspace.
if !sc.current.isWorkspace {
diff --git a/internal/lsp/source/workspace_symbol_test.go b/internal/lsp/source/workspace_symbol_test.go
index f3d9dbb..def73ce 100644
--- a/internal/lsp/source/workspace_symbol_test.go
+++ b/internal/lsp/source/workspace_symbol_test.go
@@ -5,7 +5,6 @@
package source
import (
- "strings"
"testing"
)
@@ -45,53 +44,3 @@
}
}
}
-
-func TestBestMatch(t *testing.T) {
- tests := []struct {
- desc string
- symbol string
- matcher matcherFunc
- wantMatch string
- wantScore float64
- }{
- {
- desc: "shortest match",
- symbol: "foo/bar/baz.quux",
- matcher: func(string) float64 { return 1.0 },
- wantMatch: "quux",
- wantScore: 1.0,
- },
- {
- desc: "partial match",
- symbol: "foo/bar/baz.quux",
- matcher: func(s string) float64 {
- if strings.HasPrefix(s, "bar") {
- return 1.0
- }
- return 0.0
- },
- wantMatch: "bar/baz.quux",
- wantScore: 1.0,
- },
- {
- desc: "longest match",
- symbol: "foo/bar/baz.quux",
- matcher: func(s string) float64 {
- parts := strings.Split(s, "/")
- return float64(len(parts))
- },
- wantMatch: "foo/bar/baz.quux",
- wantScore: 3.0,
- },
- }
-
- for _, test := range tests {
- test := test
- t.Run(test.desc, func(t *testing.T) {
- gotMatch, gotScore := bestMatch(test.symbol, test.matcher)
- if gotMatch != test.wantMatch || gotScore != test.wantScore {
- t.Errorf("bestMatch(%q, matcher) = (%q, %.2g), want (%q, %.2g)", test.symbol, gotMatch, gotScore, test.wantMatch, test.wantScore)
- }
- })
- }
-}
