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 <[email protected]>
eclipse-jdtls · Nov 24, 2023 · 013e536 · 013e536
1 parent 99d7f7b
commit 013e536
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 45 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
@@ -62,16 +62,17 @@ public List<TextEdit> 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));
 		}
 
 		LinkedHashSet<String> cleanUpIds = new LinkedHashSet<>();
 		List<String> lspCleanups = preferences.getCleanUpActionsOnSave();
 		Collection<String> jdtSettingCleanups = getCleanupsFromJDTUIPreferences(jdtUiPreferences);
 
-		cleanUpIds.addAll((lspCleanups != null && !lspCleanups.isEmpty()) ? lspCleanups : jdtSettingCleanups);
+		cleanUpIds.addAll(canUseInternalSettings ? jdtSettingCleanups : lspCleanups);
 		List<TextEdit> cleanUpEdits = cleanUpRegistry.getEditsForAllActiveCleanUps(params.getTextDocument(), new ArrayList<>(cleanUpIds), monitor);
 		edit.addAll(cleanUpEdits);
 		return edit;

diff --git a/...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java b/...lipse.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.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<TextEdit> 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/...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java b/...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SaveActionHandlerTest.java
@@ -18,18 +18,28 @@
 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.ClientPreferences;
 import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
 import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
 import org.eclipse.jface.text.Document;
@@ -45,6 +55,10 @@ public class SaveActionHandlerTest extends AbstractCompilationUnitBasedTest {
 
 	private PreferenceManager preferenceManager;
 
+	private Preferences mockPreferences;
+
+	private ClientPreferences mockClientPreferences;
+
 	private IProgressMonitor monitor;
 
 	@Override
@@ -53,9 +67,12 @@ 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);
+		mockPreferences = mock(Preferences.class);
+		mockClientPreferences = mock(ClientPreferences.class);
+		when(mockPreferences.isJavaSaveActionsOrganizeImportsEnabled()).thenReturn(Boolean.TRUE);
+		when(preferenceManager.getClientPreferences()).thenReturn(mockClientPreferences);
+		when(preferenceManager.getPreferences()).thenReturn(mockPreferences);
+		when(mockClientPreferences.canUseInternalSettings()).thenReturn(Boolean.FALSE);
 		monitor = mock(IProgressMonitor.class);
 		when(monitor.isCanceled()).thenReturn(false);
 		handler = new SaveActionHandler(preferenceManager);
@@ -137,4 +154,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
+		when(mockPreferences.getCleanUpActionsOnSave()).thenReturn(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<TextEdit> 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);
+	}
+
 }