How to extend the ClangdLanguageServer with the textDocument/ast extension from clangd.llvm.org

[travkin79]

Hello @jonahgraham and @ghentschke,
I came across the clangd LSP extensions, especially the client-server request for getting the abstract syntax tree (AST) for a given document and range. I'm thinking about preparing a PR for adding that extension to CDT LSP, but I'm stuck finding a way to implement new interface methods added to org.eclipse.cdt.lsp.services.ClangdLanguageServer. Could you please give me some hints?

I saw that interface is declared in the org.eclipse.lsp4e.languageServer extension point, but I don't find an implementing class. The class org.eclipse.cdt.lsp.internal.server.CLanguageServerStreamConnectionProvider from that extension point seems to be quite unrelated to the ClangdLanguageServer interface, it doesn't provide any factory method or something similar. What do I miss? Do I have to configure clangd somehow and implement the new interface method(s) somewhere else?

I saw, you're already using one extension from clangd.llvm.org in the org.eclipse.cdt.lsp.internal.clangd.editor.handler.ToggleSourceAndHeaderCommandHandler. That class seems to be the only one that references ClangdLanguageServer.

This topic is related to eclipse-lsp4e/lsp4e#911 and might be related to #157.

[ghentschke]

Please take a look at ToggleSourceAndheaderCommandHandler
It shows how switchSourceHeader works, which is defined in the org.eclipse.cdt.lsp.services.ClangdLanguageServer interface

[travkin79]

I did, but I only see the call of the switchSourceHeader method, not the method's implementation. Or do we only need the ClangdLanguageServer interface, not its implementation in CDT LSP, since the LSP calls a method named e.g. textDocument/switchSourceHeader which is already implemented in clangd?

[ghentschke]

I think the magic happens here in ClangdLanguageServer:

	@JsonRequest(value = "textDocument/switchSourceHeader")
	CompletableFuture<String> switchSourceHeader(TextDocumentIdentifier textDocument);

and here in ToggleSourceAndHeaderCommandHandler:

	org.eclipse.lsp4e.LanguageServers.forDocument(document)
			.computeFirst(
					server -> server instanceof ClangdLanguageServer
							? ((ClangdLanguageServer) server)
									.switchSourceHeader(new TextDocumentIdentifier(fileUri.toString()))
							: null)
			.thenAccept(otherFileUri -> otherFileUri
					.ifPresent(uri -> openEditor(innerEditor.getEditorSite().getPage(), URI.create(uri))));

I think the implementation will be created by the json request annotation. But I am not sure.

[travkin79]

Yes, that were exactly the places I was looking at. I'm not sure how it exactly works, but I'll try to add an additional method to the ClangdLanguageServer interface, a call of it, and will see if that works already.

[ghentschke]

I would guess you should take a closer look at org.eclipse.lsp4j. There is a data model for the Range in org.eclipse.lsp4j.Range but I haven't found a lsp4j class for the ASTNode which is the response object from clangd on the textDocument/ast call.

Maybe lsp4j has to be extended here to support the ASTNode structure since it seems not to be a part of the LSP specification

[travkin79]

Thank you @ghentschke.
Indeed, the annotation with the message / request type, e.g. @JsonRequest(value = "textDocument/ast") seems to be enough. I think, the implementation is in clangd in the LS.

I added a class AstParams for the request arguments and was able to send a request to the language server. But the response could not be parsed, since classes like ASTNode are missing in org.eclipse.lsp4j. I'll see what else is needed for parsing the response.

[jonahgraham]

The org.eclipse.lsp4j project only implements the official LSP Specification. You need to add the new data types in the same bundle place you added the annotation.

[travkin79]

Thanks @jonahgraham,
That's exactly what I expected and what I am doing now. :-)
Getting the AST in this case is clangd-specific. The implementation should not go to the common bundle org.eclipse.lsp4j.

[travkin79]

I managed to send a textDocument/ast request and receiving an AstNode. It seems, I have a first walking skeleton.

I'm far from finishing the new extension, but in case you'd like to take a look at what I have done so far, you could look at my WIP fork.

[jonahgraham]

That is great! I look forward to a future PR :-)

[travkin79]

I tried implementing a prototype that uses some of the clangd LSP extensions, but there seem to be quite a few limitations. I cannot add any feature to CDT LSP using these extensions. (Instead, I plan to use the extension in another plug-in outside CDT LSP, since it is very specific to our use case.)

Though, I think, the extended ClangdLanguageServer API might be useful for others (other features to come), too. Even if no classes in CDT LSP calls them, yet. I suggest to add these to the next official CDT LSP. Do you agree, @jonahgraham and @ghentschke?

I've prepared the PR #256 with these LS extensions in the bundle org.eclipse.cdt.lsp, but have removed my experimental uses of that API.

Is there any chance that there will be a new CDT LSP release before 23rd February 2024? (My team has an internal deadline for some features that would be based on some of the changes already merged to master and on the LS extensions I'm discussing here.)

[ghentschke]

I cannot add any feature to CDT LSP using these extensions.

Can you tell us what prevents you from adding a new feature to cdt-lsp? Maybe I can assist?

Even if no classes in CDT LSP calls them, yet. I suggest to add these to the next official CDT LSP.

I am fine with that.

Is there any chance that there will be a new CDT LSP release before 23rd February 2024?

IMO we can do a new minor release in short term. I can prepare it. I think you want the PR #256 to be included in V1.1.0 Release? Then we should wait on a feedback by @ruspl-afed
@jonahgraham: Do you have any concerns about a cdt-lsp 1.1.0 release?

[ruspl-afed]

Added +1 for PR #256

[travkin79]

The most relevant changes for us are the following. We hope, they will be released very soon, i.e. before 23rd February. It would be great, if you could prepare a minor release in short term, @ghentschke.

• PR [#227] Refactor ClangdConfigurationManager #229
• PR Add some clangd LSP extensions to the ClangdLanguageServer API #256
• PR Add spelling support #244

Can you tell us what prevents you from adding a new feature to cdt-lsp? Maybe I can assist?

In our team, we're implementing some company-specific Eclipse plug-ins on top of LSP4E and CDT LSP and we want to use the new LSP extensions for clangd in order to request certain details about the C++ element under cursor and for looking up and showing additional documentation in a custom view for that element under cursor. But I don't see any use case for that new LSP extensions in CDT LSP, yet.

Besides, it seems that we get the AST only for .hxx files, but not for .cxx files.

Concerning the PR #256: @ruspl-afed asked if I planned to add tests in order to show how the new extensions could be used, but I'm not sure what kind of tests would make sense. My first idea was to implement integration tests that really send requests to the LS and check the returned results, but it seems, clangd cannot be found in my test environment. Besides, I'm not sure if such integration tests are desirable, since they might be platform-specific.

[ruspl-afed]

Added +1 for PR #256

[ghentschke]

Thank you @ruspl-afed

[travkin79]

Thank you very much, @ruspl-afed, @ghentschke, and @jonahgraham.