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.

Sign up for GitHub

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

Jump to bottom

Using extension's lombok version #2550

Merged
merged 7 commits into from
Jul 19, 2022
Merged

Using extension's lombok version #2550

merged 7 commits into from
Jul 19, 2022

Conversation

XiaoYuhao
Copy link
Contributor

Add the feature of using extension's lombok version by default.
If the lombok support is enable, the extension always add the
latest version lombok.jar to start the JDT LS. It can avoid the
errors caused by old version lombok.

The user can select the extension's lombok version or the project's
lombok version.

Signed-off-by: Yuhao Xiao [email protected]

@testforstephen testforstephen mentioned this pull request Jul 12, 2022
Closed
testforstephen
testforstephen reviewed Jul 12, 2022
View reviewed changes
gulpfile.js Outdated Show resolved Hide resolved
testforstephen
testforstephen reviewed Jul 12, 2022
View reviewed changes
gulpfile.js Outdated Show resolved Hide resolved
This was linked to issues Jul 13, 2022
Java extension is broken with some old lombok.jar (e.g. 1.14.4) #2549
Closed
Java extension crashed if open lombok project after deleting .m2 folder #2556
Closed
@testforstephen testforstephen marked this pull request as ready for review July 14, 2022 12:20
testforstephen
testforstephen approved these changes Jul 14, 2022
View reviewed changes
Copy link
Collaborator

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

  • The language status can show the current lombok version used in Java extension.
    image

  • Clicking "Configure Version" will allow choosing between extension's version and project's version.
    image

@testforstephen testforstephen requested a review from rgrunber July 14, 2022 12:29
@testforstephen
Copy link
Collaborator

testforstephen commented Jul 14, 2022 •
edited
Loading

@rgrunber This PR will package a lombok.jar in Java extension, could you double check if the license is OK?

https://projectlombok.org/
Copyright © 2009-2022 The Project Lombok Authors, licensed under the MIT license.

Here is the license info displayed in the footer of projectlombok website.

And see the license on source repo as well https://github.com/projectlombok/lombok

@rgrunber
Copy link
Member

Yes, according to https://clearlydefined.io/definitions/maven/mavencentral/org.projectlombok/lombok/1.18.24 (which is also one of the sources Eclipse now uses), the license is MIT, which is acceptable for us to include as a 3rd party library. Note that there are also other MIT-licensed projects under redhat-developer that use 3rd party libraries also licensed as MIT.

rgrunber
rgrunber reviewed Jul 18, 2022
View reviewed changes
Copy link
Member

@rgrunber rgrunber left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a project (tried with invisible or Maven), with java.jdt.ls.lombokSupport.enabled , and another version of lombok on the project classpath. When the project loads click on the lombok version status bar to confirm the 2 versions (embedded & project) are detected. Delete the project version and click back on the status bar. Nothing will happen. Checking the extension host log, I see :

stacktrace 
[2022-07-18 15:49:26.405] [exthost] [error] TypeError: Cannot read properties of null (reading '0')
	at lombokPath2Version (/home/rgrunber/git/vscode-java/dist/extension.js:77776:58)
	at Object.<anonymous> (/home/rgrunber/git/vscode-java/dist/extension.js:77903:30)
	at Generator.next (<anonymous>)
	at /home/rgrunber/git/vscode-java/dist/extension.js:77715:71
	at new Promise (<anonymous>)
	at /home/rgrunber/git/vscode-java/dist/extension.js:77711:12
	at /home/rgrunber/git/vscode-java/dist/extension.js:77890:123
	at o._executeContributedCommand (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:85:62871)
	at o.$executeContributedCommand (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:85:63575)
	at s._doInvokeHandler (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:95:13708)
	at s._invokeHandler (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:95:13390)
	at s._receiveRequest (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:95:12159)
	at s._receiveOneMessage (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:95:10853)
	at /usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:95:8957
	at w.invoke (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:145)
	at v.deliver (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:2266)
	at p.fire (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:1844)
	at a.fire (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:18976)
	at /usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:111:34679
	at w.invoke (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:145)
	at v.deliver (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:2266)
	at p.fire (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:1844)
	at a.fire (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:18976)
	at r._receiveMessage (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:23563)
	at /usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:21097
	at w.invoke (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:145)
	at v.deliver (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:2266)
	at p.fire (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:61:1844)
	at p.acceptChunk (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:15807)
	at /usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:69:14937
	at Socket.R (/usr/share/code/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:111:13763)
	at Socket.emit (node:events:390:28)
	at addChunk (node:internal/streams/readable:315:12)
	at readableAddChunk (node:internal/streams/readable:289:9)
	at Socket.push (node:internal/streams/readable:228:10)
	at Pipe.onStreamRead (node:internal/stream_base_commons:199:23)
	at Pipe.callbackTrampoline (node:internal/async_hooks:130:17) java.lombokConfigure {"value":"redhat.java","_lower":"redhat.java"}

projectLombokPath is updated to undefined after removal (classpath update triggered) but I guess since the status bar is already showing, those methods aren't expecting to deal with undefined.

If this is a fairly minor issue, I would feel free to merge and we can prepare to fix it in a subsequent commit, as it seems to be working for me.

@XiaoYuhao XiaoYuhao force-pushed the lombok_support_v1.1 branch from ffe63f8 to 1c5e991 Compare July 19, 2022 06:09
@testforstephen
Copy link
Collaborator

Take a project (tried with invisible or Maven), with java.jdt.ls.lombokSupport.enabled , and another version of lombok on the project classpath. When the project loads click on the lombok version status bar to confirm the 2 versions (embedded & project) are detected. Delete the project version and click back on the status bar. Nothing will happen. Checking the extension host log, I see :

stacktrace
projectLombokPath is updated to undefined after removal (classpath update triggered) but I guess since the status bar is already showing, those methods aren't expecting to deal with undefined.

If this is a fairly minor issue, I would feel free to merge and we can prepare to fix it in a subsequent commit, as it seems to be working for me.

@rgrunber The latest changes work for me. Could you try again?

@rgrunber
Using extension's lombok version
e5ba194 
Add the feature of using extension's lombok version by default.
If the lombok support is enable, the extension always add the
latest version lombok.jar to start the JDT LS. It can avoid the
errors caused by old version lombok.

The user can select the extension's lombok version or the project's
lombok version.

Signed-off-by: Yuhao Xiao <[email protected]>
Yuhao Xiao added 5 commits July 19, 2022 16:17
@rgrunber
fix some issues.
690cf9d 
Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber
If the user want to use project's lombok which is lower then 1.18.2,
3949aac 
we send a warning message and continue to use extension's lombok.

Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber
Fix some issues.
261c117 
Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber
Fix some issues.
5a55ad9 
Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber
Fix the 'projectLombokPath' undefined error
90377d7 
1. If 'projectLombokPath' is updated to 'undefined' after removal, we
   hide the Lombok status bar. When Lombok is added into the project,
   we show the Lombok status bar again.

2. In the 'LombokConfigureCommand' function, we check 'extensionLombokPath'
   and 'projectLombokPath' first. If one of them is 'undefined', it is
   nothing to do.

Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber rgrunber added this to the End July 2022 milestone Jul 19, 2022
@rgrunber
When 'projectLombokPath' is updated to 'undefined' after removal, we
641c5b3 
clean the project's Lombok classpath cache.

Signed-off-by: Yuhao Xiao <[email protected]>
@rgrunber rgrunber force-pushed the lombok_support_v1.1 branch from 1c5e991 to 641c5b3 Compare July 19, 2022 21:15
@rgrunber
Copy link
Member

rgrunber commented Jul 19, 2022 •
edited
Loading

Just adjusted the java.jdt.ls.lombokSupport.enabled description. Now that we embed the lombok jar, it doesn't just detect it from the project classpath, it outright provides the support if not available.

We should also update https://github.com/redhat-developer/vscode-java/wiki/Lombok-support .

@rgrunber rgrunber merged commit 392cd67 into redhat-developer:master Jul 19, 2022
@testforstephen
Copy link
Collaborator

I have updated the wiki about Lombok support requirement in 1.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrunber rgrunber rgrunber approved these changes

@testforstephen testforstephen testforstephen approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
End July 2022
3 participants
@XiaoYuhao @testforstephen @rgrunber