Skip to content

Commit

Permalink
Improve canonicalization of Maven version strings; skip known bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
dmlloyd committed Feb 17, 2023
1 parent 570690d commit 5319c99
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ public boolean isNonEmptySeparator() {
return super.isEmptySeparator() || super.isNonEmptySeparator();
}

public boolean isNumberPart() {
return super.isNumberPart() || isReleaseString();
}

public boolean isAlphaPart() {
return super.isAlphaPart() && !isReleaseString();
}

public int getSeparatorCodePoint() {
return super.isEmptySeparator() ? '-' : super.getSeparatorCodePoint();
}
Expand All @@ -27,6 +35,14 @@ public StringBuilder appendPartTo(final StringBuilder target) {
}
}

public StringBuilder appendNumberPartTo(final StringBuilder target) throws IllegalStateException {
if (isReleaseString()) {
return target.append('0');
} else {
return super.appendNumberPartTo(target);
}
}

public StringBuilder appendAlphaPartTo(final StringBuilder b) throws IllegalStateException {
int m = getMilestoneMagnitude();
if (m != -1) {
Expand Down Expand Up @@ -56,7 +72,7 @@ public boolean hasNext() {
if (t == TokenType.PART_ALPHA && !isReleaseString()) {
return true;
}
if (t == TokenType.PART_NUMBER && !numberPartEquals(0)) {
if (t == TokenType.PART_NUMBER && !super.numberPartEquals(0)) {
return true;
}
// otherwise it depends on the next segment
Expand Down Expand Up @@ -112,7 +128,7 @@ boolean isReleaseString() {
}

boolean isZeroSegment() {
return isNumberPart() && numberPartEquals(0) || isAlphaPart() && isReleaseString();
return super.isNumberPart() && super.numberPartEquals(0) || super.isAlphaPart() && isReleaseString();
}

boolean isDashSeparator() {
Expand Down Expand Up @@ -146,7 +162,7 @@ void skipTrailer(long mark) {
// Magnitude (pop pop)

int getMilestoneMagnitude() {
if (isAlphaPart()) {
if (super.isAlphaPart()) {
final boolean nextSeparatorIsEmpty = nextSeparatorIsEmpty();

if (alphaPartEquals("alpha", true) || alphaPartEquals("a", true) && nextSeparatorIsEmpty) {
Expand All @@ -166,6 +182,8 @@ int getMilestoneMagnitude() {
} else if (alphaPartEquals("sp", true)) {
return 6;
}
} else if (isZeroSegment()) {
return 5;
}
return -1; // not a milestone
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ public void testMavenCompare() {
checkMavenConsistency("x", "x.0");
checkMavenConsistency("x", "x.x");
checkMavenConsistency("0.1", "0-1");
checkMavenConsistency("0.x", "x");
checkMavenConsistency("0.x", "0-x");
// https://issues.apache.org/jira/browse/MNG-7701
// checkMavenConsistency("0.x", "x");
// checkMavenConsistency("0.x", "0-x");
checkMavenConsistency("1.1", "1-1");
checkMavenConsistency("1.x", "1-x");
// https://issues.apache.org/jira/browse/MNG-7701
// checkMavenConsistency("1.x", "1-x");
checkMavenConsistency("0.1", "1-1");
checkMavenConsistency("0.x", "1-x");
checkMavenConsistency("1.1", "0-1");
Expand All @@ -26,18 +28,19 @@ public void testMavenCompare() {
checkMavenConsistency("1.1", "1_0");
checkMavenConsistency("1.1", "1_1");
checkMavenConsistency("1.x", "1.y");
checkMavenConsistency("1.x", "1_y");
checkMavenConsistency("1.y", "1_x");
checkMavenConsistency("1.y", "1_x");
checkMavenConsistency("1.y+z", "1_x");
checkMavenConsistency("1.y+0", "1_x+0");
// https://issues.apache.org/jira/browse/MNG-7701
// checkMavenConsistency("1.x", "1_y");
// checkMavenConsistency("1.y", "1_x");
// checkMavenConsistency("1.y+z", "1_x");
// checkMavenConsistency("1.y+0", "1_x+0");
checkMavenConsistency("0.1-0-123.xyz-alpha", "0.1");
checkMavenConsistency("1.0.0-0-final-0", "1");
checkMavenConsistency("1234", "abcd");
checkMavenConsistency("1.0.a", "1.0.A");
checkMavenConsistency("1.0.ga", "1.0.final");
checkMavenConsistency("1-23", "1.23");
checkMavenConsistency("1-alpha", "1.alpha");
// https://issues.apache.org/jira/browse/MNG-7701
// checkMavenConsistency("1-alpha", "1.alpha");
checkMavenConsistency("1-0.beta", "1-0.alpha");
checkMavenConsistency("12-foo", "foo-12");
checkMavenConsistency("12.foo", "foo.12");
Expand All @@ -55,14 +58,27 @@ public void testMavenCompare() {

private static void checkMavenConsistency(String v1, String v2) {
// Maven's comparator may return numbers outside the set of (-1, 0, 1)
assertEquals(Integer.signum(new ComparableVersion(v1).compareTo(new ComparableVersion(v2))),
VersionScheme.MAVEN.compare(v1, v2));
assertEquals(Integer.signum(new ComparableVersion(v2).compareTo(new ComparableVersion(v1))),
VersionScheme.MAVEN.compare(v2, v1));
int v1v2 = Integer.signum(new ComparableVersion(v1).compareTo(new ComparableVersion(v2)));
int v2v1 = Integer.signum(new ComparableVersion(v2).compareTo(new ComparableVersion(v1)));
if (v1v2 != -v2v1) {
System.out.printf(
"Skipping check for \"%s\" <-> \"%s\" due to internal inconsistency in the version of maven-artifact in use%n",
v1, v2);
} else {
assertEquals(v1v2, VersionScheme.MAVEN.compare(v1, v2));
assertEquals(v2v1, VersionScheme.MAVEN.compare(v2, v1));
}
}

@Test
public void testMavenCanonicalize() {
checkMavenCanonical("1.0.1");
checkMavenCanonical("0.1.0");
checkMavenCanonical("0.0.1");
checkMavenCanonical("0.1");
checkMavenCanonical("0-1");
checkMavenCanonical("0.x");
checkMavenCanonical("0-x");
checkMavenCanonical("1");
checkMavenCanonical("1.0");
checkMavenCanonical("1-0");
Expand All @@ -76,6 +92,15 @@ public void testMavenCanonicalize() {
}

private static void checkMavenCanonical(String str) {
assertEquals(new ComparableVersion(str).getCanonical(), VersionScheme.MAVEN.canonicalize(str));
ComparableVersion parsed = new ComparableVersion(str);
ComparableVersion canon = new ComparableVersion(parsed.getCanonical());
if (parsed.equals(canon) && parsed.compareTo(canon) == 0 && canon.compareTo(parsed) == 0) {
assertEquals(parsed.getCanonical(), VersionScheme.MAVEN.canonicalize(str));
} else {
// https://issues.apache.org/jira/browse/MNG-7700
System.out.printf(
"Skipping check for \"%s\" due to internal inconsistency in the version of maven-artifact in use; our output is \"%s\"%n",
str, VersionScheme.MAVEN.canonicalize(str));
}
}
}

0 comments on commit 5319c99

Please sign in to comment.