fix: align completion comparator with LSP sortText semantics

org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/CompletionOrderingTests.java

@@ -150,6 +150,21 @@ public void testOrderWithLongInsert() throws Exception {
 				62, orderedResults);
 	}
 
+	@Test
+	public void testSortTextIsComparedLexicographically() throws Exception {
+		final var completions = new ArrayList<CompletionItem>();
+
+		final var item15 = createCompletionItem("15", CompletionItemKind.Class);
+		item15.setSortText("15");
+		completions.add(item15);
+
+		final var item5 = createCompletionItem("5", CompletionItemKind.Class);
+		item5.setSortText("5");
+		completions.add(item5);
+
+		confirmCompletionResults(completions, "", 0, new String[] { "15", "5" });
+	}
+
 	@Test
 	public void testMovingOffset() throws Exception {
 		final var range = new Range(new Position(0, 0), new Position(0, 4));

org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/LSCompletionProposalComparatorTest.java

@@ -0,0 +1,184 @@
+/*******************************************************************************
+ * Copyright (c) 2025 Vegard IT GmbH and others.
+ * 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:
+ *  Sebastian Thomschke (Vegard IT GmbH) - initial implementation
+ *******************************************************************************/
+package org.eclipse.lsp4e.test.completion;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.eclipse.jface.text.Document;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.lsp4e.operations.completion.LSCompletionProposal;
+import org.eclipse.lsp4e.operations.completion.LSCompletionProposalComparator;
+import org.eclipse.lsp4j.CompletionItem;
+import org.junit.jupiter.api.Test;
+
+public class LSCompletionProposalComparatorTest {
+
+	/**
+	 * DocumentFilter length currently has top priority in LSCompletionProposalComparator.
+	 * This test encodes the expectation that, once proposals are considered matching,
+	 * sortText should decide the order, not the document filter length.
+	 */
+	@Test
+	public void testDocumentFilterLengthDoesNotOverrideSortText() {
+		IDocument document = new Document("");
+
+		final var item1 = new CompletionItem("p1");
+		item1.setSortText("B");
+		final var item2 = new CompletionItem("p2");
+		item2.setSortText("A");
+
+		final var proposalWithLongFilter = new StubProposal(document, item1, "longFilter");
+		final var proposalWithShortFilter = new StubProposal(document, item2, "x");
+
+		final var comparator = new LSCompletionProposalComparator();
+		final var proposals = new ArrayList<LSCompletionProposal>();
+		proposals.add(proposalWithLongFilter);
+		proposals.add(proposalWithShortFilter);
+
+		proposals.sort(comparator);
+
+		// Desired order: sortText ascending -> "A", then "B"
+		assertEquals("A", proposals.get(0).getSortText());
+		assertEquals("B", proposals.get(1).getSortText());
+	}
+
+	/**
+	 * CompletionProposalPopup.computeFilteredProposals filters an already sorted
+	 * list of proposals. To avoid surprising reordering if re-sorting is added
+	 * later, LSCompletionProposalComparator must not change the relative order
+	 * of proposals solely because their documentFilter length changes.
+	 *
+	 * This test verifies that changing the documentFilter does not change the
+	 * comparator outcome for otherwise identical proposals.
+	 */
+	@Test
+	public void testFilteredProposalsShouldBeResortedWhenFilterChanges() {
+		IDocument document = new Document("");
+
+		final var item1 = new CompletionItem("p1");
+		final var item2 = new CompletionItem("p2");
+
+		final int initialOffset = 1;
+		final int updatedOffset = 2;
+
+		final var proposalA = new VarFilterProposal(document, item1);
+		final var proposalB = new VarFilterProposal(document, item2);
+
+		// At the initial offset, proposalA has a longer filter than proposalB
+		// so A should be ordered before B.
+		proposalA.setFilterForOffset(initialOffset, "xx");
+		proposalB.setFilterForOffset(initialOffset, "x");
+
+		// After typing more, the filters swap lengths so the desired order
+		// (if resorted) would become B before A.
+		proposalA.setFilterForOffset(updatedOffset, "x");
+		proposalB.setFilterForOffset(updatedOffset, "xx");
+
+		final var comparator = new LSCompletionProposalComparator();
+
+		// Initial sort at invocation offset
+		final var initiallySorted = new ArrayList<LSCompletionProposal>();
+		initiallySorted.add(proposalA);
+		initiallySorted.add(proposalB);
+		proposalA.setOffsetForSorting(initialOffset);
+		proposalB.setOffsetForSorting(initialOffset);
+		initiallySorted.sort(comparator);
+
+		// Simulate JFace behaviour: keep the original order and just filter
+		final var filteredWithoutResort = new ArrayList<>(initiallySorted);
+
+		// Expected behaviour: update filters for the new offset and resort
+		final var expectedResorted = new ArrayList<LSCompletionProposal>();
+		expectedResorted.add(proposalA);
+		expectedResorted.add(proposalB);
+		proposalA.setOffsetForSorting(updatedOffset);
+		proposalB.setOffsetForSorting(updatedOffset);
+		expectedResorted.sort(comparator);
+
+		// We would like the filtered list to reflect the re-sorted order.
+		assertEquals(expectedResorted, filteredWithoutResort);
+	}
+
+	private static class StubProposal extends LSCompletionProposal {
+
+		private final String filter;
+
+		StubProposal(IDocument document, CompletionItem item, String filter) {
+			super(document, 0, item, null, null, false);
+			this.filter = filter;
+		}
+
+		@Override
+		public String getDocumentFilter() {
+			return filter;
+		}
+
+		@Override
+		public String getDocumentFilter(int offset) {
+			return filter;
+		}
+
+		@Override
+		public int getRankCategory() {
+			return 5;
+		}
+
+		@Override
+		public int getRankScore() {
+			return 0;
+		}
+	}
+
+	private static class VarFilterProposal extends LSCompletionProposal {
+
+		private final Map<Integer, String> filtersByOffset = new HashMap<>();
+		private int sortOffset;
+
+		VarFilterProposal(IDocument document, CompletionItem item) {
+			super(document, 0, item, null, null, false);
+		}
+
+		void setFilterForOffset(int offset, String filter) {
+			filtersByOffset.put(Integer.valueOf(offset), filter);
+		}
+
+		void setOffsetForSorting(int offset) {
+			this.sortOffset = offset;
+		}
+
+		@Override
+		public String getDocumentFilter() {
+			String filter = filtersByOffset.get(Integer.valueOf(sortOffset));
+			return filter != null ? filter : "";
+		}
+
+		@Override
+		public String getDocumentFilter(int offset) {
+			String filter = filtersByOffset.get(Integer.valueOf(offset));
+			return filter != null ? filter : "";
+		}
+
+		@Override
+		public int getRankCategory() {
+			return 5;
+		}
+
+		@Override
+		public int getRankScore() {
+			return 0;
+		}
+	}
+}

org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/CompletionProposalTools.java

@@ -20,6 +20,12 @@ private CompletionProposalTools() {
 		// to avoid instances, requested by sonar
 	}
 
+	/**
+	 * Category used when the document filter does not meaningfully match
+	 * the completion filter (catch-all / no-match case).
+	 */
+	public static final int CATEGORY_NO_MATCH = 5;
+
 	/**
 	 * The portion of the document leading up to the cursor that is being used as a
 	 * filter for requesting completion assist
@@ -101,9 +107,9 @@ public static int getCategoryOfFilterMatch(String documentFilter, String complet
 		documentFilter = documentFilter.toLowerCase();
 		completionFilter = completionFilter.toLowerCase();
 		int subIndex = completionFilter.indexOf(documentFilter);
-		int topCategory = 5;
+		int topCategory = CATEGORY_NO_MATCH;
 		if (subIndex == -1) {
-			return isSubstringFoundOrderedInString(documentFilter, completionFilter) ? 4 : 5;
+			return isSubstringFoundOrderedInString(documentFilter, completionFilter) ? 4 : CATEGORY_NO_MATCH;
 		}
 		final int documentFilterLength = documentFilter.length();
 		final int completionFilterLength = completionFilter.length();

org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java

@@ -258,7 +258,7 @@ public int getRankCategory() {
 					getFilterString());
 		} catch (BadLocationException e) {
 			LanguageServerPlugin.logError(e);
-			rankCategory = 5;
+			rankCategory = CompletionProposalTools.CATEGORY_NO_MATCH;
 		}
 		this.rankCategory = rankCategory;
 		return rankCategory;

org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposalComparator.java

@@ -13,43 +13,60 @@
  *******************************************************************************/
 package org.eclipse.lsp4e.operations.completion;
 
+import static org.eclipse.lsp4e.operations.completion.CompletionProposalTools.*;
+
 import java.util.Comparator;
 
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.lsp4e.LanguageServerPlugin;
 
-final class LSCompletionProposalComparator implements Comparator<LSCompletionProposal> {
+public final class LSCompletionProposalComparator implements Comparator<LSCompletionProposal> {
 	@Override
 	public int compare(LSCompletionProposal o1, LSCompletionProposal o2) {
-		try {
-			int docFilterLen1 = o1.getDocumentFilter().length();
-			int docFilterLen2 = o2.getDocumentFilter().length();
-			if (docFilterLen1 > docFilterLen2) {
-				return -1;
-			} else if (docFilterLen1 < docFilterLen2) {
-				return +1;
+		int category1 = o1.getRankCategory();
+		int category2 = o2.getRankCategory();
+
+		// Prefer proposals that have a stronger match (categories 1-4),
+		// but use the document filter length only for those categories so that
+		// completely unmatched proposals (category CATEGORY_NO_MATCH) are not affected.
+		if (category1 < CATEGORY_NO_MATCH && category2 < CATEGORY_NO_MATCH) {
+			try {
+				int docFilterLen1 = o1.getDocumentFilter().length();
+				int docFilterLen2 = o2.getDocumentFilter().length();
+				if (docFilterLen1 > docFilterLen2) {
+					return -1;
+				} else if (docFilterLen1 < docFilterLen2) {
+					return 1;
+				}
+			} catch (BadLocationException e) {
+				LanguageServerPlugin.logError(e);
 			}
-		} catch (BadLocationException e) {
-			LanguageServerPlugin.logError(e);
 		}
-		if (o1.getRankCategory() < o2.getRankCategory()) {
+
+		if (category1 < category2) {
 			return -1;
-		} else if (o1.getRankCategory() > o2.getRankCategory()) {
-			return +1;
+		} else if (category1 > category2) {
+			return 1;
 		}
-		if ((o1.getRankCategory() < 5 && o2.getRankCategory() < 5)
-				&& (!(o1.getRankScore() == -1 && o2.getRankScore() == -1))) {
-			if (o2.getRankScore() == -1 || o1.getRankScore() < o2.getRankScore()) {
-				return -1;
-			} else if (o1.getRankScore() == -1 || o1.getRankScore() > o2.getRankScore()) {
-				return +1;
+
+		if (category1 < CATEGORY_NO_MATCH /* && category2 < CATEGORY_NO_MATCH */) {
+			int score1 = o1.getRankScore();
+			int score2 = o2.getRankScore();
+			if (!(score1 == -1 && score2 == -1)) {
+				if (score1 == -1) {
+					return 1;
+				} else if (score2 == -1) {
+					return -1;
+				} else if (score1 < score2) {
+					return -1;
+				} else if (score1 > score2) {
+					return 1;
+				}
 			}
 		}
+
 		String c1 = o1.getSortText();
 		String c2 = o2.getSortText();
-		if (c1 == null) {
-			return -1;
-		}
 		return c1.compareToIgnoreCase(c2);
 	}
-}
+}