-
Notifications
You must be signed in to change notification settings - Fork 824
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
Support Type Hierarchy #1231
Support Type Hierarchy #1231
Changes from all commits
4c847a4
a2be815
21db26e
d4cc98e
2aa99a1
03ee936
8cab387
90090af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1927,6 +1927,13 @@ export interface TextDocumentClientCapabilities { | |
* @since 3.16.0 | ||
*/ | ||
moniker?: MonikerClientCapabilities; | ||
|
||
/** | ||
* Capabilities specific to the various type hierarchy requests. | ||
* | ||
* @since 3.17.0 | ||
*/ | ||
typeHierarchy?: TypeHierarchyClientCapabilities; | ||
} | ||
``` | ||
|
||
|
@@ -2419,6 +2426,14 @@ interface ServerCapabilities { | |
} | ||
} | ||
|
||
/** | ||
* The server provides type hierarchy support. | ||
* | ||
* @since 3.17.0 | ||
*/ | ||
typeHierarchyProvider?: boolean | TypeHierarchyOptions | ||
| TypeHierarchyRegistrationOptions; | ||
|
||
/** | ||
* Experimental server capabilities. | ||
*/ | ||
|
@@ -8173,6 +8188,164 @@ export interface Moniker { | |
} | ||
``` | ||
|
||
#### <a href="#textDocument_prepareTypeHierarchy" name="textDocument_prepareTypeHierarchy" class="anchor">Prepare Type Hierarchy Request (:leftwards_arrow_with_hook:)</a> | ||
|
||
> *Since version 3.17.0* | ||
|
||
The type hierarchy request is sent from the client to the server to return a type hierarchy for the language element of given text document positions. Will return `null` if the server couldn't infer a valid type from the position. The type hierarchy requests are executed in two steps: | ||
|
||
1. first a type hierarchy item is prepared for the given text document position. | ||
1. for a type hierarchy item the supertype or subtype type hierarchy items are resolved. | ||
|
||
_Client Capability_: | ||
|
||
* property name (optional): `textDocument.typeHierarchy` | ||
* property type: `TypeHierarchyClientCapabilities` defined as follows: | ||
|
||
```typescript | ||
interface TypeHierarchyClientCapabilities { | ||
/** | ||
* Whether implementation supports dynamic registration. If this is set to | ||
* `true` the client supports the new `(TextDocumentRegistrationOptions & | ||
* StaticRegistrationOptions)` return value for the corresponding server | ||
* capability as well. | ||
*/ | ||
dynamicRegistration?: boolean; | ||
} | ||
``` | ||
|
||
_Server Capability_: | ||
|
||
* property name (optional): `typeHierarchyProvider` | ||
* property type: `boolean | TypeHierarchyOptions | TypeHierarchyRegistrationOptions` where `TypeHierarchyOptions` is defined as follows: | ||
|
||
```typescript | ||
export interface TypeHierarchyOptions extends WorkDoneProgressOptions { | ||
} | ||
``` | ||
|
||
_Registration Options_: `TypeHierarchyRegistrationOptions` defined as follows: | ||
|
||
```typescript | ||
export interface TypeHierarchyRegistrationOptions extends | ||
TextDocumentRegistrationOptions, TypeHierarchyOptions, | ||
StaticRegistrationOptions { | ||
} | ||
``` | ||
|
||
_Request_: | ||
|
||
* method: 'textDocument/prepareTypeHierarchy' | ||
* params: `TypeHierarchyPrepareParams` defined as follows: | ||
|
||
```typescript | ||
export interface TypeHierarchyPrepareParams extends TextDocumentPositionParams, | ||
WorkDoneProgressParams { | ||
} | ||
``` | ||
|
||
_Response_: | ||
|
||
* result: `TypeHierarchyItem[] | null` defined as follows: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why should we allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the Thanks for the comment, I'll add a description in the next commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
```typescript | ||
export interface TypeHierarchyItem { | ||
/** | ||
* The name of this item. | ||
*/ | ||
name: string; | ||
|
||
/** | ||
* The kind of this item. | ||
*/ | ||
kind: SymbolKind; | ||
|
||
/** | ||
* Tags for this item. | ||
*/ | ||
tags?: SymbolTag[]; | ||
|
||
/** | ||
* More detail for this item, e.g. the signature of a function. | ||
*/ | ||
detail?: string; | ||
|
||
/** | ||
* The resource identifier of this item. | ||
*/ | ||
uri: DocumentUri; | ||
|
||
/** | ||
* The range enclosing this symbol not including leading/trailing whitespace | ||
* but everything else, e.g. comments and code. | ||
*/ | ||
range: Range; | ||
|
||
/** | ||
* The range that should be selected and revealed when this symbol is being | ||
* picked, e.g. the name of a function. Must be contained by the | ||
* [`range`](#TypeHierarchyItem.range). | ||
*/ | ||
selectionRange: Range; | ||
|
||
/** | ||
* A data entry field that is preserved between a type hierarchy prepare and | ||
* supertypes or subtypes requests. It could also be used to identify the | ||
* type hierarchy in the server, helping improve the performance on | ||
* resolving supertypes and subtypes. | ||
*/ | ||
data?: unknown; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make more sense for the prepare response to be an object (versus an array), and have a single data field? I guess I'm not sure to what extent the full call needs data versus each individual item (where if you really just wanted the same data, you'd have to dupe it on each of them). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, in clangd, what we put in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we need a data field in each individual item would depend on server. If server could hold or cache the whole type hierarchy and use it to find a type in coming requests, one data field is OK, but if server wants to use |
||
} | ||
``` | ||
|
||
* error: code and message set in case an exception happens during the 'textDocument/prepareTypeHierarchy' request | ||
|
||
#### <a href="#typeHierarchy_supertypes" name="typeHierarchy_supertypes" class="anchor">Type Hierarchy Supertypes(:leftwards_arrow_with_hook:)</a> | ||
|
||
> *Since version 3.17.0* | ||
|
||
The request is sent from the client to the server to resolve the supertypes for a given type hierarchy item. Will return `null` if the server couldn't infer a valid type from `item` in the params. The request doesn't define its own client and server capabilities. It is only issued if a server registers for the [`textDocument/prepareTypeHierarchy` request](#textDocument_prepareTypeHierarchy). | ||
|
||
_Request_: | ||
|
||
* method: 'typeHierarchy/supertypes' | ||
* params: `TypeHierarchySupertypesParams` defined as follows: | ||
|
||
```typescript | ||
export interface TypeHierarchySupertypesParams extends | ||
WorkDoneProgressParams, PartialResultParams { | ||
item: TypeHierarchyItem; | ||
} | ||
``` | ||
_Response_: | ||
|
||
* result: `TypeHierarchyItem[] | null` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does |
||
* partial result: `TypeHierarchyItem[]` | ||
* error: code and message set in case an exception happens during the 'typeHierarchy/supertypes' request | ||
|
||
#### <a href="#typeHierarchy_subtypes" name="typeHierarchy_subtypes" class="anchor">Type Hierarchy Subtypes(:leftwards_arrow_with_hook:)</a> | ||
|
||
> *Since version 3.17.0* | ||
|
||
The request is sent from the client to the server to resolve the subtypes for a given type hierarchy item. Will return `null` if the server couldn't infer a valid type from `item` in the params. The request doesn't define its own client and server capabilities. It is only issued if a server registers for the [`textDocument/prepareTypeHierarchy` request](#textDocument_prepareTypeHierarchy). | ||
|
||
_Request_: | ||
|
||
* method: 'typeHierarchy/subtypes' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not obvious why there are so many messages here, nor why the prepare and resolve steps are required. For clients (and presumably many servers), it would be simpler to return the whole tree in a single message. With this current API proposal there will be a lot of chatter for an ostensibly simple dataset. Could we perhaps incorporate the ability for the server to return the full type hierarchy in response to just a simple It's also unclear to me when a client should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as you mentioned, calculating the full type hierarchy is extremely expensive for a given type with many subtypes. We currently have an implementation in Java language server, it may cost about 25 seconds to calculate the direct subtypes of For the second question, we have two common views for all languages, they are |
||
* params: `TypeHierarchySubtypesParams` defined as follows: | ||
|
||
```typescript | ||
export interface TypeHierarchySubtypesParams extends | ||
WorkDoneProgressParams, PartialResultParams { | ||
item: TypeHierarchyItem; | ||
} | ||
``` | ||
_Response_: | ||
|
||
* result: `TypeHierarchyItem[] | null` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, what's |
||
* partial result: `TypeHierarchyItem[]` | ||
* error: code and message set in case an exception happens during the 'typeHierarchy/subtypes' request | ||
|
||
##### Notes | ||
|
||
Server implementations of this method should ensure that the moniker calculation matches to those used in the corresponding LSIF implementation to ensure symbols can be associated correctly across IDE sessions and LSIF indexes. | ||
|
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.
Is a simple
boolean
necessary/sufficient here since each server capability can be defined underTypeHierarchyOptions
?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.
Yeah, we don't need
boolean
here in this version. We are considering whether to remove the capabilityinheritanceTreeSuppport
to keep the type hierarchy general. I'll update this PR then.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 find it was referring call hierarchy.
which is equivalent to:
specifying whether server has capabitility to support work done progress / unregistration / customized scope of the feature.
@KamasamaK I think
boolean
is insufficient for advanced features mentioned above. For me, it's somehow necessary . Because on one hand, when I implement it in server side I might not care about any advanced feature, then I can simply return atrue
; on the other hand it's consistent with other capabilities like call hierarchy.@CsCherrYY I'm voting +1 for removing
inheritanceTreeSuppport
, otherwise for languages supporting multiple inheritance (e.g. C++), you may also need something likeinheritanceGraphSuppport
?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 you don't care about implementing or can't implement advanced features, you can return
false
for their capabilities. For example,codeLensProvider
only has a type ofCodeLensOptions
with one optional native capability, so that's already established for LSP. That is the reason I say it is probably unnecessary. But if the extra capability is going to be removed, that might change things.Perhaps @dbaeumer can weigh in on the necessity of a simple
boolean
for new operations when *Options exists, and whether them having no or only optional native properties makes a difference in that determination.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.
Thank you for pointing out
CodeLensOptions
. It looks:{ resolveProvider: false }
means server doesn't have a resolve provider.{ resolveProvider: true }
means server also support to resolve code lens.So now I have a question, if server doesn't support code lens at all, what should it return? (if boolean is not allowed)
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.
Excluding
codeLensProvider
would indicate that the server does not support it.