Properly implement document/documentSymbol #206
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
milesziemer
force-pushed
the
improve-document-symbols
branch
3 times, most recently
from
4701a1b to
0abfdd1
Compare
milesziemer
changed the title
When I re-wrote the language feature implementations in smithy-lang#166, I refactored documentSymbol. But it mostly just copied the previous implementation, which produced a flat list of symbols. This meant that member symbols would appear at the top-level, but documentSymbol is meant to provide a tree-like view, where symbols can have children. So this commit re-writes documentSymbol to produce a list of only the top-level shapes' symbols (and the namespace statement's symbol), with members' symbols being added as children to the top-level symbols, and members of inline i/o are children of the inline i/o symbol. I also changed up the Symbol kind a bit, so root shapes are of type `Class` (except enum/intEnum, which are of type `Enum`), members are of type `Field` (except enum/intEnum members, which are of type `EnumMember`), and service/resource/operation members are of type `Property`. I explored having different symbol kinds for different types of shapes, like making service-type shapes be of kind `Interface`, and primitive types being of their corresponding kind (for example, an integer shape would have kind `Number`), but I wasn't sure what some shape types should be. For example, what should a `blob` shape be? So I decided to just make all top-level shapes just be of kind `Class`. I also fixed the range and selection range of symbols. Range now covers everything from shape type -> end of block (if applicable), and selection range is just the shape/member name. I considered adding more children for service-type shape members, like making the `operations` child symbol of a service shape have a child for each of the operations in the list, but I think it would make the tree view more noisy, plus I think the intent is to show symbol definitions. I also made a minor adjustment to the parsing of operations members, making them always be inline or node member defs, instead of possibly a regular member def for non-inline i/o. This is consistent with resource/service members, which are always node members.
Uses new Document methods for computing range and selectionRange.
milesziemer
force-pushed
the
improve-document-symbols
branch
from
0abfdd1 to
538606f
Compare
milesziemer
requested review from
haydenbaker and
yefrig
and removed request for
haydenbaker
yefrig
approved these changes
Mar 27, 2025
| List<Either<SymbolInformation, DocumentSymbol>> result = new ArrayList<>(); | ||
| // Passing around the list would make the code super noisy, and we'd have | ||
| // to do Either.forRight everywhere, so use a consumer. | ||
| addSymbols((symbol) -> result.add(Either.forRight(symbol))); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When I re-wrote the language feature implementations in #166, I
refactored documentSymbol. But it mostly just copied the previous
implementation, which produced a flat list of symbols. This meant that
member symbols would appear at the top-level, but documentSymbol is
meant to provide a tree-like view, where symbols can have children. So
this commit re-writes documentSymbol to produce a list of only the
top-level shapes' symbols (and the namespace statement's symbol), with
members' symbols being added as children to the top-level symbols, and
members of inline i/o are children of the inline i/o symbol.
I also changed up the Symbol kind a bit, so root shapes are of type
Class(except enum/intEnum, which are of typeEnum), members areof type
Field(except enum/intEnum members, which are of typeEnumMember), and service/resource/operation members are of typeProperty.I explored having different symbol kinds for different types of shapes,
like making service-type shapes be of kind
Interface, and primitivetypes being of their corresponding kind (for example, an integer shape
would have kind
Number), but I wasn't sure what some shape typesshould be. For example, what should a
blobshape be? So I decided tojust make all top-level shapes just be of kind
Class.I also fixed the range and selection range of symbols. Range now covers
everything from shape type -> end of block (if applicable), and
selection range is just the shape/member name.
I considered adding more children for service-type shape members,
like making the
operationschild symbol of a service shape have achild for each of the operations in the list, but I think it would make
the tree view more noisy, plus I think the intent is to show symbol
definitions.
I also made a minor adjustment to the parsing of operations members,
making them always be inline or node member defs, instead of possibly a
regular member def for non-inline i/o. This is consistent with
resource/service members, which are always node members.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.