eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
613 stars 145 forks source link

How to use extended org.eclipse.lsp4j.services.TextDocumentService ? #577

Closed 823984418 closed 2 years ago

823984418 commented 2 years ago

Clangd expand some things in textDocument, so I have ClangdTextDocumentService extends TextDocumentService But when I tried to create it, I had trouble. When I try to

public interface ClangdLanguageServer extends LanguageServer {
    @Override
    @JsonDelegate
    ClangdTextDocumentService getTextDocumentService();
}

I got an error

Exception in thread "main" java.lang.IllegalStateException: Duplicate RPC method public abstract net.dxzc.demo.ClangdTextDocumentService net.dxzc.demo.ClangdLanguageServer.getTextDocumentService()
    at org.eclipse.lsp4j.jsonrpc.services.EndpointProxy.lambda$new$1(EndpointProxy.java:75)
    at org.eclipse.lsp4j.jsonrpc.services.AnnotationUtil.findDelegateSegments(AnnotationUtil.java:32)
    at org.eclipse.lsp4j.jsonrpc.services.EndpointProxy.<init>(EndpointProxy.java:69)
    at org.eclipse.lsp4j.jsonrpc.services.EndpointProxy.<init>(EndpointProxy.java:42)
    at org.eclipse.lsp4j.jsonrpc.services.ServiceEndpoints.toServiceObject(ServiceEndpoints.java:40)
    at org.eclipse.lsp4j.jsonrpc.Launcher$Builder.createProxy(Launcher.java:364)
    at org.eclipse.lsp4j.jsonrpc.Launcher$Builder.create(Launcher.java:321)
    at net.dxzc.demo.ClangdDemo.main(ClangdDemo.java:108)

I have viewed org.eclipse.lsp4j.jsonrpc.services.AnnotationUtil.findDelegateSegments,

public static void findDelegateSegments(Class<?> clazz, Set<Class<?>> visited, Consumer<Method> acceptor) {
    if (clazz == null || !visited.add(clazz))
        return;
    findDelegateSegments(clazz.getSuperclass(), visited, acceptor);
    for (Class<?> interf : clazz.getInterfaces()) {
        findDelegateSegments(interf, visited, acceptor);
    }
    for (Method method : clazz.getDeclaredMethods()) {
        if (isDelegateMethod(method)) {
            acceptor.accept(method);
        }
    }
}

Obviously the overwrite method does not remove the original, so I got duplicate RPC method...

So how does TextDocumentService extend in design ?

szarnekow commented 2 years ago

Does it work if you don't repeat the@JsonDelegate annotation on your ClangdLanguageServer.getTextDocumentService() ?

823984418 commented 2 years ago

Does it work if you don't repeat the@JsonDelegate annotation on your ClangdLanguageServer.getTextDocumentService() ?

If I comment out this line:

public interface ClangdLanguageServer extends LanguageServer {
    @Override
//    @JsonDelegate
    ClangdTextDocumentService getTextDocumentService();
}

Obviously the LSP4J's jsonrpc will not resolve it to ClangdTextDocumentService, I got a cast exception.

Exception in thread "main" java.lang.ClassCastException: com.sun.proxy.$Proxy7 cannot be cast to net.dxzc.demo.ClangdTextDocumentService
    at com.sun.proxy.$Proxy9.getTextDocumentService(Unknown Source)
    at net.dxzc.demo.ClangdDemo.main(ClangdDemo.java:152)

This is because:

public class EndpointProxy implements InvocationHandler {
    // ...
    @Override
    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
        args = args == null ? new Object[0] : args;
        MethodInfo methodInfo = this.methodInfos.get(method.getName());
        if (methodInfo != null) {
            Object params = getParams(args, methodInfo);
            if (methodInfo.isNotification) {
                delegate.notify(methodInfo.name, params);
                return null;
            }
            return delegate.request(methodInfo.name, params);
        }
        DelegateInfo delegateInfo = this.delegatedSegments.get(method.getName());
        if (delegateInfo != null) {
// ----------------
            return delegateInfo.delegate;
// ---------------- This is not ClangdTextDocumentService 
        }
        if (object_equals.equals(method) && args.length == 1) {
            if(args[0] != null ) {
                try {
                    return this.equals(Proxy.getInvocationHandler(args[0]));
                } catch (IllegalArgumentException exception) {
                }
            }
            return this.equals(args[0]);
        }
        if (object_hashCode.equals(method)) {
            return this.hashCode();
        }
        if (object_toString.equals(method)) {
            return this.toString();
        }
        return method.invoke(delegate, args);
    }
    // ...
}

Does anyone try to expand TextDocumentService before?

pisv commented 2 years ago

Please note that by trying to extend the standard textDocument namespace, you run the risk of name clashing as the LSP evolves.

Anyway, a solution could be to not extend ClangdTextDocumentService from TextDocumentService.

@JsonSegment("textDocument") // or "clangdTextDocument" to avoid clashing
public interface ClangdTextDocumentService {
    ...
}

public interface ClangdLanguageServer extends LanguageServer {
    @JsonDelegate
    ClangdTextDocumentService getClangdTextDocumentService();
}

Just my 2c.

823984418 commented 2 years ago

Please note that by trying to extend the standard textDocument namespace, you run the risk of name clashing as the LSP evolves.

Anyway, a solution could be to not extend ClangdTextDocumentService from TextDocumentService.

@JsonSegment("textDocument") // or "clangdTextDocument" to avoid clashing
public interface ClangdTextDocumentService {
    ...
}

public interface ClangdLanguageServer extends LanguageServer {
    @JsonDelegate
    ClangdTextDocumentService getClangdTextDocumentService();
}

Just my 2c.

That solved my problem perfectly, Although I still think extending TextDocumentService should be supported. Clangd was designed that way, This textDocument's name makes I didn't think about writing a new method. Thank you very much for your help!

pisv commented 2 years ago

That solved my problem perfectly

Great!

Although I still think extending TextDocumentService should be supported. Clangd was designed that way

I see. BTW, if you must extend textDocument, you could probably also make ClangdTextDocumentService extend TextDocumentService. Just introduce a new delegate method (ClangdLanguageServer.getClangdTextDocumentService() as shown above) instead of overriding the existing one (LanguageServer.getTextDocumentService()). I didn't try it myself, but looking at the code, it should probably work.