Quick Fixes are missing JsonbTransientDiagnostic test class.

[anusreelakshmi934]

When testing using the latest LSP4IJ version ( 0.0.2 ) , we are not able to get the quickfixes in the class JsonbTransientDiagnostic. Only the diagnostics are present

Also Investigate on Associated Tests becoming success eventhough we are not getting the QuickFIxes

[aparnamichael]

Getting class cast exception for this quickfix in lsp4ij-market-0.0.2-integration branch. In the LTI 24.0.6 code, we use lsp4ij from Microshed and there is no class cast exceptions in it.

2024-07-05 12:15:50,317 [ 79166] WARN - io.openliberty.tools.intellij.lsp4mp4ij.psi.internal.core.java.codeaction.JavaCodeActionDefinition - Error while calling getCodeActions java.lang.ClassCastException: class com.google.gson.JsonObject cannot be cast to class com.google.gson.JsonArray (com.google.gson.JsonObject is in unnamed module of loader com.intellij.ide.plugins.cl.PluginClassLoader @109d7631; com.google.gson.JsonArray is in unnamed module of loader com.intellij.ide.plugins.cl.PluginClassLoader @80c09f7) at io.openliberty.tools.intellij.lsp4jakarta.lsp4ij.codeAction.proposal.quickfix.RemoveMultipleAnnotations.getCodeActions(RemoveMultipleAnnotations.java:48) at io.openliberty.tools.intellij.lsp4mp4ij.psi.internal.core.java.codeaction.JavaCodeActionDefinition.getCodeActions(JavaCodeActionDefinition.java:87) at io.openliberty.tools.intellij.lsp4mp4ij.psi.internal.core.java.codeaction.CodeActionHandler.lambda$codeAction$3(CodeActionHandler.java:142) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at io.openliberty.tools.intellij.lsp4mp4ij.psi.internal.core.java.codeaction.CodeActionHandler.codeAction(CodeActionHandler.java:129) at io.openliberty.tools.intellij.lsp4jakarta.lsp4ij.PropertiesManagerForJakarta.lambda$getCodeAction$0(PropertiesManagerForJakarta.java:100) at com.intellij.openapi.application.impl.RwLockHolder.runReadAction(RwLockHolder.kt:271) at com.intellij.openapi.application.impl.ApplicationImpl.runReadAction(ApplicationImpl.java:845) at io.openliberty.tools.intellij.lsp4jakarta.lsp4ij.PropertiesManagerForJakarta.getCodeAction(PropertiesManagerForJakarta.java:99) at io.openliberty.tools.intellij.lsp4jakarta.lsp.JakartaLanguageClient.lambda$getJavaCodeAction$1(JakartaLanguageClient.java:68) at com.redhat.devtools.lsp4ij.internal.PromiseToCompletableFuture.lambda$nonBlockingReadActionPromise$2(PromiseToCompletableFuture.java:144) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$OTelMonitor.callWrapped(NonBlockingReadActionImpl.java:851) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$OTelMonitor$MonitoredComputation.call(NonBlockingReadActionImpl.java:883) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$Submission.insideReadAction(NonBlockingReadActionImpl.java:604) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$Submission.lambda$attemptComputation$4(NonBlockingReadActionImpl.java:567) at com.intellij.openapi.application.impl.RwLockHolder.tryRunReadAction(RwLockHolder.kt:310) at com.intellij.openapi.application.impl.ApplicationImpl.tryRunReadAction(ApplicationImpl.java:958) at com.intellij.openapi.progress.util.ProgressIndicatorUtils.lambda$runInReadActionWithWriteActionPriority$0(ProgressIndicatorUtils.java:93) at com.intellij.openapi.progress.util.ProgressIndicatorUtilService.runActionAndCancelBeforeWrite(ProgressIndicatorUtilService.java:66) at com.intellij.openapi.progress.util.ProgressIndicatorUtils.runActionAndCancelBeforeWrite(ProgressIndicatorUtils.java:155) at com.intellij.openapi.progress.util.ProgressIndicatorUtils.lambda$runWithWriteActionPriority$1(ProgressIndicatorUtils.java:138) at com.intellij.openapi.progress.ProgressManager.lambda$runProcess$0(ProgressManager.java:100) at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$2(CoreProgressManager.java:217) at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$executeProcessUnderProgress$13(CoreProgressManager.java:660) at com.intellij.openapi.progress.impl.CoreProgressManager.registerIndicatorAndRun(CoreProgressManager.java:735) at com.intellij.openapi.progress.impl.CoreProgressManager.computeUnderProgress(CoreProgressManager.java:691) at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:659) at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:79) at com.intellij.openapi.progress.impl.CoreProgressManager.runProcess(CoreProgressManager.java:202) at com.intellij.openapi.progress.ProgressManager.runProcess(ProgressManager.java:100) at com.intellij.openapi.progress.util.ProgressIndicatorUtils.runWithWriteActionPriority(ProgressIndicatorUtils.java:135) at com.intellij.openapi.progress.util.ProgressIndicatorUtils.runInReadActionWithWriteActionPriority(ProgressIndicatorUtils.java:93) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$Submission.attemptComputation(NonBlockingReadActionImpl.java:567) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$Submission.lambda$transferToBgThread$1(NonBlockingReadActionImpl.java:466) at com.intellij.openapi.application.impl.NonBlockingReadActionImpl$Submission.lambda$transferToBgThread$2(NonBlockingReadActionImpl.java:481) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:702) at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:699) at java.base/java.security.AccessController.doPrivileged(AccessController.java:399) at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1.run(Executors.java:699) at java.base/java.lang.Thread.run(Thread.java:840)

[angelozerr]

I think it is again because your Jakarta LS doesnt support resolve code action.

In MP ls and in Jakarta LS redolve code action must be supported because the compute of textedit is very expansive because it uses Java PsiFile and IJ throws many ProcessCancelledExxeption which cancel the code action.

And I repeat me but LSP4IJ can support code action without resolve with all language servers. You should see that with lemminx which doesnt support resolve code action for somes quickfixes but for MP ls and Jakarta ls it is a requirement to have very good performance in Java files and to see Quick fixes. Without resolve support IJ cancel the quickfixes if there several code actions to resolve.

[anusreelakshmi934]

Adding @turkeylurkey 's findings below: I've been looking at the missing quick fix problem. I find that three other quick fixes are affected. The JsonArray is created correctly so I'm not sure why there is a ClassCastException.

[turkeylurkey]

Just to clarify: the JsonArray is created correctly so I'm not sure how the diagnostic data field is changed into a JsonObject before this data is used and the exception results.

[turkeylurkey]

When we process a diagnostic object we get the data (diagnostic.getData()). We need this object to get an array of String values. The value of the object indicates it is an array that has been converted to an object: value: {"elements":[{"value":"jakarta.json.bind.annotation.JsonbProperty"},{"value":"jakarta.json.bind.annotation.JsonbAnnotation"},{"value":"jakarta.json.bind.annotation.JsonbTransient"}]} The object says its type is diagnostic data class name:com.google.gson.JsonObject The object's classloader hash is classloader hash:496815570 In our plugin the class JsonObject name is JsonObject class name:com.google.gson.JsonObject This class's loader hash is JsonObject classloader hash:1665090887 These classes come from different classloaders. Most likely one CL is for lsp4ij and the other CL is for LTI.

[angelozerr]

We had the same problem in IntelliJ Quarkus and we wanted to keep the gson library used in Intellij Quarkus and not from LSP4IJ

I suggest that you see how we fix that with the JsonUtils from LSP4IJ because we use getData for code action resolve

[turkeylurkey]

@angelozerr This doesn't quite work as required for our plugin. Our plugin generates a JsonArray which is set as the data in the diagnostic. Later, in the quick fix, when we call getData() we get back a JsonObject: "element" : JsonArray. That is, our array is wrapped in a JsonObject:

{"elements":[{"value":"jakarta.json.bind.annotation.JsonbProperty"},{"value":"jakarta.json.bind.annotation.JsonbAnnotation"},{"value":"jakarta.json.bind.annotation.JsonbTransient"}]}

Using your code to get JsonArray (.toModel(g, o, JsonArray.class)) we get an exception: com.google.gson.JsonIOException: Abstract classes can't be instantiated! Register an InstanceCreator or a TypeAdapter for this type. Class name: com.google.gson.JsonElement

Calling toModel with JsonObject.class just gets us an empty object "{}". If I call it with Map.class your code returns a Map<String, List<Map<String, String>>>, a literal translation of the serialized Json. We can peel this open but it is not clean.

If we could get the JsonObject as "element" : JsonArray we could call .toJsonArray() on it. I hope this brief description makes sense. Is there something we could add to toModel(g, o, clazz) to make this API more useful?

[turkeylurkey]

While some quick fixes are working, certain ones do not appear when you hover over the diagnostic. This issue arose after we started using the plugin version of lsp4ij. It is not immediately clear how it happened but the difference is observed in quick fixes which rely upon data stored in the diagnostic and are accessed using the getData() method. The diagnostic stores an array of strings as a JsonArray and the quick fix extracts the Object and casts it to this type. The problem is that the lsp4ij plugin changes the data into a JsonObject. In addition the class is loaded from the lsp4ij plugin class loader so the liberty-tools-intellij plugin cannot use it.

The reason the existing unit tests do not catch this problem is that they do not pass through the language server during testing. The test objects are created and verified locally. Therefore at least one new test case is required in the integration bucket (UI) to test quick fixes that use getData().

Affected diagnostics:

• ErrorMessageJsonbTransientOnField, JsonbTransientDiagnostic.java line 31
• ErrorMessageJsonbTransientOnAccessor, JsonbTransientDiagnostic.java line 43
• ScopeTypeAnnotationsManagedBean, ScopeDeclaration.java line 11
• ScopeTypeAnnotationsProducerField, ScopeDeclaration.java line 13
• ScopeTypeAnnotationsProducerMethod, ScopeDeclaration.java line 16

[angelozerr]

@turkeylurkey IMHO I think using JsonObject, JsonArray for data is a bad idea because for an new developer it is hard to know quickly what data contains. I think it is better to use a custom class like we did (and like you did since you have copied/pasted our IJ Quarkus) with CodeActionResolveData in https://github.com/OpenLiberty/liberty-tools-intellij/blob/a4857acddbf46b7b33d5cfc1f3185a4beb8bb8c8/src/main/java/io/openliberty/tools/intellij/lsp4mp/lsp/MicroProfileLanguageClient.java#L256

But if you want to continue to use your JsonArray, you need:

• solution 1: exclude your gson library from your plugin, but I think it is a bad idea because you don't master the version of Gson which is used by LSP4IJ.
• solution 2 : you need to get the JsonArray loaded by the Gson from LSP4IJ to a new JSonArray loaded by your plugin classloader. In LSP4IJ we did that for LSPCommand at https://github.com/redhat-developer/lsp4ij/blob/00fc081833001099e1ca1a0866f41ec408573ff9/src/main/java/com/redhat/devtools/lsp4ij/commands/LSPCommand.java#L96

So I suggest you try the following code:

JsonArray jsonArrayLoadedByYourPlugin = JSONUtils.getJsonElementFromClassloader(jsonArrayLoadedByLSP4IJ, classLoader);

where classLoader is the class loader of your plugin.

[turkeylurkey]

Thanks for the help. I haven't investigated that method.

[mrglavas]

@angelozerr Is there a compelling reason for adopters of LSP4iJ to package their own version of GSON? I'm not sure I understand why the GSON library included with LSP4iJ isn't a "good" version that everyone can use downstream to do very basic JSON processing.

[angelozerr]

Imagine that your plugin requires to use last version of GSON to support another thing than LSP (ex : in IJ Quarkus Tools we consume a JSON services to generate a project, I believe that you have the same feature), if you use the GSON version from LSP4IJ you will not be able to migrate to the new GSON version.

Imagine in the LSP4J future, the GSON library will be replaced with ananother JSON library, LSP4IJ will remove GSON library and your plugin will not work.

We have suffered about this kind of problem in IJ Quarkus (and I think you too) when IntelliJ IDEA has removed flexmark since they support now Markdown with their own MarkDown parser. Our plugin was broken with last version of IDEA and we had to add flexmark in our plugin.

It is because of those reasons that I think you should not be linked to the GSON library from LSP4IJ.

[mrglavas]

If adopters are using another JSON library and you happen to choose the same one they're using in a future version of LSP4iJ wouldn't that just introduce the same problem that currently exists with GSON? If GSON's inclusion in LSP4iJ is only meant for LSP4iJ's internal usage, is there no way to hide GSON within LSP4iJ so that it is not exposed at all?

[angelozerr]

Indeed if LSP4J (not LSP4IJ) choose an another JSON parser you will have the same problem except if you are working with Pojo class instead of using GSON JsonObject, JsonArray.

In short, if your plugin don't need GSON library execpt for LSP, you can use the GSON library from LSP4IJ, otherwise I think it is better tu use your own GSON library for your non LSP code.

It is just a suggestion not a requirement.

[anusreelakshmi934]

Tested this and we are getting the quickfixes.