Fix artifact versions sorting in Content Assistant

Fixes: eclipse#410
vrubezhny · Jul 18, 2023 · e3b9d4b · e3b9d4b
1 parent 1ebfbdf
commit e3b9d4b
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 21 deletions.
diff --git a/.../eclipse/lemminx/extensions/maven/participants/completion/MavenCompletionParticipant.java b/.../eclipse/lemminx/extensions/maven/participants/completion/MavenCompletionParticipant.java
@@ -66,6 +66,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.jar.JarEntry;
@@ -303,7 +304,6 @@ public void onXMLContent(ICompletionRequest request, ICompletionResponse respons
 
 			// Sort and move nonArtifactCollector items to the response and clear nonArtifactCollector
 			nonArtifactCollector.entrySet().stream()
-//			.sorted(null)
 			.map(entry -> entry.getValue()).forEach(response::addCompletionItem);
 			nonArtifactCollector.clear();
 			break;
@@ -354,21 +354,23 @@ public void onXMLContent(ICompletionRequest request, ICompletionResponse respons
 				response.addCompletionItem(toTextCompletionItem(request, "-SNAPSHOT"));
 			} else {
 				// Sort and move nonArtifactCollector items to the response and clear nonArtifactCollector
+				final AtomicInteger sortIndex = new AtomicInteger(0);
 				nonArtifactCollector.entrySet().stream()
-				.map(entry -> entry.getValue())
-				.filter(Objects::nonNull)
-				.filter(item -> item.getSortText() != null || item.getLabel() != null)
-				.sorted(new Comparator<CompletionItem>() {
-					// Backward order
-					@Override
-					public int compare(CompletionItem o1, CompletionItem o2) {
-						String sortText1 = o1.getSortText() != null ? o1.getSortText() : o1.getLabel();
-						String sortText2 = o2.getSortText() != null ? o2.getSortText() : o2.getLabel();
-						return new DefaultArtifactVersion(sortText2)
-								.compareTo(new DefaultArtifactVersion(sortText1));
-					}
-				})
-				.forEach(response::addCompletionItem);
+					.map(entry -> entry.getValue())
+					.filter(Objects::nonNull)
+					.filter(item -> item.getSortText() != null || item.getLabel() != null)
+					.sorted(new Comparator<CompletionItem>() {
+						// Sort in reverse order to correctly fill 'sortText' 
+						@Override
+						public int compare(CompletionItem o1, CompletionItem o2) {
+							String sortText1 = o1.getSortText() != null ? o1.getSortText() : o1.getLabel();
+							String sortText2 = o2.getSortText() != null ? o2.getSortText() : o2.getLabel();
+							return new DefaultArtifactVersion(sortText2)
+									.compareTo(new DefaultArtifactVersion(sortText1));
+						}
+					})
+					.peek(item -> item.setSortText(String.format("%06d", (sortIndex.getAndIncrement())) + '.' + item.getLabel()))
+					.forEach(response::addCompletionItem);
 				nonArtifactCollector.clear();
 			}
 			break;

diff --git a/...x/extensions/maven/participants/completion/MavenCompletionParticipantDuplicationTest.java b/...x/extensions/maven/participants/completion/MavenCompletionParticipantDuplicationTest.java
@@ -28,7 +28,6 @@
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Stream;
 
-import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
 import org.eclipse.lemminx.extensions.maven.searcher.RemoteCentralRepositorySearcher;
 import org.eclipse.lemminx.extensions.maven.utils.MavenLemminxTestsUtils;
 import org.eclipse.lemminx.services.XMLLanguageService;
@@ -100,20 +99,32 @@ public void testDuplicateCompletionVersionWithRemoteRepo() throws IOException, U
 
 	@Test
 	public void testDuplicateCompletionVersionOrder() throws IOException, URISyntaxException {
-		// Check Content Assist result
+		// Check Content Assist result - accept only version items:
+		// they have sortText != null && sortText starts with 6 decimal digits followed by '.'
+		//
 		List<CompletionItem> completions = languageService.doComplete(
 					createDOMDocument("/pom-duplicate-version-completion.xml", languageService),
 					new Position(15, 13), new SharedSettings())
-				.getItems();
-
+				.getItems().stream().filter(i -> {
+					String sortText = i.getSortText();
+					if (sortText == null || sortText.indexOf('.') != 6) {
+						return false;
+					}
+					try {
+						Integer.valueOf(sortText.substring(0, 6), 10);
+					} catch (NumberFormatException | IndexOutOfBoundsException e) {
+						return false;
+					}
+					return true;
+				}).toList();
 		List<CompletionItem> orderedCompletions = completions.stream()
 				.sorted(new Comparator<CompletionItem>() {
-					// Backward order
+					// Sort in direct order using sortText/label values
 					@Override
 					public int compare(CompletionItem o1, CompletionItem o2) {
 						String sortText1 = o1.getSortText() != null ? o1.getSortText() : o1.getLabel();
 						String sortText2 = o2.getSortText() != null ? o2.getSortText() : o2.getLabel();
-						return new DefaultArtifactVersion(sortText2).compareTo(new DefaultArtifactVersion(sortText1));
+						return sortText1.compareTo(sortText2);
 					}
 				}).toList();
 		assertEquals(orderedCompletions, completions);