-
Notifications
You must be signed in to change notification settings - Fork 328
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
[WIP] Proposal for type hierarchy #346
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,77 @@ | ||
|
||
#### Type Hierarchy | ||
|
||
Many language support inheritance and LSP provides retrieving the hierarchy information with the following requests. | ||
|
||
|
||
The `textDocument/subTypes` request is sent from the client to the server to resolve resolve the subtypes of a symbol at a given text document position. Returns the direct subtypes in no particular order. | ||
|
||
_Client Capabilities_: | ||
|
||
```ts | ||
TextDocumentClientCapabilities { | ||
/** | ||
* Capabilities specific to the `textDocument/subTypes` | ||
* and `textDocument/superTypes | ||
*/ | ||
typeHierarchy?: { | ||
/** | ||
* Whether implementation supports dynamic registration. | ||
*/ | ||
dynamicRegistration?: boolean; | ||
}; | ||
``` | ||
|
||
_Server Capabilities_: | ||
```ts | ||
ServerCapabilities{ | ||
/** | ||
* The server provides type hierarchy information | ||
*/ | ||
typeHierarchyProvider?: boolean | ||
} | ||
``` | ||
|
||
```ts | ||
/** | ||
* Represents hierarchical information about programming constructs like variables, classes, | ||
* interfaces etc. | ||
*/ | ||
export interface SymbolNode extends SymbolInformation { | ||
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. As of LSP 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. We need one structural change, and we could simplify this PR: The export class DocumentSymbol {
name: string;
detail?: string;
kind: SymbolKind;
deprecated?: boolean;
range: Range | Location;
selectionRange: Range | Location;
children?: DocumentSymbol[];
} instead of the current state: export class DocumentSymbol {
name: string;
detail?: string;
kind: SymbolKind;
deprecated?: boolean;
range: Range;
selectionRange: Range;
children?: DocumentSymbol[];
} When the If we accept this structural change in the We can even stick to the CC: @tsmaeder (since you have taken over the PR) 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. +1 See related #420 (comment).
So with this contract it is enough if 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. Actually, I would vote for adding an optional 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. |
||
/** | ||
* Immediate descendants of this node | ||
*/ | ||
descendants? : SymbolNode[] | ||
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. If it's the immediate descendants, it should be called 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. The doc is incorrect needs to be updated. I am not sure about children either. This can actually hold hierarchies with depth larger than one. Probably needs a better name. 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. Even if it's depth larger than one, wouldn't the elements of this particular array still be the immediate children? 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. We use this same data structure for both 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. Talking about 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 may sometimes be useful to expand multiple levels in one request, This is how ccls and cquery do. |
||
/** | ||
* true if this node has children. hasChildren can be | ||
* true even when the descendants are empty. In that case | ||
* the clients should do a call to retrieve the descendants. | ||
*/ | ||
hasChildren: boolean | ||
} | ||
``` | ||
|
||
_Request_: | ||
|
||
method: ‘textDocument/subTypes’ | ||
params: TextDocumentPositionParams | ||
|
||
_Response_ | ||
|
||
* result: `SymbolNode[] | null` | ||
* error: code and message set in case an exception happens during the 'textDocument/subTypes' request | ||
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. When I open a type hierarchy on a certain position in the text, I would like to see 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. There can be multiple dimensions of sub-super relations: template parameters related to a template, derived types related to a base, specializations. Is there some way to differentiate them in this proposal? In ccls, both derived types and specializations are stored in the |
||
|
||
|
||
|
||
The `textDocument/superTypes` request is sent from the client to the server to resolve resolve the supertypes of a symbol at a given text document position. Returns all the supertypes in bottom-up order. | ||
|
||
_Request_: | ||
|
||
method: ‘textDocument/superTypes’ | ||
params: TextDocumentPositionParams | ||
|
||
_Response_ | ||
|
||
* result: `SymbolNode[] | 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. So a type can have multiple parent types? If that is true, how would it be displayed in a tree UI (vs a graph)? 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. multiple-inheritance has been around and there have been tree based representations of it implemented before. The most generic approach I have seen is to replicate the same Type hierarchy tree under all parent nodes. We can add additional information to this data structure if it is going to help with its representation. 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 you should reclarify the goal of this proposal, I misunderstood it. The linked issue describes a symbol hierarchy (e.g. namespaces -> classes -> members), not inheritance 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. Also take a look at microsoft/language-server-protocol#472 I think that should include the symbol hirearchy |
||
* error: code and message set in case an exception happens during the 'textDocument/superTypes' request | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* -------------------------------------------------------------------------------------------- | ||
* Copyright (c) Red Hat Inc and others . All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
* ------------------------------------------------------------------------------------------ */ | ||
'use strict'; | ||
|
||
import { RequestType } from 'vscode-jsonrpc'; | ||
import {SymbolInformation} from 'vscode-languageserver-types'; | ||
import {TextDocumentPositionParams, TextDocumentRegistrationOptions} from './protocol' | ||
|
||
export interface TypeHierarchyClientCapabilities { | ||
/** | ||
* The text document client capabilities | ||
*/ | ||
textDocument?: { | ||
/** | ||
* Capabilities specific to the `textDocument/subTypes` and `textDocument/superTypes` | ||
*/ | ||
typeHierarchy?: { | ||
/** | ||
* Whether implementation supports dynamic registration. | ||
*/ | ||
dynamicRegistration?: boolean; | ||
}; | ||
} | ||
} | ||
|
||
export interface TypeHierarchyServerCapabilities { | ||
/** | ||
* The server provides type hierarchy information | ||
*/ | ||
typeHierarchyProvider?: boolean | ||
} | ||
|
||
/** | ||
* | ||
*/ | ||
export namespace SubTypesRequest { | ||
export const type = new RequestType<TextDocumentPositionParams, SymbolNode[], void, TextDocumentRegistrationOptions>('textDocument/subTypes'); | ||
} | ||
|
||
/** | ||
* | ||
*/ | ||
export namespace SuperTypesRequest { | ||
export const type = new RequestType<TextDocumentPositionParams, SymbolNode[], void, TextDocumentRegistrationOptions>('textDocument/superTypes'); | ||
} | ||
|
||
|
||
/** | ||
* Represents hierarchical information about programming constructs like variables, classes, | ||
* interfaces etc. | ||
*/ | ||
export interface SymbolNode extends SymbolInformation { | ||
/** | ||
* Immediate descendants of this node | ||
*/ | ||
descendants? : SymbolNode[] | ||
/** | ||
* true if this node has children. hasChildren can be | ||
* true even when the descendants are empty. In that case | ||
* the clients should do a call to retrieve the descendants. | ||
*/ | ||
hasChildren: boolean | ||
} |
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 there a reason this proposal is using the term "type" everywhere and not "symbol"? For example in TypeScript, those are two very different concepts
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 think those are very different concepts on most languages.