-
Notifications
You must be signed in to change notification settings - Fork 12
Parametric LSP: call hierarchy API #688
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
base: main
Are you sure you want to change the base?
Conversation
8b1c6c5 to
335278e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've written down some thoughts, let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small and a smaller comment.
|
IMHO it is essential for efficiency that we know in advance if we are looking for callers or callees.
So those two are significantly different algorithms in general. If Bird accidentally does not make the same distinction, then that's an exception rather than a rule IMHO. Especially the "closed world" aspect has big impact on both usability (what else do you need it for?) and efficiency (Now we need up-to-date summaries of all possible caller files). |
|
I'm worried that with the current setup DSL developers will need to re-invent their reverse modular indexing again and again for every language. At the same time we already have the Summary concept for indexing a single file, and summaries have been designed to be modularly composable. So we are pretty close... Let's take a step back and see what that means in general, and if we can factor out that aspect in a language contribution by itself? Or perhaps a general feature.. I don't know. |
|
Funny note: this indexing feature was why IBM was interested in |
|
Okay, I had a call with @jurgenvinju and we decided:
|
cec9230 to
223345c
Compare
523c2fe to
ab448fc
Compare
223345c to
811779a
Compare
b79c77e to
7a9d722
Compare
811779a to
e1a09ad
Compare
fb44e22 to
0e08fe4
Compare
f57c97f to
6998a70
Compare
6998a70 to
f71dbb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just 2 small things, but this looks close to merging.
Also: please add an entry to the rascal-vscode-extension/CHANGELOG.md
rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java
Outdated
Show resolved
Hide resolved
| import io.usethesource.vallang.type.TypeFactory; | ||
| import io.usethesource.vallang.type.TypeStore; | ||
|
|
||
| public class CallHierarchy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a small comment on why this class exists, what its purpose is.
| private static final String NAME = "name"; | ||
| private static final String KIND = "kind"; | ||
| private static final String DEFINITION = "src"; | ||
| private static final String SELECTION = "selection"; | ||
| private static final String TAGS = "tags"; | ||
| private static final String DETAIL = "detail"; | ||
| private static final String DATA = "data"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if all these fields are used once, why are they in a separate static field? I do not see how that makes the code more readable? cons.get(NAME) vs cons.get("name")?
94b7827 to
5c0a0b5
Compare
|



This PR designs and implements
textDocument/prepareCallHierarchy,callHierarchy/incomingCalls, andcallHierarchy/outgoingCallsfor parametric language servers, allowing DSLs to implement call hierarchies.Design
This design collapses the incoming/outgoing calls services into, since they are very similar. It also groups
prepareCallHierarchyin the same constructor, since neither can exist without the other.Since a single cursor can produce multiple hierarchy items (e.g. in the case of overloads), and the implementer might want to influence the order, we require a
list/lrel.Alternatives considered
Summaryin an attempt to make call to the hierarchy services cheaper and simpler. This requries quite some additions to the summary, means that a hierarchy always requires a summary to be present, and additionally makes passing around the optional state (value datafield) cumbersome.An example of this implementation can be found here: https://github.com/SWAT-engineering/bird/blob/demo/call-hierarchy/bird-ide/src/main/rascal/lang/bird/LanguageServer.rsc
Closes #132