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

Handle methods with Void return types properly #675

Closed jonahgraham closed 2 years ago

jonahgraham commented 2 years ago

The DAP part of LSP4J was being too strict and failing to parse response objects for methods with Void return type. With Java 17 (or some version since Java 11) turning on more strict reflection rules, the "An illegal reflective access operation has occurred" that used to be a warning is now an exception like "Failed making constructor 'java.lang.Void#Void()' accessible"

This change makes the code work and compatible with how LSP4J handles LSP Void return types.

Fixes #674

jonahgraham commented 2 years ago

@sebthom it would be very helpful if you can test your failing case in #674 with this fix. You can pick up a p2 repo here: https://download.eclipse.org/lsp4j/builds/jonah/dap_return_void/

sebthom commented 2 years ago

I somehow cannot use the update site, I am getting: image

I get the same error if I try to use https://download.eclipse.org/lsp4j/updates/releases/0.17.0/ instead of https://download.eclipse.org/lsp4j/updates/releases/0.16.0/

Apparently as of lsp4j 0.17.0 the org.eclipse.xtext.xbase.lib plugin is not part of the update site anymore. Is that on purpose?

cdietrich commented 2 years ago

We do not package it https://github.com/eclipse/lsp4j/blob/main/releng/p2/category.xml you can consume it from the 2022-09 Or the Xtext 2.28 update site

https://download.eclipse.org/modeling/tmf/xtext/updates/releases/2.28.0/

sebthom commented 2 years ago

That's odd, I am seeing this: image

Maybe it's a UI bug in the Target Editor.

cdietrich commented 2 years ago

https://github.com/eclipse/lsp4j/blob/2260c479d5e02a37db5d65af51291c060d183009/releng/lsp4j-feature/feature.xml#L35

cdietrich commented 2 years ago

There was some back and forth in tycho how to deal with imports , maybe thus it got packaged in 15

sebthom commented 2 years ago

@cdietrich I added the xtext update site and now it works. thanks!

@jonahgraham I can confirm, that the change of this PR works as expected. Thanks for the quick resolution!