OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
12 stars 24 forks source link

LSP4iJ 2024/04/01 integration: LCLS hover fails with an IllegalArgumentException. #711

Closed mrglavas closed 5 months ago

mrglavas commented 5 months ago

With the branch of LSP4iJ from here: https://github.com/MicroShed/lsp4ij/tree/q0401, getting documentation with hover is failing with the following exception (see below). I believe this is based off a snapshot that Paul took from here: https://github.com/redhat-developer/lsp4ij very recently (April 1st).

java.lang.IllegalArgumentException: Argument for @NotNull parameter 'file' of com/intellij/openapi/vfs/VfsUtilCore.virtualToIoFile must not be null
    at com.intellij.openapi.vfs.VfsUtilCore.$$$reportNull$$$0(VfsUtilCore.java)
    at com.intellij.openapi.vfs.VfsUtilCore.virtualToIoFile(VfsUtilCore.java)
    at org.microshed.lsp4ij.LSPIJUtils.toUri(LSPIJUtils.java:179)
    at org.microshed.lsp4ij.LSPIJUtils.toTextDocumentIdentifier(LSPIJUtils.java:634)
    at org.microshed.lsp4ij.features.codelens.LSPCodeLensSupport.<init>(LSPCodeLensSupport.java:46)
    at org.microshed.lsp4ij.LSPFileSupport.<init>(LSPFileSupport.java:55)
    at org.microshed.lsp4ij.LSPFileSupport.createSupport(LSPFileSupport.java:183)
    at org.microshed.lsp4ij.LSPFileSupport.getSupport(LSPFileSupport.java:173)
    at org.microshed.lsp4ij.features.documentation.LSPDocumentationProvider.generateDoc(LSPDocumentationProvider.java:110)
    ...

Seems it may have been broken by this commit: https://github.com/redhat-developer/lsp4ij/commit/44c528ffddf4a2e99b5d0ce8d1e666accc7669de#diff-3d8fe11c1cd05e4c7f8ea16ae7d057863794ed222b51ec69818b767efb13ca8b

See changes to the generateDoc() method in LSPDocumentationProvider.java.

mrglavas commented 5 months ago

This PR: https://github.com/MicroShed/lsp4ij/pull/15 seems to resolve the issue but needs more testing.

angelozerr commented 5 months ago

The fix is indeed correct. I have reported this fix in the official LSP4IJ, but it would be very nice if you could contribute to the official LSP4IJ.

mrglavas commented 5 months ago

Thanks for verifying. I wasn't sure that this was the correct fix or if this would have been applied to the official LSP4iJ since I saw there was another PR already open (as a draft) which refactors this code again.

angelozerr commented 5 months ago

Thanks for verifying.

You are welcome.

I wasn't sure that this was the correct fix or if this would have been applied to the official LSP4iJ since I saw there was another PR already open (as a draft) which refactors this code again.

I think you mention https://github.com/redhat-developer/lsp4ij/pull/171 which uses a non deprecated API, but I need to find time to finish it.

mrglavas commented 5 months ago

The LCLS server.xml hover test is passing:

GradleSingleModLSTest > testServerXMLFeatureHover() STANDARD_OUT
    INFO: 2024-04-09T17:46:23.542612297: GradleSingleModLSTest.testServerXMLFeatureHover(). Entry
    Description: This feature provides 
    support for the MicroProfile Health 
    specification.

    Enabled by: microProfile-5.0, 
    microProfile-6.0, microProfile-6.1

    Enables: cdi-3.0, jndi-1.0, json-1.0, 
    mpConfig-3.0
    INFO: 2024-04-09T17:46:30.085877781: GradleSingleModLSTest.testServerXMLFeatureHover(). Exit

The build: https://github.com/OpenLiberty/liberty-tools-intellij/actions/runs/8617069262/job/23616171201?pr=716 had some other failures related to hover and diagnostics but I'll open other issues to investigate those ones.