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

addFinalModifier save action cannot be disabled #3370

Closed
knutae opened this issue Oct 30, 2023 · 8 comments · Fixed by eclipse-jdtls/eclipse.jdt.ls#2945
Closed

addFinalModifier save action cannot be disabled #3370

knutae opened this issue Oct 30, 2023 · 8 comments · Fixed by eclipse-jdtls/eclipse.jdt.ls#2945
Labels

Comments

@knutae
Copy link

knutae commented Oct 30, 2023

After upgrading to the latest version (1.24.0), final modifiers are added to parameters and variables on save, even though the addFinalModifier has been explicitly omitted from the Java save actions settings.

Environment
  • Operating System: Ubuntu 22.04
  • JDK version: 17.0.9
  • Visual Studio Code version: 1.83.1
  • Java extension version: 1.24.0
Steps To Reproduce
  1. Configure Java Save Actions to only contain addOverride. E.g.: "java.cleanup.actionsOnSave": ["addOverride"]
  2. Edit and save any Java file, and observe that final modifiers are added automatically.
Current Result

Final modifiers are added during save, and there does not appear to be a way to disable this.

Expected Result

Final modifiers should only be added if the "addFinalModifier" save action is configured.

Additional Information

The issue exists both in 1.24.0 and the latest prerelease version. Downgrading the plugin to 1.23.0 fixes the issue.

@snjeza
Copy link
Contributor

snjeza commented Oct 30, 2023

I can't reproduce the issue.
@knutae could you check your workspace settings (<your_project>/.vscode/settings.json)?

@stianeikeland
Copy link

I'm experiencing the same issue, on macOS 14, vscode 1.83.1, extension 1.24.0.

Workspace settings contain:

"java.cleanup.actionsOnSave": ["addOverride"],

Personal settings.json doesn't contain any java.cleanup settings.

@snjeza
Copy link
Contributor

snjeza commented Oct 30, 2023

@knutae @stianeikeland could you try to remove the <your_project>/.settings/org.eclipse.jdt.ui.prefs file if it exists.

@knutae
Copy link
Author

knutae commented Oct 31, 2023

Yes, removing .settings/org.eclipse.jdt.ui.prefs fixes the problem. Investigated a bit further and figured out that flipping sp_cleanup.make_variable_declarations_final from true to false also fixes it.

@rgrunber
Copy link
Member

Seems like this is due to eclipse-jdtls/eclipse.jdt.ls#2870 . The language server detects the preference constants that would have been supported in Eclipse-land under settings, and opts to include them. @knutae , did you also have editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup entry within that .settings/org.eclipse.jdt.ui.prefs file ?

It's a bit odd that a user could enable cleanups via. the vscode option, "java.cleanup.actionsOnSave" and cleanups not mentioned there get activated because they happen to have a .settings/org.eclipse.jdt.ui.prefs file with some cleanups set there. On the other hand, we are just respecting (merging) settings in supported settings files.

@knutae
Copy link
Author

knutae commented Nov 1, 2023

Yes editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true is also present.

@rgrunber
Copy link
Member

rgrunber commented Nov 6, 2023

Based on the fact that this is causing confusion, I'll disable it when we detect that JDT-LS is running headless (or when there is no jdt.ui bundle detected)

@rgrunber
Copy link
Member

I think I'll close this for now as the main issue is addressed. There remains a much smaller issue, which is that if java.cleanup.actionsOnSave is empty, or not defined at all, then any cleanups defined under .settings/org.eclipse.jdt.ui.prefs (assuming editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true) will get picked up.

This is really a case of the client wanting to take ownership over a setting for all scopes, but a particular project type (eg. Eclipse-style projects) has a way to set those settings at the project-level through another mechanism. I don't think it's too annoying to respect those setting if someone has taken the time to place them in the project.

This reminds me a lot of eclipse-jdtls/eclipse.jdt.ls#2942 .

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

Successfully merging a pull request may close this issue.

4 participants