Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaFileTextDocumentService doesn't gracefully handle lack of rename support #700

Closed
rgrunber opened this issue Aug 5, 2022 · 5 comments · Fixed by #701
Closed

JavaFileTextDocumentService doesn't gracefully handle lack of rename support #700

rgrunber opened this issue Aug 5, 2022 · 5 comments · Fixed by #701
Labels
bug Something isn't working
Milestone

Comments

@rgrunber
Copy link
Member

rgrunber commented Aug 5, 2022

Try to rename some element in a Java file when the Quarkus extension is started.

We retrieve the TextDocumentService and call rename on it. However JavaFileTextDocumentService doesn't support rename, and will throw an UnsupportedOperationException. The rename will still succeed.

TextDocumentService service = getTextDocumentService(params.getTextDocument());
if (service != null) {
return service.rename(params);

[Error - 12:56:35 p.m.] Request textDocument/rename failed.
  Message: Internal error.
  Code: -32603 
java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:67)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.request(GenericEndpoint.java:120)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleRequest(RemoteEndpoint.java:261)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:190)
	at com.redhat.qute.ls.commons.ParentProcessWatcher.lambda$apply$0(ParentProcessWatcher.java:148)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at java.base/java.lang.Thread.run(Unknown Source)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
	... 12 more
Caused by: java.lang.UnsupportedOperationException
	at org.eclipse.lsp4j.services.TextDocumentService.rename(TextDocumentService.java:337)
	at com.redhat.qute.ls.QuteTextDocumentService.rename(QuteTextDocumentService.java:235)
	... 17 more
@rgrunber rgrunber added the bug Something isn't working label Aug 5, 2022
@rgrunber
Copy link
Member Author

rgrunber commented Aug 5, 2022

I think if AbstractTextDocumentService just implements a return CompletableFuture.completedFuture(null); for rename() then it should fall back to that instead of throwing the UnsupportedOperationException

@rgrunber rgrunber changed the title JavaFileTextDocumentService doesn't support rename but will still get called JavaFileTextDocumentService doesn't gracefully handle lack of rename support Aug 5, 2022
@datho7561
Copy link
Contributor

I think if AbstractTextDocumentService just implements a return CompletableFuture.completedFuture(null); for rename() then it should fall back to that instead of throwing the UnsupportedOperationException

tried this, don't think it works. I'll take a look into this issue though

@angelozerr
Copy link
Contributor

Indeed it is a bug, JavaFileTextDocumentService should implement this rename method which should do nothing.

@rgrunber
Copy link
Member Author

rgrunber commented Aug 5, 2022

I just added the rename to both instances of AbstractTextDocumentService (there's 2, in qute & in lsp4mp), and that fixed it for me, without having to implement anything in JavaFileTextDocumentService . The rename still succeeds, just without any errors.

datho7561 added a commit to datho7561/quarkus-ls that referenced this issue Aug 5, 2022
Closes redhat-developer#700

Signed-off-by: David Thompson <davthomp@redhat.com>
@angelozerr
Copy link
Contributor

Indeed you can do that but it is not consistent with other methods. If you prefer doeing that, please update other methods.

angelozerr pushed a commit that referenced this issue Aug 5, 2022
Closes #700

Signed-off-by: David Thompson <davthomp@redhat.com>
@rgrunber rgrunber added this to the 0.13.0 milestone Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants