Cleanup Version implementation

Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>

Author: BoykoAlex

headless-services/commons/commons-lsp-extensions/src/main/java/org/springframework/ide/vscode/commons/Version.java

@@ -33,7 +33,6 @@ public final class Version implements Comparable<Version> {
 	// Groups: 1=major, 2=minor, 3=patch, 4=build (4th numeric part), 5=fifth part, 6=qualifier suffix
 	private static final Pattern RELEASE_PATTERN = Pattern.compile(
 			"(\\d+)(?:\\.(\\d+))?(?:\\.(\\d+))?(?:\\.(\\d+))?(?:\\.(\\d+))?([-.+].*)?");
-	private static final String[] RELEASE_SUFFIXES = { ".final", ".ga", ".release" };
 	private static final int QUALIFIER_GROUP = 6;
 
 	private static final Pattern QUALIFIER_TYPE_PATTERN =
@@ -81,7 +80,9 @@ public boolean isSnapshot() {
 	/** Qualifier text without its leading separator (e.g. {@code "M1"}, {@code "SNAPSHOT"}), or {@code null}. */
 	private final String qualifier;
 	private final ReleaseType releaseType;
-	/** Original parsed string — used for display and comparison. */
+	/** Numeric suffix of the qualifier (e.g. {@code 1} for {@code M1} or {@code RC2→2}); 0 if absent. */
+	private final int qualifierNumber;
+	/** Original parsed string — used for display. */
 	private final String versionString;
 
 	private Version(int major, int minor, int patch, int build, String qualifier, String originalString) {
@@ -91,6 +92,7 @@ private Version(int major, int minor, int patch, int build, String qualifier, St
 		this.build = build;
 		this.qualifier = qualifier;
 		this.releaseType = toReleaseType(qualifier);
+		this.qualifierNumber = toQualifierNumber(qualifier);
 		this.versionString = originalString;
 	}
 
@@ -123,6 +125,11 @@ public ReleaseType getReleaseType() {
 		return releaseType;
 	}
 
+	/** Numeric suffix of the qualifier; 0 when absent (e.g. {@code SNAPSHOT→0}, {@code M1→1}, {@code RC2→2}). */
+	public int getQualifierNumber() {
+		return qualifierNumber;
+	}
+
 	/** True for GA releases and service packs. */
 	public boolean isRelease() {
 		return releaseType == ReleaseType.RELEASE || releaseType == ReleaseType.SERVICE_PACK;
@@ -160,7 +167,13 @@ public String toString() {
 	 */
 	@Override
 	public int compareTo(Version o) {
-		return compareVersionStrings(this.versionString, o.versionString);
+		int d;
+		if ((d = Integer.compare(major, o.major)) != 0) return d;
+		if ((d = Integer.compare(minor, o.minor)) != 0) return d;
+		if ((d = Integer.compare(patch, o.patch)) != 0) return d;
+		if ((d = Integer.compare(build, o.build)) != 0) return d;
+		if ((d = Integer.compare(releaseType.getPriority(), o.releaseType.getPriority())) != 0) return d;
+		return Integer.compare(qualifierNumber, o.qualifierNumber);
 	}
 
 	@Override
@@ -214,120 +227,6 @@ public static Version parse(String versionStr) {
 		}
 	}
 
-	// ---- comparison logic (derived from org.openrewrite.semver.LatestRelease, Apache 2.0) ----
-
-	private static int compareVersionStrings(String v1, String v2) {
-		if (v1.equalsIgnoreCase(v2)) {
-			return 0;
-		}
-
-		String nv1 = normalizeVersion(v1);
-		String nv2 = normalizeVersion(v2);
-
-		int vp1 = countVersionParts(nv1);
-		int vp2 = countVersionParts(nv2);
-
-		// Pad the shorter version with ".0" segments so both have the same number of parts
-		if (vp1 > vp2) {
-			StringBuilder sb = new StringBuilder(nv2);
-			for (int i = vp2; i < vp1; i++) sb.append(".0");
-			nv2 = sb.toString();
-		} else if (vp2 > vp1) {
-			StringBuilder sb = new StringBuilder(nv1);
-			for (int i = vp1; i < vp2; i++) sb.append(".0");
-			nv1 = sb.toString();
-		}
-
-		Matcher m1 = RELEASE_PATTERN.matcher(nv1);
-		Matcher m2 = RELEASE_PATTERN.matcher(nv2);
-		m1.find();
-		m2.find();
-
-		int maxParts = Math.max(vp1, vp2);
-		for (int i = 1; i <= maxParts; i++) {
-			String p1 = m1.group(i);
-			String p2 = m2.group(i);
-			if (p1 == null) {
-				return p2 == null ? nv1.compareTo(nv2) : -1;
-			} else if (p2 == null) {
-				return 1;
-			}
-			long diff = Long.parseLong(p1) - Long.parseLong(p2);
-			if (diff != 0) {
-				return diff > 0 ? 1 : -1;
-			}
-		}
-
-		// All numeric parts equal — compare by qualifier priority
-		int prio1 = qualifierPriority(m1.group(QUALIFIER_GROUP));
-		int prio2 = qualifierPriority(m2.group(QUALIFIER_GROUP));
-		if (prio1 != prio2) {
-			return Integer.compare(prio1, prio2);
-		}
-		return nv1.compareTo(nv2);
-	}
-
-	/** Strips trailing {@code .final} / {@code .ga} / {@code .release} and pads to at least 3 parts. */
-	private static String normalizeVersion(String version) {
-		int lastDotIdx = version.lastIndexOf('.');
-		for (String suffix : RELEASE_SUFFIXES) {
-			if (version.regionMatches(true, lastDotIdx, suffix, 0, suffix.length())) {
-				version = version.substring(0, lastDotIdx);
-				break;
-			}
-		}
-		int parts = countVersionParts(version);
-		if (parts <= 2) {
-			String[] split = version.split("(?=[-+])");
-			for (; parts <= 2; parts++) {
-				split[0] += ".0";
-			}
-			version = split.length > 1 ? split[0] + split[1] : split[0];
-		}
-		return version;
-	}
-
-	private static int countVersionParts(String version) {
-		int count = 0;
-		int len = version.length();
-		int lastSepIdx = -1;
-		for (int i = 0; i < len; i++) {
-			char c = version.charAt(i);
-			if (c == '.' || c == '-' || c == '$') {
-				if (lastSepIdx == i - 1) return count;
-				lastSepIdx = i;
-			} else if (lastSepIdx == i - 1) {
-				if (!Character.isDigit(c)) break;
-				count++;
-			}
-		}
-		return count;
-	}
-
-	private static int qualifierPriority(String suffix) {
-		switch (extractQualifier(suffix)) {
-			case "alpha": case "a":                        return 1;
-			case "beta":  case "b":                        return 2;
-			case "milestone": case "m":                    return 3;
-			case "rc":    case "cr":                       return 4;
-			case "snapshot":                               return 5;
-			case "": case "ga": case "final": case "release": return 6;
-			case "sp":                                     return 7;
-			default:                                       return 8;
-		}
-	}
-
-	private static String extractQualifier(String suffix) {
-		if (suffix == null) return "";
-		StringBuilder sb = new StringBuilder();
-		for (int i = 1; i < suffix.length(); i++) {
-			char c = suffix.charAt(i);
-			if (Character.isLetter(c)) sb.append(Character.toLowerCase(c));
-			else break;
-		}
-		return sb.toString();
-	}
-
 	// ---- parsing helpers ----
 
 	private static int parseGroup(Matcher m, int group) {
@@ -347,6 +246,18 @@ private static String stripSeparator(String qualifierWithSep) {
 		return qualifierWithSep;
 	}
 
+	private static int toQualifierNumber(String qualifier) {
+		if (qualifier == null || qualifier.isEmpty()) {
+			return 0;
+		}
+		Matcher m = QUALIFIER_TYPE_PATTERN.matcher(qualifier);
+		if (m.matches()) {
+			String num = m.group(2);
+			return num.isEmpty() ? 0 : Integer.parseInt(num);
+		}
+		return 0;
+	}
+
 	private static ReleaseType toReleaseType(String qualifier) {
 		if (qualifier == null || qualifier.isEmpty()) {
 			return ReleaseType.RELEASE;
@@ -355,6 +266,9 @@ private static ReleaseType toReleaseType(String qualifier) {
 		if (lower.equals("release") || lower.equals("ga") || lower.equals("final")) {
 			return ReleaseType.RELEASE;
 		}
+		if (lower.startsWith("snapshot")) {
+			return ReleaseType.SNAPSHOT;
+		}
 		if (lower.startsWith("sp")) {
 			return ReleaseType.SERVICE_PACK;
 		}

headless-services/commons/commons-lsp-extensions/src/test/java/org/springframework/ide/vscode/commons/VersionTests.java

@@ -291,16 +291,13 @@ void toMajorMinorPatchVersionStr() {
 
 	@Test
 	void sortAllThreePartVersionVariants() {
-		// Covers every qualifier type, both short aliases (A/B/CR) and full names,
-		// numeric progression within a qualifier (M1<M2, RC1<RC2, SP1<SP2).
+		// Covers every qualifier type and numeric progression (M1<M2, RC1<RC2, SP1<SP2).
+		// Aliases (A/ALPHA, B/BETA, CR/RC) compare equal and are tested in aliasesCompareEqual.
 		assertSortedOrder(
-			"3.3.0-A1",        // alpha (short alias)
 			"3.3.0-ALPHA1",    // alpha
-			"3.3.0-B1",        // beta (short alias)
 			"3.3.0-BETA1",     // beta
 			"3.3.0-M1",        // milestone
 			"3.3.0-M2",        // milestone, higher qualifier number
-			"3.3.0-CR1",       // rc (alias; CR < RC lexicographically)
 			"3.3.0-RC1",       // rc
 			"3.3.0-RC2",       // rc, higher qualifier number
 			"3.3.0-SNAPSHOT",  // snapshot
@@ -314,13 +311,10 @@ void sortAllThreePartVersionVariants() {
 	void sortAllFourPartVersionVariants() {
 		// Same qualifier coverage as the 3-part test but with a 4th numeric component.
 		assertSortedOrder(
-			"3.3.0.1-A1",
 			"3.3.0.1-ALPHA1",
-			"3.3.0.1-B1",
 			"3.3.0.1-BETA1",
 			"3.3.0.1-M1",
 			"3.3.0.1-M2",
-			"3.3.0.1-CR1",
 			"3.3.0.1-RC1",
 			"3.3.0.1-RC2",
 			"3.3.0.1-SNAPSHOT",
@@ -333,29 +327,23 @@ void sortAllFourPartVersionVariants() {
 	@Test
 	void sortThreeAndFourPartVersionsMixed() {
 		// All 3-part and 4-part variants interleaved in one list.
-		// The critical boundary is 3.3.0-SP2 < 3.3.0.1-A1: the 4th numeric
+		// The critical boundary is 3.3.0-SP2 < 3.3.0.1-ALPHA1: the 4th numeric
 		// component (build=1 vs 0) dominates even the highest 3-part qualifier.
 		assertSortedOrder(
-			"3.3.0-A1",
 			"3.3.0-ALPHA1",
-			"3.3.0-B1",
 			"3.3.0-BETA1",
 			"3.3.0-M1",
 			"3.3.0-M2",
-			"3.3.0-CR1",
 			"3.3.0-RC1",
 			"3.3.0-RC2",
 			"3.3.0-SNAPSHOT",
 			"3.3.0",
 			"3.3.0-SP1",
 			"3.3.0-SP2",
-			"3.3.0.1-A1",
 			"3.3.0.1-ALPHA1",
-			"3.3.0.1-B1",
 			"3.3.0.1-BETA1",
 			"3.3.0.1-M1",
 			"3.3.0.1-M2",
-			"3.3.0.1-CR1",
 			"3.3.0.1-RC1",
 			"3.3.0.1-RC2",
 			"3.3.0.1-SNAPSHOT",
@@ -382,6 +370,17 @@ void numericComponentsDominateQualifier() {
 		);
 	}
 
+	@Test
+	void timestampedSnapshotTreatedAsSnapshot() {
+		// Maven timestamped snapshots (e.g. from a remote repo) must be < the GA release.
+		Version tsSnap = Version.parse("4.0.2-SNAPSHOT-20250101.123456-1");
+		Version snap   = Version.parse("4.0.2-SNAPSHOT");
+		Version fin    = Version.parse("4.0.2.Final");
+		assertEquals(ReleaseType.SNAPSHOT, tsSnap.getReleaseType());
+		assertTrue(tsSnap.compareTo(fin) < 0,  "timestamped snapshot < Final");
+		assertEquals(0, tsSnap.compareTo(snap), "timestamped snapshot == plain snapshot");
+	}
+
 	@Test
 	void gaAliasesCompareEqualToPlainRelease() {
 		Version plain = Version.parse("3.3.0");
@@ -397,15 +396,22 @@ void dotSeparatorQualifiersRespectPriority() {
 		// Old-style dot-separated qualifiers observe the same priority order.
 		assertTrue(Version.parse("3.3.0.M1").compareTo(Version.parse("3.3.0.RC1")) < 0);
 		assertTrue(Version.parse("3.3.0.RC1").compareTo(Version.parse("3.3.0.SNAPSHOT")) < 0);
-		// Dot separator ('.' = 46) > hyphen ('-' = 45), so when qualifier priority
-		// ties the fallback string comparison puts dot-style after hyphen-style:
-		// "3.3.0.M1" > "3.3.0-M2" even though M2 > M1 numerically.
-		assertTrue(Version.parse("3.3.0-M2").compareTo(Version.parse("3.3.0.M1")) < 0);
+		// Separator style is irrelevant — only the qualifier number matters,
+		// so "3.3.0-M2" (number=2) > "3.3.0.M1" (number=1).
+		assertTrue(Version.parse("3.3.0-M2").compareTo(Version.parse("3.3.0.M1")) > 0);
+	}
+
+	@Test
+	void aliasesCompareEqual() {
+		// Short and long qualifier aliases have the same ReleaseType and number, so they compare equal.
+		assertEquals(0, Version.parse("3.3.0-A1").compareTo(Version.parse("3.3.0-ALPHA1")));
+		assertEquals(0, Version.parse("3.3.0-B1").compareTo(Version.parse("3.3.0-BETA1")));
+		assertEquals(0, Version.parse("3.3.0-CR1").compareTo(Version.parse("3.3.0-RC1")));
 	}
 
 	@Test
 	void caseInsensitiveVersionStringsCompareEqual() {
-		// Identical strings differing only in case short-circuit to 0 without parsing.
+		// Qualifier parsing is case-insensitive, so mixed-case versions compare equal.
 		assertEquals(0, Version.parse("3.3.0-SNAPSHOT").compareTo(Version.parse("3.3.0-snapshot")));
 		assertEquals(0, Version.parse("3.3.0-RC1").compareTo(Version.parse("3.3.0-rc1")));
 		assertEquals(0, Version.parse("3.3.0-M1").compareTo(Version.parse("3.3.0-m1")));