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 27, 2023 · 9de85d5 · 9de85d5
1 parent 4a4ddb8
commit 9de85d5
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 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
@@ -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.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<String, Object> configuration) {
 	 * @param enabledCleanUps
 	 *            the new list of enabled clean ups
 	 */
-	private void setCleanUpActionsOnSave(List<String> enabledCleanUps) {
+	public void setCleanUpActionsOnSave(List<String> 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
@@ -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,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,19 +50,15 @@ public class SaveActionHandlerTest extends AbstractCompilationUnitBasedTest {
 
 	private SaveActionHandler handler;
 
-	private PreferenceManager preferenceManager;
-
 	private IProgressMonitor monitor;
 
 	@Override
 	@Before
 	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<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);
+	}
+
 }
diff --git a/...tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java b/...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);