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
582 stars 141 forks source link

Invalid generation of @JsonRpcData when using lsp4j 0.21 and xtend 2.31.0 #742

Closed dvojtise closed 5 months ago

dvojtise commented 1 year ago

When installing lsp4j and xtend from the current Eclipse release update site (2023-06) It installs lspj4 0.21.0 and xtend 2.31.0

(I started from Eclipse Modeling 2023-06 and then installed lsp4j using the updatesite https://download.eclipse.org/releases/2023-06/ )

A simple project using @JsonRpcData from lsp4j.generator will now fail. The generated java class is incorrect. (the toString

package bug_lsp4j_xtend

import org.eclipse.lsp4j.generator.JsonRpcData

@JsonRpcData
class MyAnnotatedClass {

}

will generate a code that doesn't compile image

An equivalent code was working in the previous release I tried (xtend 2.25.0 + lsp4j 0.14.0) (I haven't tried yet to find the exact last working version ...)

project showing the issue joined bug_lsp4j_xtend_2023-06.zip

cdietrich commented 1 year ago

how does your manifest look like? package import (org.eclipse.lsp4j.util)? require bundle?

dvojtise commented 1 year ago

(The example project is joined if you want to try)

this is using require bundle

cdietrich commented 1 year ago

you dont have a dependency to lsp4j itself? is this intentional? if yes you have to provide your own ToStringBuilder in bug_lsp4j_xtend.util package

@jonahgraham maybe we should move the ToStringBuilder class to jsonrpc module? the problem is this is not a runtime dependency

dvojtise commented 1 year ago

I mainly need to use the same approach as lsp4j in order to generate similar protocols. JSONRPC based but neither DAP nor LSP. (your jsonrpc lib is cool ;-) )

I added several of the other bundles in the dependencies, without better result on this example.

cdietrich commented 1 year ago

simply copy the ToStringBuilder class to xxx.util package

dvojtise commented 1 year ago

Thanks, it works

I'll update my projects with that

dvojtise commented 1 year ago

BTW, I second your proposal about moving the ToStringBuilder class since I haven't seen any differences between https://github.com/eclipse-lsp4j/lsp4j/blob/main/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/util/ToStringBuilder.java and https://github.com/eclipse-lsp4j/lsp4j/blob/main/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/util/ToStringBuilder.java

cdietrich commented 1 year ago

yes it is the same, but we dont have a util place that is available from both

jonahgraham commented 1 year ago

@jonahgraham maybe we should move the ToStringBuilder class to jsonrpc module?

Yes that makes sense. The generator makes code that works with jsonrpc, so having the ToStringBuilder in (e.g.) new package org.eclipse.lsp4j.jsonrpc.util makes sense to me.

the problem is this is not a runtime dependency

Sorry, I don't know what you mean here? Does this mean that org.eclipse.lsp4j.jsonrpc is not a runtime dep of the generator? Is it a problem adding it?

cdietrich commented 1 year ago

@jonahgraham no its not a (runtime) dependency of org.eclipse.lsp4j itself. so it wont work at runtime or pde is fooling me. in the manifest i can see a package import

Screenshot 2023-06-21 070742

jonahgraham commented 5 months ago

The same problem occurs with the less often used reference to Preconditions.

pisv commented 5 months ago

The same problem occurs with the less often used reference to Preconditions.

Looks like it would make sense to handle it in the same way, after #798 is merged.