From 9de85d52b9a8b3d51ec60112646e043ff222533d Mon Sep 17 00:00:00 2001 From: Roland Grunberg Date: Fri, 24 Nov 2023 12:16:40 -0500 Subject: [PATCH] Use project's Eclipse settings if extended client capability permits. - If a project has an 'org.eclipse.jdt.ui' preference node (eg. .settings/org.eclipse.jdt.ui.prefs), for clean up fixes, only use it if the client has set the 'canUseInternalSettings' extended client capability - Fix testNoConflictBetweenLSPAndJDTUI testcase Signed-off-by: Roland Grunberg --- .../internal/handlers/SaveActionHandler.java | 5 +- .../preferences/ClientPreferences.java | 4 ++ .../internal/preferences/Preferences.java | 2 +- .../core/internal/cleanup/CleanUpsTest.java | 40 ----------- .../handlers/SaveActionHandlerTest.java | 68 ++++++++++++++++--- .../AbstractProjectsManagerBasedTest.java | 6 +- 6 files changed, 73 insertions(+), 52 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandler.java index 2223974821..029811013f 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandler.java @@ -62,8 +62,9 @@ public List willSaveWaitUntil(WillSaveTextDocumentParams params, IProg Preferences preferences = preferenceManager.getPreferences(); IEclipsePreferences jdtUiPreferences = getJdtUiProjectPreferences(documentUri); + boolean canUseInternalSettings = preferenceManager.getClientPreferences().canUseInternalSettings(); if (preferences.isJavaSaveActionsOrganizeImportsEnabled() || - (jdtUiPreferences != null && jdtUiPreferences.getBoolean("sp_" + CleanUpConstants.ORGANIZE_IMPORTS, false))) { + (canUseInternalSettings && jdtUiPreferences != null && jdtUiPreferences.getBoolean("sp_" + CleanUpConstants.ORGANIZE_IMPORTS, false))) { edit.addAll(handleSaveActionOrganizeImports(documentUri, monitor)); } @@ -71,7 +72,7 @@ public List willSaveWaitUntil(WillSaveTextDocumentParams params, IProg List lspCleanups = preferences.getCleanUpActionsOnSave(); Collection jdtSettingCleanups = getCleanupsFromJDTUIPreferences(jdtUiPreferences); - cleanUpIds.addAll((lspCleanups != null && !lspCleanups.isEmpty()) ? lspCleanups : jdtSettingCleanups); + cleanUpIds.addAll(canUseInternalSettings ? jdtSettingCleanups : lspCleanups); List cleanUpEdits = cleanUpRegistry.getEditsForAllActiveCleanUps(params.getTextDocument(), new ArrayList<>(cleanUpIds), monitor); edit.addAll(cleanUpEdits); return edit; diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java index bcf7bed80a..61463936c7 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java @@ -299,6 +299,10 @@ public String getCompletionItemCommand() { return String.valueOf(extendedClientCapabilities.getOrDefault("onCompletionItemSelectedCommand", "")); } + public boolean canUseInternalSettings() { + return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("canUseInternalSettings", "false").toString()); + } + public boolean isSupportsCompletionDocumentationMarkdown() { //@formatter:off return v3supported && capabilities.getTextDocument().getCompletion() != null diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java index e6b059dab9..531fede8d9 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java @@ -1287,7 +1287,7 @@ public static Preferences createFrom(Map configuration) { * @param enabledCleanUps * the new list of enabled clean ups */ - private void setCleanUpActionsOnSave(List enabledCleanUps) { + public void setCleanUpActionsOnSave(List enabledCleanUps) { this.cleanUpActionsOnSave = enabledCleanUps; } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/cleanup/CleanUpsTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/cleanup/CleanUpsTest.java index 41164e30d9..d6bd6e8c8d 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/cleanup/CleanUpsTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/cleanup/CleanUpsTest.java @@ -15,17 +15,14 @@ import static org.junit.Assert.assertEquals; import java.io.File; -import java.nio.file.Files; import java.util.Arrays; import java.util.Collections; import java.util.Hashtable; import java.util.List; -import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.NullProgressMonitor; -import org.eclipse.core.runtime.Path; import org.eclipse.jdt.core.ICompilationUnit; import org.eclipse.jdt.core.IJavaProject; import org.eclipse.jdt.core.IPackageFragment; @@ -582,41 +579,4 @@ public void test() { assertEquals(expected, actual); } - // https://github.com/redhat-developer/vscode-java/issues/3370 - @Test - public void testNoConflictBetweenLSPAndJDTUI() throws Exception { - // addFinalModifier enabled via. cleanup.make_variable_declarations_final - IFile jdtCorePrefs = javaProject.getProject().getFile(Path.fromOSString(".settings/org.eclipse.jdt.core.prefs")); - Files.writeString(jdtCorePrefs.getLocation().toPath(), """ - editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true - sp_cleanup.make_variable_declarations_final=true"""); - - String contents = "package test1;\n" - + "public class NoConflictWithLSP {\n" - + " public void test() {\n" - + " String MESSAGE = \"This is a message.\" +\n" - + " \"This message has multiple lines.\" +\n" - + " \"We can convert it to a text block\";\n" - + " }\n" - + "}\n" - + ""; - - ICompilationUnit unit = pack1.createCompilationUnit("NoConflictWithLSP.java", contents, false, monitor); - String uri = unit.getUnderlyingResource().getLocationURI().toString(); - List textEdits = registry.getEditsForAllActiveCleanUps(new TextDocumentIdentifier(uri), Arrays.asList("stringConcatToTextBlock"), monitor); - String actual = TextEditUtil.apply(unit, textEdits); - String expected = "package test1;\n" - + "public class NoConflictWithLSP {\n" - + " public void test() {\n" - + " String MESSAGE = \"\"\"\n" - + " This is a message.\\\n" - + " This message has multiple lines.\\\n" - + " We can convert it to a text block\"\"\";\n" - + " }\n" - + "}\n" - + ""; - - assertEquals(expected, actual); - } - } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java index 1e57fd9271..f110673b91 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java @@ -18,20 +18,27 @@ import static org.mockito.Mockito.when; import java.net.URI; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.Path; import org.eclipse.jdt.core.ICompilationUnit; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.IPackageFragment; +import org.eclipse.jdt.core.IPackageFragmentRoot; +import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.ls.core.internal.JDTUtils; import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin; import org.eclipse.jdt.ls.core.internal.ResourceUtils; import org.eclipse.jdt.ls.core.internal.TextEditUtil; import org.eclipse.jdt.ls.core.internal.WorkspaceHelper; import org.eclipse.jdt.ls.core.internal.managers.ProjectsManager.CHANGE_TYPE; -import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager; -import org.eclipse.jdt.ls.core.internal.preferences.Preferences; import org.eclipse.jface.text.Document; import org.eclipse.lsp4j.TextDocumentIdentifier; import org.eclipse.lsp4j.TextEdit; @@ -43,8 +50,6 @@ public class SaveActionHandlerTest extends AbstractCompilationUnitBasedTest { private SaveActionHandler handler; - private PreferenceManager preferenceManager; - private IProgressMonitor monitor; @Override @@ -52,10 +57,8 @@ public class SaveActionHandlerTest extends AbstractCompilationUnitBasedTest { public void setup() throws Exception { importProjects("eclipse/hello"); project = WorkspaceHelper.getProject("hello"); - preferenceManager = mock(PreferenceManager.class); - Preferences preferences = mock(Preferences.class); - when(preferences.isJavaSaveActionsOrganizeImportsEnabled()).thenReturn(Boolean.TRUE); - when(preferenceManager.getPreferences()).thenReturn(preferences); + preferences.setJavaSaveActionAutoOrganizeImportsEnabled(true); + when(preferenceManager.getClientPreferences().canUseInternalSettings()).thenReturn(Boolean.FALSE); monitor = mock(IProgressMonitor.class); when(monitor.isCanceled()).thenReturn(false); handler = new SaveActionHandler(preferenceManager); @@ -137,4 +140,53 @@ public void testMissingFormatterUrl() throws Exception { } } + // https://github.com/redhat-developer/vscode-java/issues/3370 + @Test + public void testNoConflictBetweenLSPAndJDTUI() throws Exception { + // invertEquals enabled via. LSP settings + preferences.setCleanUpActionsOnSave(Arrays.asList("invertEquals")); + // addFinalModifier enabled via. cleanup.make_variable_declarations_final + IFile jdtCorePrefs = project.getFile(Path.fromOSString(".settings/org.eclipse.jdt.ui.prefs")); + Files.writeString(jdtCorePrefs.getLocation().toPath(), """ + editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true + sp_cleanup.make_variable_declarations_final=true"""); + + String contents = "package test1;\n" + + "public class NoConflictWithLSP {\n" + + " public void test() {\n" + + " String MESSAGE = \"This is a message.\";\n" + + " if (MESSAGE.equals(\"message\"))\n" + + " }\n" + + " }\n" + + "}\n" + + ""; + + IJavaProject javaProject = JavaCore.create(project); + IPackageFragmentRoot fSourceFolder = javaProject.getPackageFragmentRoot(javaProject.getProject().getFolder("src/")); + project.refreshLocal(IResource.DEPTH_INFINITE, new NullProgressMonitor()); + + IPackageFragment pack = fSourceFolder.createPackageFragment("test1", false, monitor); + ICompilationUnit unit = pack.createCompilationUnit("NoConflictWithLSP.java", contents, false, monitor); + String uri = unit.getUnderlyingResource().getLocationURI().toString(); + + WillSaveTextDocumentParams params = new WillSaveTextDocumentParams(); + TextDocumentIdentifier document = new TextDocumentIdentifier(); + document.setUri(uri); + params.setTextDocument(document); + + List result = handler.willSaveWaitUntil(params, monitor); + String actual = TextEditUtil.apply(unit, result); + String expected = "package test1;\n" + + "public class NoConflictWithLSP {\n" + + " public void test() {\n" + + " String MESSAGE = \"This is a message.\";\n" + + " if (\"message\".equals(MESSAGE))\n" + + " }\n" + + " }\n" + + "}\n" + + ""; + + assertEquals(expected, actual); + } + } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java index 8be1b76945..1fa094c876 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java @@ -104,6 +104,8 @@ public abstract class AbstractProjectsManagerBasedTest { private PreferenceManager oldPreferenceManager; + private ClientPreferences clientPreferences; + protected Preferences preferences; protected SimpleLogListener logListener; @@ -138,6 +140,9 @@ public void initProjectManager() throws Exception { if (preferenceManager == null) { preferenceManager = mock(StandardPreferenceManager.class); } + if (clientPreferences == null) { + clientPreferences = mock(ClientPreferences.class); + } initPreferenceManager(true); oldPreferenceManager = JavaLanguageServerPlugin.getPreferencesManager(); @@ -176,7 +181,6 @@ protected ClientPreferences initPreferenceManager(boolean supportClassFileConten Mockito.lenient().when(preferenceManager.getPreferences()).thenReturn(preferences); Mockito.lenient().when(preferenceManager.getPreferences(any())).thenReturn(preferences); Mockito.lenient().when(preferenceManager.isClientSupportsClassFileContent()).thenReturn(supportClassFileContents); - ClientPreferences clientPreferences = mock(ClientPreferences.class); Mockito.lenient().when(clientPreferences.isProgressReportSupported()).thenReturn(true); Mockito.lenient().when(preferenceManager.getClientPreferences()).thenReturn(clientPreferences); Mockito.lenient().when(clientPreferences.isSupportedCodeActionKind(anyString())).thenReturn(true);