Protocol extension for 'implementors' #379
Conversation
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
| * | ||
| */ | ||
| @JsonRequest(value = "java/implementors", useSegment = false) | ||
| CompletableFuture<List<? extends SymbolInformation>> implementors(TextDocumentPositionParams position); |
There was a problem hiding this comment.
We probably need something more open for the parameter, eg. what if the request only contains an FCQN? I think we should have only 1 java/implementors entry point, but the incoming parameter should probably wrap TextDocumentPositionParams and some other stuff. Shooting from the hip:
ImplementorRequestParams {
- query {
- fullyQualifiedName: String (optional)
or - position : TextDocumentPositionParams (optional)
}
- options : Map<String, String> (optional)
}
We need more info from @tsmaeder about potential usage of that API in Che.
There was a problem hiding this comment.
Shouldn't we call it java/typeHierarchy instead?
| private List<SymbolInformation> findImplementations(IType type, IProgressMonitor monitor) throws JavaModelException { | ||
| IType[] results = type.newTypeHierarchy(monitor).getAllSubtypes(type); | ||
| final List<SymbolInformation> symbols = new ArrayList<>(); | ||
| for (IType t : results) { |
There was a problem hiding this comment.
return empty list if monitor is cancelled
| si.setName(name == null ? t.getElementName() : name); | ||
| si.setKind(t.isInterface() ? SymbolKind.Interface : SymbolKind.Class); | ||
| if (t.getParent() != null) { | ||
| si.setContainerName(t.getParent().getElementName()); |
There was a problem hiding this comment.
does t.getParent().getElementName() return a fcqn or simple name? We need to guarantee unambiguous parent hierarchy
|
@snjeza I think there's room for refactoring the existing codelens handler returning references here |
There was a problem hiding this comment.
According to eclipse-che/che#6157 (comment), for request options, typically we need to have at least 2 params:
- supertypes : boolean
- subtypes : boolean (true by default)
Please see #362 (comment) |
|
I think the requirement in #362 is incomplete, eclipse-che/che#6157 provides more info on what we should expect |
|
@tsmaeder Could you, please, complete your request? |
|
Abandoned in favour of a jdt.ls extension on the Che side: eclipse-che/che#6736 |
|
@tsmaeder @fbricon this is cool, but now that LSP supports protocol extensions it would be even cooler to have this as a protocol extension:
In this way both the LSP and other consumers of the eclipse.jdt.ls language server profit from this support. |
Fixes #362
Signed-off-by: Snjezana Peco snjezana.peco@redhat.com