fix: Make sure DefaultFormatRegionsProvider is the default IFormatRegionsProvider

FlorianKroiss · rubenporras · commit 64f9ed6afe31 · 2025-11-06T11:43:48.000+01:00
+ Add `service.ranking=1` to the properties of DefaultFormatRegionsProvider, so that it is properly used as the default
+ Extract lookup for IFormatRegionsProvider into Utility class
+ Add test cases for lookup
diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatRegionsProviderUtilTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatRegionsProviderUtilTest.java
@@ -0,0 +1,69 @@
+/*******************************************************************************
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   See git history
+ *******************************************************************************/
+package org.eclipse.lsp4e.test.format;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.Dictionary;
+import java.util.Hashtable;
+
+import org.eclipse.jdt.annotation.Nullable;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.IRegion;
+import org.eclipse.lsp4e.format.DefaultFormatRegionsProvider;
+import org.eclipse.lsp4e.format.IFormatRegionsProvider;
+import org.eclipse.lsp4e.internal.FormatRegionsProviderUtil;
+import org.junit.Test;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.ServiceRegistration;
+
+public class FormatRegionsProviderUtilTest {
+
+	@Test
+	public void lookupOnlyDefault() throws Exception {
+		var provider = FormatRegionsProviderUtil.lookup("noFormatProviderForThisId"); //$NON-NLS-1$
+		assertTrue(provider instanceof DefaultFormatRegionsProvider);
+	}
+
+	@Test
+	public void lookup() throws Exception {
+		// Setup
+		IFormatRegionsProvider instance = new IFormatRegionsProvider() {
+
+			@Override
+			public IRegion @Nullable [] getFormattingRegions(IDocument document) {
+				return null;
+			}
+
+		};
+
+		BundleContext bundleContext = FrameworkUtil.getBundle(FormatRegionsProviderUtil.class).getBundleContext();
+		final Dictionary<String, Object> props = new Hashtable<>();
+		props.put("serverDefinitionId", "foo");
+		ServiceRegistration<IFormatRegionsProvider> serviceRegistration = bundleContext
+				.registerService(IFormatRegionsProvider.class, instance, props);
+
+		IFormatRegionsProvider provider;
+		// There is no matching provider, so we expect the default.
+		provider = FormatRegionsProviderUtil.lookup("notMatching");
+		assertTrue(provider instanceof DefaultFormatRegionsProvider);
+
+		// There is a matching provider
+		provider = FormatRegionsProviderUtil.lookup("foo"); //$NON-NLS-1$
+		assertTrue(provider == instance);
+
+		// Cleanup
+		serviceRegistration.unregister();
+	}
+
+}
diff --git a/org.eclipse.lsp4e/OSGI-INF/org.eclipse.lsp4e.format.DefaultFormatRegionsProvider.xml b/org.eclipse.lsp4e/OSGI-INF/org.eclipse.lsp4e.format.DefaultFormatRegionsProvider.xml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" name="org.eclipse.lsp4e.format.DefaultFormatRegionsProvider">
+   <property name="service.ranking" value="1"/>
    <service>
       <provide interface="org.eclipse.lsp4e.format.IFormatRegionsProvider"/>
    </service>
diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/DocumentContentSynchronizer.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/DocumentContentSynchronizer.java
@@ -45,6 +45,7 @@
 import org.eclipse.jface.text.MultiTextSelection;
 import org.eclipse.lsp4e.format.IFormatRegionsProvider;
 import org.eclipse.lsp4e.internal.DocumentUtil;
+import org.eclipse.lsp4e.internal.FormatRegionsProviderUtil;
 import org.eclipse.lsp4e.operations.format.LSPFormatter;
 import org.eclipse.lsp4e.ui.Messages;
 import org.eclipse.lsp4j.DidChangeTextDocumentParams;
@@ -68,9 +69,6 @@
 import org.eclipse.lsp4j.jsonrpc.messages.Either;
 import org.eclipse.lsp4j.services.LanguageServer;
 import org.eclipse.osgi.util.NLS;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
 
 final class DocumentContentSynchronizer implements IDocumentListener {
 
@@ -304,34 +302,12 @@ private void formatDocument() {
 	}
 
 	private synchronized IRegion @Nullable [] getFormatRegions() {
+		if (formatRegionsProvider == null) {
+			formatRegionsProvider = FormatRegionsProviderUtil.lookup(languageServerWrapper.serverDefinition.id);
+		}
 		if (formatRegionsProvider != null) {
 			return formatRegionsProvider.getFormattingRegions(document);
 		}
-		var serverId = "(serverDefinitionId=" + languageServerWrapper.serverDefinition.id + ")";  //$NON-NLS-1$ //$NON-NLS-2$
-		final var bundle = FrameworkUtil.getBundle(this.getClass());
-		if (bundle != null) {
-			var bundleContext = bundle.getBundleContext();
-			if (bundleContext != null) {
-				try {
-					ServiceReference<?> reference = null;
-					var serviceReferences = bundleContext.getAllServiceReferences(IFormatRegionsProvider.class.getName(), serverId);
-					if (serviceReferences != null) {
-						reference = serviceReferences[0];
-					} else {
-						//Use LSP4E default implementation:
-						reference = bundleContext.getServiceReference(IFormatRegionsProvider.class.getName());
-					}
-					if (reference != null) {
-						formatRegionsProvider = (IFormatRegionsProvider) bundleContext.getService(reference);
-						if (formatRegionsProvider != null) {
-							return formatRegionsProvider.getFormattingRegions(document);
-						}
-					}
-				} catch (InvalidSyntaxException e) {
-					LanguageServerPlugin.logError(e);
-				}
-			}
-		}
 		return null;
 	}
 
diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/format/DefaultFormatRegionsProvider.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/format/DefaultFormatRegionsProvider.java
@@ -14,13 +14,17 @@
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jface.text.IDocument;
 import org.eclipse.jface.text.IRegion;
+import org.osgi.framework.BundleContext;
 import org.osgi.service.component.annotations.Component;
 
 /**
  * Default OSGi service implementation if no bundle provides a OSGi service for {@link IFormatRegionsProvider}.
  *
+ * <p>
+ * This implementation has a 'service.ranking' of 1 so it is chosen as the default when querying
+ * {@link BundleContext#getServiceReference(String)} without specifying a serverDefinitionId.
  */
-@Component
+@Component(property={"service.ranking=1"})
 public class DefaultFormatRegionsProvider implements IFormatRegionsProvider {
 
 	@Override
diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/FormatRegionsProviderUtil.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/FormatRegionsProviderUtil.java
@@ -0,0 +1,70 @@
+/*******************************************************************************
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   See git history
+ *******************************************************************************/
+package org.eclipse.lsp4e.internal;
+
+import org.eclipse.jdt.annotation.Nullable;
+import org.eclipse.lsp4e.LanguageServerPlugin;
+import org.eclipse.lsp4e.format.IFormatRegionsProvider;
+import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.framework.ServiceReference;
+
+public class FormatRegionsProviderUtil {
+
+	private FormatRegionsProviderUtil() {
+		// this class shouldn't be instantiated
+	}
+
+	/**
+	 * Lookup the {@link IFormatRegionsProvider} for the given server definition id.
+	 *
+	 * @param serverDefinitionId
+	 *            The id the of the language server
+	 * @return The found {@link IFormatRegionsProvider} or {@code null}.
+	 * @see IFormatRegionsProvider
+	 */
+	public static @Nullable IFormatRegionsProvider lookup(String serverDefinitionId) {
+		final var bundle = FrameworkUtil.getBundle(FormatRegionsProviderUtil.class);
+		if (bundle == null) {
+			return null;
+		}
+
+		var bundleContext = bundle.getBundleContext();
+		if (bundleContext == null) {
+			return null;
+		}
+
+		final ServiceReference<?>[] serviceReferences;
+		try {
+			String filter = "(serverDefinitionId=" + serverDefinitionId + ")"; //$NON-NLS-1$ //$NON-NLS-2$
+			serviceReferences = bundleContext.getAllServiceReferences(IFormatRegionsProvider.class.getName(), filter);
+		} catch (InvalidSyntaxException e) {
+			LanguageServerPlugin.logError(e);
+			return null;
+		}
+
+		final ServiceReference<?> reference;
+		if (serviceReferences != null) {
+			reference = serviceReferences[0];
+		} else {
+			// Fallback to default
+			reference = bundleContext.getServiceReference(IFormatRegionsProvider.class.getName());
+		}
+
+		if (reference == null) {
+			return null;
+		}
+
+		return (IFormatRegionsProvider) bundleContext.getService(reference);
+	}
+
+}
