Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SymbolInformation.kind #156

Merged
merged 4 commits into from
May 23, 2023
Merged

Add SymbolInformation.kind #156

merged 4 commits into from
May 23, 2023

Conversation

olafurpg
Copy link
Member

This is the first step towards closing #154.

Previously, the only way to determine the type of a symbol was through SymbolDescriptor.Suffix. This solution was not ideal since Suffix had very coarse grained types, and it was intentionally named "suffix" to indicate it was primarily used to determine the syntax of the symbol (not semantics).

This PR adds a new enum SymbolInformation.Kind to add first-class support to determine the type of a symbol.

Test plan

Green CI.

This is the first step towards closing #154.

Previously, the only way to determine the type of a symbol was through `SymbolDescriptor.Suffix`. This solution was not ideal since `Suffix` had very coarse grained types, and it was intentionally named "suffix" to indicate it was primarily used to determine the syntax of the symbol (not semantics).

This PR adds a new enum `SymbolInformation.Kind` to add first-class support to determine the type of a symbol.
@olafurpg
Copy link
Member Author

cc/ @donsbot if you have a moment to review.

scip.proto Outdated Show resolved Hide resolved
scip.proto Outdated Show resolved Hide resolved
scip.proto Outdated Show resolved Hide resolved
scip.proto Outdated Show resolved Hide resolved
scip.proto Outdated Show resolved Hide resolved
scip.proto Outdated Show resolved Hide resolved
@olafurpg
Copy link
Member Author

olafurpg commented May 23, 2023

I tried to include the union of kinds from Glean, LSP, and SemanticDB. One open question for me is whether we should order the kinds alphabetically or by the LSP order (and add custom Glean kinds at the end). My preference is to use alphabetical order like we did for Language and add a commented out NextKind = NUMBER; at the bottom to make it easy to add new kinds.

@varungandhi-src
Copy link
Contributor

cc @donsbot @olafurpg in case you have any more feedback. I may have gone a bit over-the-top with adding kinds of many different languages. 😅

If there's no more feedback, we can merge this.

@donsbot
Copy link
Contributor

donsbot commented May 23, 2023

I think it's safe to be wider than current specs (eg LSP). I had to add a few for our uses. Clients will refine the set if needed. Most indexers emit a small set is kinds, but do care about getting naming right

@varungandhi-src varungandhi-src merged commit 4e11295 into main May 23, 2023
@varungandhi-src varungandhi-src deleted the olafurpg/kind branch May 23, 2023 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants