Support inlay hints for parameter names

[jdneo]

fix redhat-developer/vscode-java#2099

requires redhat-developer/vscode-java#2354

Signed-off-by: sheche sheche@microsoft.com

[rgrunber]

Just wondering. Is there a reason to go through the delegate API (java.execute.workspaceCommand) as opposed to https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java ? I think the only difference on the client-side would be calling sendRequest directly instead of executeCommand.

[jdneo]

I followed the way we did before to support the semantic highlighting. It was firstly leveraging the delegate command then migrated to LSP when it's available in LSP4J:

[1]. 9a6d61b
[2]. 00b8336

[angelozerr]

I implemented inlayhint too for Qute, see my PR https://github.com/redhat-developer/quarkus-ls/pull/593/files

The main idea is that I define an interface which follow the LSP spec with InlayHint, InlayHintParams LSP structures.

package org.eclipse.lsp4j.proposed;

import java.util.List;
import java.util.concurrent.CompletableFuture;

import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;

public interface QuteInlayHintProvider {

	@JsonRequest("qute/textDocument/inlayHint")
	default CompletableFuture<List<InlayHint>> inlayHint(InlayHintParams params) {
		throw new UnsupportedOperationException();
	}
}

Please note that I defined "qute/textDocument/inlayHint" but when LSP4J will be released, we don't need this interface.

The QuteInlayHintProvider implements my LanguageServer implementation.

On client side,I use sendRequest to consume inlay hint https://github.com/redhat-developer/vscode-quarkus/pull/474/files#diff-aeb59384a9e0b18adc89b06d548c119ed02ead2a48e1c11a26eb4972409dfe27R57

[jdneo]

Thank you for sharing the information and the proposed implementation. My personal opinion is that these are just two different ways of the implementation but with the same purpose. Finally when the structures of Inlay hints are added in LSP4J, those codes will be removed. So I prefer chose the easy way to do it as for now.

[angelozerr]

@jdneo I changed my strategy, and now I'musing the real LSP request type textDocument/inlayHint described in the LSP specification. My interface becomes:

package org.eclipse.lsp4j.proposed;

import java.util.List;
import java.util.concurrent.CompletableFuture;

import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;

public interface QuteInlayHintProvider {

	@JsonRequest("textDocument/inlayHint")
	default CompletableFuture<List<InlayHint>> inlayHint(InlayHintParams params) {
		throw new UnsupportedOperationException();
	}
}

It means that when vscode-language-client will be released (the master ofvscode-language-client already implement the request type textDocument/inlayHint), I will just remove my typescript InlayHintProvider and and use last version of vscode-language-client and that's it.

On server side, when LSP4J will be removed, I will remove all my code from my org.eclipse.lsp4j.proposed package and replace org.eclipse.lsp4j.proposed with org.eclipse.lsp4j and that's it.

In otherwise for client and server side, it will be just a matter of remove code and adapt some package without developping something.

[jdneo]

Sounds cool! @testforstephen @rgrunber Any input about this?

[rgrunber]

This checks EnumConstantDeclaration, ClassInstanceCreation & MethodInvocation. https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/tree/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/CalleeJavaMethodParameterVisitor.java also does ConstructorInvocation, SuperConstructorInvocation. I think these 2 cover this(...) and super(...). Are they worth handling ?

[jdneo]

This checks EnumConstantDeclaration, ClassInstanceCreation & MethodInvocation. https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/tree/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/CalleeJavaMethodParameterVisitor.java also does ConstructorInvocation, SuperConstructorInvocation. I think these 2 cover this(...) and super(...). Are they worth handling ?

Thanks for reminding. Sure and besides ConstructorInvocation and SuperConstructorInvocation, SuperMethodInvocation also added in the latest change.

[rgrunber]

Overall, change works well for me. Once addressed, I think this is ready to be merged.