From f4a07a8b1258b99d926f388ea1fcd819f8c9ed56 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Wed, 31 Jul 2024 12:07:27 +0300 Subject: [PATCH] fix(search): correctly handle select:symbol.enum (#64170) To implement `select:symbol.enum` filters, we look at each symbol's ctags 'kind' and check if it matches the filter value `enum`. We accidentally didn't include 'enum' in this match logic, so all these symbols were filtered away. This PR fixes that, and adds a few improvements: * Use a shared map between `symbol.LSPKind` and `symbol.SelectKind`, to avoid drift between these two conversions. * Audit the ctags mapping from [sourcegraph/zoekt#674](https://github.com/sourcegraph/zoekt/pull/674) and add other missing kinds (besides enum) Closes SPLF-178 --- internal/search/result/match_test.go | 9 +- internal/search/result/symbol.go | 228 +++++++++----------- internal/search/streaming/search_filters.go | 4 +- 3 files changed, 111 insertions(+), 130 deletions(-) diff --git a/internal/search/result/match_test.go b/internal/search/result/match_test.go index 219fa6dc7764..65ce07eec87f 100644 --- a/internal/search/result/match_test.go +++ b/internal/search/result/match_test.go @@ -22,12 +22,16 @@ func TestSelect(t *testing.T) { {Symbol: Symbol{Name: "a()", Kind: "func"}}, {Symbol: Symbol{Name: "b()", Kind: "function"}}, {Symbol: Symbol{Name: "var c", Kind: "variable"}}, + {Symbol: Symbol{Name: "enum d", Kind: "enumerator"}}, + {Symbol: Symbol{Name: "enum e", Kind: "enum"}}, }, } test := func(input string) string { selectPath, _ := filter.SelectPathFromString(input) - symbols := data.Select(selectPath).(*FileMatch).Symbols + + dataCopy := *data // copy the data because Select modifies the set of symbols + symbols := dataCopy.Select(selectPath).(*FileMatch).Symbols var values []string for _, s := range symbols { values = append(values, s.Symbol.Name+":"+s.Symbol.Kind) @@ -35,8 +39,9 @@ func TestSelect(t *testing.T) { return strings.Join(values, ", ") } - autogold.Expect("a():func, b():function, var c:variable").Equal(t, test("symbol")) + autogold.Expect("a():func, b():function, var c:variable, enum d:enumerator, enum e:enum").Equal(t, test("symbol")) autogold.Expect("var c:variable").Equal(t, test("symbol.variable")) + autogold.Expect("enum d:enumerator, enum e:enum").Equal(t, test("symbol.enum")) }) t.Run("path match", func(t *testing.T) { diff --git a/internal/search/result/symbol.go b/internal/search/result/symbol.go index b2b1851d97cd..f5de74fe1dd9 100644 --- a/internal/search/result/symbol.go +++ b/internal/search/result/symbol.go @@ -60,62 +60,31 @@ func NewSymbolMatch(file *File, lineNumber, character int, name, kind, parent, p } func (s Symbol) LSPKind() lsp.SymbolKind { - // Ctags kinds are determined by the parser and do not (in general) match LSP symbol kinds. - switch strings.ToLower(s.Kind) { - case "file": - return lsp.SKFile - case "module": - return lsp.SKModule - case "namespace": - return lsp.SKNamespace - case "package", "packagename", "subprogspec": - return lsp.SKPackage - case "class", "classes", "type", "service", "typedef", "union", "section", "subtype", "component": - return lsp.SKClass - case "method", "methodspec": - return lsp.SKMethod - case "property": - return lsp.SKProperty - case "field", "member", "anonmember", "recordfield": - return lsp.SKField - case "constructor": - return lsp.SKConstructor - case "enum", "enumerator": - return lsp.SKEnum - case "interface": - return lsp.SKInterface - case "function", "func", "subroutine", "macro", "subprogram", "procedure", "command", "singletonmethod": - return lsp.SKFunction - case "variable", "var", "functionvar", "define", "alias", "val": - return lsp.SKVariable - case "constant", "const": - return lsp.SKConstant - case "string", "message", "heredoc": - return lsp.SKString - case "number": - return lsp.SKNumber - case "bool", "boolean": - return lsp.SKBoolean - case "array": - return lsp.SKArray - case "object", "literal", "map": - return lsp.SKObject - case "key", "label", "target", "selector", "id", "tag": - return lsp.SKKey - case "null": - return lsp.SKNull - case "enum member", "enumconstant": - return lsp.SKEnumMember - case "struct": - return lsp.SKStruct - case "event": - return lsp.SKEvent - case "operator": - return lsp.SKOperator - case "type parameter", "annotation": - return lsp.SKTypeParameter + kind, ok := lspKinds[s.Kind] + if !ok { + return 0 + } + return kind +} + +// SelectKind maps an internal symbol kind (cf. ctagsKind) to a corresponding +// symbol selector kind value in select.go. The single selector value `kind` +// corresponds 1-to-1 with LSP symbol kinds. +func (s Symbol) SelectKind() (string, bool) { + kind, ok := lspKinds[s.Kind] + if !ok { + return "", false + } + + kindName := strings.ToLower(kind.String()) + switch kindName { + case "enummember": + return "enum-member", true + case "typeparameter": + return "type-parameter", true + default: + return kindName, true } - return 0 } func (s Symbol) Range() lsp.Range { @@ -198,76 +167,6 @@ func lineSpecFromPosition(pos lsp.Position, forceIncludeCharacter bool) string { return fmt.Sprintf("%d:%d", pos.Line+1, pos.Character+1) } -// toSelectKind maps an internal symbol kind (cf. ctagsKind) to a corresponding -// symbol selector kind value in select.go. The single selector value `kind` -// corresponds 1-to-1 with LSP symbol kinds. -var ToSelectKind = map[string]string{ - "file": "file", - "module": "module", - "namespace": "namespace", - "package": "package", - "packagename": "package", - "subprogspec": "package", - "class": "class", - "classes": "class", - "type": "class", - "service": "class", - "typedef": "class", - "union": "class", - "section": "class", - "subtype": "class", - "component": "class", - "method": "method", - "methodspec": "method", - "property": "property", - "field": "field", - "member": "field", - "anonmember": "field", - "recordfield": "field", - "constructor": "constructor", - "interface": "interface", - "function": "function", - "func": "function", - "subroutine": "function", - "macro": "function", - "subprogram": "function", - "procedure": "function", - "command": "function", - "singletonmethod": "function", - "variable": "variable", - "var": "variable", - "functionvar": "variable", - "define": "variable", - "alias": "variable", - "val": "variable", - "constant": "constant", - "const": "constant", - "string": "string", - "message": "string", - "heredoc": "string", - "number": "number", - "boolean": "boolean", - "bool": "boolean", - "array": "array", - "object": "object", - "literal": "object", - "map": "object", - "key": "key", - "label": "key", - "target": "key", - "selector": "key", - "id": "key", - "tag": "key", - "null": "null", - "enum member": "enum-member", - "enumconstant": "enum-member", - "struct": "struct", - "event": "event", - "operator": "operator", - "type parameter": "type-parameter", - "annotation": "type-parameter", -} - func pick(symbols []*SymbolMatch, satisfy func(*SymbolMatch) bool) []*SymbolMatch { var result []*SymbolMatch for _, symbol := range symbols { @@ -280,6 +179,83 @@ func pick(symbols []*SymbolMatch, satisfy func(*SymbolMatch) bool) []*SymbolMatc func SelectSymbolKind(symbols []*SymbolMatch, field string) []*SymbolMatch { return pick(symbols, func(s *SymbolMatch) bool { - return field == ToSelectKind[strings.ToLower(s.Symbol.Kind)] + kind, ok := s.Symbol.SelectKind() + return ok && field == kind }) } + +// lspKinds maps a ctags kind to an LSP symbol kind. Ctags kinds are determined +// by the parser and do not (in general) match LSP symbol kinds. +var lspKinds = map[string]lsp.SymbolKind{ + "file": lsp.SKFile, + "module": lsp.SKModule, + "namespace": lsp.SKNamespace, + "package": lsp.SKPackage, + "packagename": lsp.SKPackage, + "subprogspec": lsp.SKPackage, + "class": lsp.SKClass, + "classes": lsp.SKClass, + "type": lsp.SKClass, + "service": lsp.SKClass, + "typedef": lsp.SKClass, + "union": lsp.SKClass, + "section": lsp.SKClass, + "subtype": lsp.SKClass, + "component": lsp.SKClass, + "accessor": lsp.SKMethod, + "getter": lsp.SKMethod, + "method": lsp.SKMethod, + "methodalias": lsp.SKMethod, + "methodspec": lsp.SKMethod, + "setter": lsp.SKMethod, + "singletonmethod": lsp.SKFunction, + "property": lsp.SKProperty, + "field": lsp.SKField, + "member": lsp.SKField, + "anonmember": lsp.SKField, + "recordfield": lsp.SKField, + "constructor": lsp.SKConstructor, + "enum": lsp.SKEnum, + "enumerator": lsp.SKEnum, + "interface": lsp.SKInterface, + "function": lsp.SKFunction, + "func": lsp.SKFunction, + "subroutine": lsp.SKFunction, + "macro": lsp.SKFunction, + "subprogram": lsp.SKFunction, + "procedure": lsp.SKFunction, + "command": lsp.SKFunction, + "variable": lsp.SKVariable, + "var": lsp.SKVariable, + "functionvar": lsp.SKVariable, + "define": lsp.SKVariable, + "alias": lsp.SKVariable, + "val": lsp.SKVariable, + "constant": lsp.SKConstant, + "const": lsp.SKConstant, + "string": lsp.SKString, + "message": lsp.SKString, + "heredoc": lsp.SKString, + "number": lsp.SKNumber, + "bool": lsp.SKBoolean, + "boolean": lsp.SKBoolean, + "array": lsp.SKArray, + "object": lsp.SKObject, + "literal": lsp.SKObject, + "map": lsp.SKObject, + "key": lsp.SKKey, + "label": lsp.SKKey, + "target": lsp.SKKey, + "selector": lsp.SKKey, + "id": lsp.SKKey, + "tag": lsp.SKKey, + "null": lsp.SKNull, + "enum member": lsp.SKEnumMember, + "enumconstant": lsp.SKEnumMember, + "enummember": lsp.SKEnumMember, + "struct": lsp.SKStruct, + "event": lsp.SKEvent, + "operator": lsp.SKOperator, + "type parameter": lsp.SKTypeParameter, + "annotation": lsp.SKTypeParameter, +} diff --git a/internal/search/streaming/search_filters.go b/internal/search/streaming/search_filters.go index 1b8c5577bfb7..65d7e6ccc304 100644 --- a/internal/search/streaming/search_filters.go +++ b/internal/search/streaming/search_filters.go @@ -169,10 +169,10 @@ func (s *SearchFilters) Update(event SearchEvent) { addSymbolFilter := func(symbols []*result.SymbolMatch) { for _, sym := range symbols { - selectKind, ok := result.ToSelectKind[strings.ToLower(sym.Symbol.Kind)] + selectKind, ok := sym.Symbol.SelectKind() if !ok { // Skip any symbols we don't know how to select - // TODO(@camdencheek): figure out which symbols are missing from result.ToSelectKind + // TODO(@camdencheek): figure out which symbols are missing from symbol.SelectKind continue } filter := fmt.Sprintf(`select:symbol.%s`, selectKind)