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

Notify clients that enabling preview features on incompatible compliance level fails the build #3131

Closed
HaraldKi opened this issue Apr 9, 2024 · 9 comments · Fixed by #3137
Assignees

Comments

@HaraldKi
Copy link

HaraldKi commented Apr 9, 2024

When adding
org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=enabled to a settings file loaded per the initialize request, versions 1.34.0 and 1.35.0 get problems.

Versions check:

  • jdt-language-server-1.33.0-202402151717: works
  • jdt-language-server-1.34.0-202404031240: starts but does not send any compilation errors
  • jdt-language-server-1.35.0-202404090315: fails eglot's 30s request timeout on initialize

This is what eglot reports as being sent as part of the initialize request:

    "initializationOptions": {
      "settings": {
        "java": {
          "format": {
            "enabled": true,
            "profile": "harald-variant",
            "settings": {
              "url": "/home/harald/.emacs.d/eclipse-formatter.xml"
            }
          },
          "settings": {
            "url": "/home/harald/.emacs.d/eclipse-settings.prefs"
          }
        }
      }

In the latter file, eclipse-settings.prefs, I have

org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=enabled

and get the behavior described above.

If I comment out the enablePreviewFeatures, 1.34 and 1.35 start up normally, but I don't get the preview features, obviously.

The .metadata/.log file does not contain anything helpful. Neither does eglot's communication log.

Is there some debug logging I can enable to get more hints?

For reference, here is how I start the server:

 java \
	-Declipse.application=org.eclipse.jdt.ls.core.id1 \
	-Dosgi.bundles.defaultStartLevel=4 \
	-Declipse.product=org.eclipse.jdt.ls.core.product \
	-Dlog.level=ALL \
	-Xmx1G \
	--add-modules=ALL-SYSTEM \
	--add-opens java.base/java.util=ALL-UNNAMED \
	--add-opens java.base/java.lang=ALL-UNNAMED \
	-jar "$JDTLS/plugins/org.eclipse.equinox.launcher_"*.jar \
	-configuration "$JDTLS/config_linux" \
	-data "$DATA"

where java is openjdk-21.

@rgrunber
Copy link
Contributor

rgrunber commented Apr 9, 2024

I'm guessing your project that is being opened has Java 21 compliance ? In 1.34.0 we added support for Java 22, which means the "preview feature" flags apply to Java 22 from that point on. It cannot be used against an older JDK once support for a newer one exists. It's not that well explained but https://bugs.eclipse.org/bugs/show_bug.cgi?id=549258#c11 describes it.

Could you try using Java 22 to start up, or setting your compiler compliance for the project to Java 22 ? With that, any features that are in their 2nd preview phase for Java 22 should be there.

I'm also aware this can be an annoying limitation for those wishing to say on Java 21 (it is LTS..).

@HaraldKi
Copy link
Author

You are right. The project is a gradle project with

project.sourceCompatibility = 21
project.targetCompatibility = 21

and I am using openjdk-21. After switching to openjdk-22, the problem goes away, albeit, as you say, I don't get hints from the compiler when using features which are preview for 21.

I understand that "--enable-preview" applies with respect to the eclipse.jdt.ls version in use and not to the targeted JDK version. This seems logical.

But I wonder if the server behavior could/should be changed from "unresponsive" to a clear error message stating the problem, even combined with System.exit() if operation is so severely compromised. As I said, I could find no hint in neither .metadata/.log nor the communication log provided by the client (eglot).

@HaraldKi

This comment was marked as outdated.

@rgrunber
Copy link
Contributor

But I wonder if the server behavior could/should be changed from "unresponsive" to a clear error message stating the problem, even combined with System.exit() if operation is so severely compromised. As I said, I could find no hint in neither .metadata/.log nor the communication log provided by the client (eglot).

JDT-LS does send messages back to the client like :

[Trace - 17:39:45] Received notification 'language/actionableNotification'.
Params: {
    "severity": 1,
    "message": "Invalid runtime for JavaSE-22: Runtime at '/usr/lib/jvm/java-22-openjdk/' is not compatible with the 'JavaSE-22' environment.",
    "commands": [
        {
            "title": "Open Settings",
            "command": "java.runtimeValidation.open"
        }
    ]
}

if one configures a JavaSE-22 on 1.33.0. With a Maven project, the runtime seems to select the next most compatible, and on my system I do have Java 21 as well. These kinds of messages aren't really defined by the LSP though so it'd be up to clients to handle them properly. However, the language server shouldn't be unresponsive. Most things should continue to work. Is there anything at all in the .metadata/.log that indicates something failed ?

Do you mean starting JDT-LS 1.33.0 with a Java 22 runtime, or just that having a project with Java 22 compliance doesn't work ? JDT-LS 1.33.0 didn't have support for Java 22 but one should have no issues using any Java >= 17 as the runtime and a compiler compliance anywhere between 1.6 to 21 for JDT-LS 1.33.0 (and that range grows to include 22 at 1.34.0).

@HaraldKi
Copy link
Author

I had to retract my latest comment as I seem to have done too many experiments and combinations and confused some things. Let me try a more thorough summary.

Lets start with this combination:

  • jdt.ls: 1.34
  • jdk: 22

I tried the following variations:

  • org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=enabled in the settings or commented out.
  • gradle project.sourceCompatibility as well as targetCompatibility both set to either 21 or 22 in build.gradle.

Of these four combinations, the combination in/21 does not work. The server starts and communication seems to flow, but the server just does not report any very obvious syntax errors.

This is what I meant with "unresponsive" in a previous comment. The eglot logs show messages coming in, but they are like:

{
  "jsonrpc": "2.0",
  "method": "textDocument/publishDiagnostics",
  "params": {
    "uri": "file:///some/path/Tryout.java",
    "diagnostics": []
  }
}

And respectively with previews enabled, the syntax error is not marked, naturally:

with-preview

While after disabling the preview features:

no-preview

I am sorry that my previous comment was wrong. I experimented with lots of combinations of jdt.ls and jdk versions and specifying or not enablePreviewFeatures and setting source/targetCompatibility to 21 or 22 or not at all and, in one setup I had overlooked, even org.eclipse.jdt.core.compiler.source in the settings. But the above in/out scenario is now stable and reproducible, with nothing interesting in .metatdata/.log (attached).

@rgrunber rgrunber added bug and removed question labels Apr 12, 2024
@rgrunber
Copy link
Contributor

You're absolutely right.

21-preview-1.34-fails.mp4

Eclipse also has the same behaviour (if you manually add the setting) :

image

The UI actually prevents you from enabling preview features unless you're using Java 22 compliance.

image

The client/server communication mentions :

[Trace - 13:41:55] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "Apr. 12, 2024, 1:41:55 p.m. Error occured while building workspace. Details: \n message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/main/java/org/example/App.java;\n message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/test/java/org/example/AppTest.java;"
}


[Error - 13:41:55] Apr. 12, 2024, 1:41:55 p.m. Error occured while building workspace. Details: 
 message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/main/java/org/example/App.java;
 message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/test/java/org/example/AppTest.java;

Our notification to the client is cryptic at best. The odd behaviour of the diagnostics disappearing is a similar to redhat-developer/vscode-java#1111 . So there's basically 2 issues :

  • The diagnostics that might give a hint as to there being a problem are disappearing with JDT-LS
  • There's no explicit indication that enabling the preview feature on an older execution environment (eg. < 22) would produce a build error and since we actively recommend adding settings to text files, the only way is for the language server to send a better warning/error.

@rgrunber rgrunber changed the title Startup fail (1.35.0) or compilation not working (1.34.0) when using enablePreviewFeatures=enabled in settings Inform clients explicitly that enabling preview features on incompatible compliance level is a build failure Apr 12, 2024
@rgrunber rgrunber changed the title Inform clients explicitly that enabling preview features on incompatible compliance level is a build failure Notify clients that enabling preview features on incompatible compliance level is a build failure Apr 12, 2024
@rgrunber rgrunber changed the title Notify clients that enabling preview features on incompatible compliance level is a build failure Notify clients that enabling preview features on incompatible compliance level fails the build Apr 12, 2024
@HaraldKi
Copy link
Author

Having everything in the UI is certainly a good thing, but given that the language server is attracting diverse clients which may or may not provide the error messages in a suitable form, I personally wouldn't mind to have more information in .metadata/.log, in addition to information sent to the client. A software developer should be used to looking at logs once in a while, and if we learn that this file may help to debug problems, it is easy to check. (Another issue would be to give it a more telling name, rather than .log 😃.)

But I guess more logging might open a can of worms, as the strategy currently seems to be to rather not log anything. I mention it anyway, trying to nudge in the 'more logging' direction.

@rgrunber
Copy link
Contributor

I think we can do better than just logging. We could send a notification that clients can use to display a more prominent error message. The errors do show up in the .log file as :

!ENTRY org.eclipse.jdt.ls.core 4 0 2024-04-15 19:51:17.594
!MESSAGE Error occured while building workspace. Details: 
 message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/main/java/org/example/App.java;
 message: Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22; code: 2098258; resource: /home/rgrunber/sample-projects/gradle-simple/app/src/test/java/org/example/AppTest.java;

and get sent as window/logMessage which is something a client can definitely listen to.

@rgrunber
Copy link
Contributor

The language server should now report a notification of the form :

[Trace - 17:30:12] Received notification 'language/eventNotification'.
Params: {
    "eventType": 600,
    "data": [
        {
            "uri": "file:///home/rgrunber/sample-projects/gradle-simple/app",
            "message": "Preview features enabled at an invalid source release level 21, preview can be enabled only at source level 22"
        }
    ]
}

for each project in the workspace that configures the source release level and enables preview features in an incompatible manner. It will do so on project import and/or when a build occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants