Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Commit

Permalink
fix(search): correctly handle select:symbol.enum (#64170)
Browse files Browse the repository at this point in the history
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](sourcegraph/zoekt#674)
and add other missing kinds (besides enum)

Closes SPLF-178
  • Loading branch information
jtibshirani authored Jul 31, 2024
1 parent d17660d commit f4a07a8
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 130 deletions.
9 changes: 7 additions & 2 deletions internal/search/result/match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,26 @@ 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)
}
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) {
Expand Down
228 changes: 102 additions & 126 deletions internal/search/result/symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
4 changes: 2 additions & 2 deletions internal/search/streaming/search_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f4a07a8

Please sign in to comment.