LSP: textDocument/documentSymbol lacks containerName

[chergert]

When providing the symbol results for textDocument/documentSymbol, there is an optional field named containerName which provides the name of the parent symbol. It would be nice if rls supported this so that we can build a proper symbol tree in GNOME Builder.

[nrc]

The relevant part of the spec: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#document-symbols-request

AIUI, we have this info coming out of the compiler, but we don't record it in rls-analysis and so it is non-trivial (but not exactly difficult) to surface from the RLS.

Historicaly, this has also been a bit of a tricky thing to get right - the compiler has multiple different ideas of what the container symbol is (e.g., whether anonymous blocks get counted, whether an impl counts, or should instead use the concrete type or trait, etc.), so I'm not 100% convinced a single one-size-fits-all container symbol will work (especially as we can only provide a name, not an id, so we can't use impls here).

[bruno-medeiros]

Why is the compiler and rls-analysis info necessary for this? The https://github.com/RustDT/Rainicorn tool provides this functionality using only the Rust parser. The protocol format mapping is easier here, because it provides the symbols (aka structure elements) in a hierarchical model already, thus the container info is already included implicitly. However, the problem of mapping hierarchical model to flat one is a different problem. In any case, it seems only the rust parser would be needed.

[bruno-medeiros]

BTW, I'm not sure we can create a tree model reliably in a client using containerName. The thing is, from what I understand from VS Code, the containerName is used by the "Go to Symbol in File" functionality, which uses containerName just for display purposes (https://code.visualstudio.com/images/cpp_workspacesearch.png). This means it doesn't have to worry about duplicates, etc.

But to use containerName to build a tree model, we would have to ensure those names are unique, otherwise you run into problems with code like:

struct Foo {
}
impl Foo {
  fn func() { }
}

if the func symbol is sent with containerName=Foo, which container is that?

I guess we could specify containerName in a modified way that actually is unique (for example assigning id's like 1:Foo for struct Foo , 2:Foo for impl Foo, etc, but this would be an LSP extension that clients would have to know how to handle. Point being, to implement this reliably, it seems we need need an LSP extension.

[nrc]

Why is the compiler and rls-analysis info necessary for this?

Depends how you define it, if you only ever want the syntactic container, then you don't, if you want the semantic container, e.g., Foo containing func in your example above, then you do.

[chergert]

It would be nice to have a cross-unit id extension to the symbol JSON node for more complex use cases. Then allow the tooling to choose how precise they want to be with the tree.

[bruno-medeiros]

Depends how you define it, if you only ever want the syntactic container, then you don't, if you want the semantic container, e.g., Foo containing func in your example above, then you do.

I think the way this is typically used, is with syntactic container only.

[bruno-medeiros]

Actually, (as was pointed out here: microsoft/language-server-protocol#112 (comment) ), the source range information (if correct) should be enough to build a tree of the symbols, so container name is not actually needed.

[chergert]

I must be missing something, because doesn't that still require a language parser client side to know the boundary of scopes (which would defeat the purpose of IDE side langserv provider being language agnostic). For languages where classes can be embedded inside of classes, or multiple classes per file, how do you know if the class was nested or simply after the previous?

Edit: (I guess that is what you meant by reliable range information, so you'd have an end position). Nevermind, feel free to disregard this :)

[chergert]

Okay, so I implemented support for building the tree using the provided range in GNOME Builder https://github.com/chergert/gnome-builder/commit/7d6ae63bd31f5aa3e5fe7d7b59c0b100d7099357 but it looks like that rustls does not provide source ranges that map to the entire semantic range of the AST node but rather the keyword.

So once rustls adjusts the range to be accurate, symbol trees should just start working for us.

[bruno-medeiros]

So once rustls adjusts the range to be accurate, symbol trees should just start working for us.

That's a good point, I hadn't checked what kind of range rls returned. However, it might might that the intention of LSP is that the range of the symbol is meant to be only the range of the identifier. I've asked for a clarification: microsoft/language-server-protocol#132

[alexheretic]

Atom users would appreciate this too, see rust-lang/atom-ide-rust#74.

Rls providing a containerName for symbols is perfectly valid in the spec, so I can't see anything blocking it's implementation (other than someone to do it). The client's use/misuse for creating a tree is up to the client, but in any case that ship has sailed and no alternative for clients seems to exist yet.