8177738: Runtime.Version must be a value-based class
8148822: (spec) Regex in Runtime.Version and JEP 223 should match
8160954: (spec) Runtime.Version regex and $PRE/$OPT issues
8148877: (spec) Specify when an empty '+' is required in a version string
Reviewed-by: psandoz, rriggs
Contributed-by: Iris Clark <iris.clark@oracle.com>, Pavel Rappo <pavel.rappo@oracle.com>
--- a/jdk/src/java.base/share/classes/java/lang/Runtime.java Thu Apr 20 22:46:22 2017 -0700
+++ b/jdk/src/java.base/share/classes/java/lang/Runtime.java Fri Apr 21 19:13:47 2017 +0100
@@ -930,10 +930,9 @@
}
/**
- * Returns the version of the Java Runtime Environment as a {@link
- * Runtime.Version}.
+ * Returns the version of the Java Runtime Environment as a {@link Version}.
*
- * @return the {@link Runtime.Version} of the Java Runtime Environment
+ * @return the {@link Version} of the Java Runtime Environment
*
* @since 9
*/
@@ -948,7 +947,7 @@
/**
* A representation of a version string for an implementation of the
- * Java SE Platform. A version string contains a version number
+ * Java SE Platform. A version string consists of a version number
* optionally followed by pre-release and build information.
*
* <h2><a name="verNum">Version numbers</a></h2>
@@ -960,7 +959,7 @@
* </p>
*
* <blockquote><pre>
- * ^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$
+ * [1-9][0-9]*((\.0)*\.[1-9][0-9]*)*
* </pre></blockquote>
*
* <p> The sequence may be of arbitrary length but the first three
@@ -1026,10 +1025,13 @@
*
* <p> A <em>version string</em>, {@code $VSTR}, consists of a version
* number {@code $VNUM}, as described above, optionally followed by
- * pre-release and build information, in the format </p>
+ * pre-release and build information, in one of the following formats:
+ * </p>
*
* <blockquote><pre>
- * $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)?
+ * $VNUM(-$PRE)?\+$BUILD(-$OPT)?
+ * $VNUM-$PRE(-$OPT)?
+ * $VNUM(+-$OPT)?
* </pre></blockquote>
*
* <p> where: </p>
@@ -1039,17 +1041,17 @@
* <li><p> <a name="pre">{@code $PRE}</a>, matching {@code ([a-zA-Z0-9]+)}
* --- A pre-release identifier. Typically {@code ea}, for a
* potentially unstable early-access release under active development,
- * or {@code internal}, for an internal developer build.
+ * or {@code internal}, for an internal developer build. </p></li>
*
* <li><p> <a name="build">{@code $BUILD}</a>, matching {@code
* (0|[1-9][0-9]*)} --- The build number, incremented for each promoted
* build. {@code $BUILD} is reset to {@code 1} when any portion of {@code
- * $VNUM} is incremented. </p>
+ * $VNUM} is incremented. </p></li>
*
* <li><p> <a name="opt">{@code $OPT}</a>, matching {@code
- * ([-a-zA-Z0-9\.]+)} --- Additional build information, if desired. In
+ * ([-a-zA-Z0-9.]+)} --- Additional build information, if desired. In
* the case of an {@code internal} build this will often contain the date
- * and time of the build. </p>
+ * and time of the build. </p></li>
*
* </ul>
*
@@ -1067,15 +1069,21 @@
*
* <p> A <em>short version string</em>, {@code $SVSTR}, often useful in
* less formal contexts, is a version number optionally followed by a
- * pre-release identifier:
+ * pre-release identifier:</p>
*
* <blockquote><pre>
* $VNUM(-$PRE)?
* </pre></blockquote>
*
+ * <p>This is a <a href="./doc-files/ValueBased.html">value-based</a>
+ * class; use of identity-sensitive operations (including reference equality
+ * ({@code ==}), identity hash code, or synchronization) on instances of
+ * {@code Version} may have unpredictable results and should be avoided.
+ * </p>
+ *
* @since 9
*/
- public static class Version
+ public static final class Version
implements Comparable<Version>
{
private final List<Integer> version;
@@ -1083,9 +1091,18 @@
private final Optional<Integer> build;
private final Optional<String> optional;
- Version(List<Integer> version, Optional<String> pre,
- Optional<Integer> build, Optional<String> optional) {
- this.version = Collections.unmodifiableList(version);
+ /*
+ * List of version number components passed to this constructor MUST
+ * be at least unmodifiable (ideally immutable). In the case on an
+ * unmodifiable list, the caller MUST hand the list over to this
+ * constructor and never change the underlying list.
+ */
+ private Version(List<Integer> unmodifiableListOfVersions,
+ Optional<String> pre,
+ Optional<Integer> build,
+ Optional<String> optional)
+ {
+ this.version = unmodifiableListOfVersions;
this.pre = pre;
this.build = build;
this.optional = optional;
@@ -1129,9 +1146,11 @@
+ s + "'");
// $VNUM is a dot-separated list of integers of arbitrary length
- List<Integer> version = new ArrayList<>();
- for (String i : m.group(VersionPattern.VNUM_GROUP).split("\\."))
- version.add(Integer.parseInt(i));
+ String[] split = m.group(VersionPattern.VNUM_GROUP).split("\\.");
+ Integer[] version = new Integer[split.length];
+ for (int i = 0; i < split.length; i++) {
+ version[i] = Integer.parseInt(split[i]);
+ }
Optional<String> pre = Optional.ofNullable(
m.group(VersionPattern.PRE_GROUP));
@@ -1158,7 +1177,7 @@
+ " build or optional components: '" + s + "'");
}
}
- return new Version(version, pre, build, optional);
+ return new Version(List.of(version), pre, build, optional);
}
private static boolean isSimpleNumber(String s) {
@@ -1269,9 +1288,7 @@
* During this comparison, a version with optional build information is
* considered to be greater than a version without one. </p>
*
- * <p> A version is not comparable to any other type of object.
- *
- * @param ob
+ * @param obj
* The object to be compared
*
* @return A negative integer, zero, or a positive integer if this
@@ -1282,8 +1299,8 @@
* If the given object is {@code null}
*/
@Override
- public int compareTo(Version ob) {
- return compare(ob, false);
+ public int compareTo(Version obj) {
+ return compare(obj, false);
}
/**
@@ -1294,9 +1311,10 @@
* described in {@link #compareTo(Version)} with the exception that the
* optional build information is always ignored. </p>
*
- * <p> A version is not comparable to any other type of object.
+ * <p> This method provides ordering which is consistent with
+ * {@code equalsIgnoreOptional()}. </p>
*
- * @param ob
+ * @param obj
* The object to be compared
*
* @return A negative integer, zero, or a positive integer if this
@@ -1306,47 +1324,47 @@
* @throws NullPointerException
* If the given object is {@code null}
*/
- public int compareToIgnoreOptional(Version ob) {
- return compare(ob, true);
+ public int compareToIgnoreOptional(Version obj) {
+ return compare(obj, true);
}
- private int compare(Version ob, boolean ignoreOpt) {
- if (ob == null)
- throw new NullPointerException("Invalid argument");
+ private int compare(Version obj, boolean ignoreOpt) {
+ if (obj == null)
+ throw new NullPointerException();
- int ret = compareVersion(ob);
+ int ret = compareVersion(obj);
if (ret != 0)
return ret;
- ret = comparePre(ob);
+ ret = comparePre(obj);
if (ret != 0)
return ret;
- ret = compareBuild(ob);
+ ret = compareBuild(obj);
if (ret != 0)
return ret;
if (!ignoreOpt)
- return compareOptional(ob);
+ return compareOptional(obj);
return 0;
}
- private int compareVersion(Version ob) {
+ private int compareVersion(Version obj) {
int size = version.size();
- int oSize = ob.version().size();
+ int oSize = obj.version().size();
int min = Math.min(size, oSize);
for (int i = 0; i < min; i++) {
int val = version.get(i);
- int oVal = ob.version().get(i);
+ int oVal = obj.version().get(i);
if (val != oVal)
return val - oVal;
}
return size - oSize;
}
- private int comparePre(Version ob) {
- Optional<String> oPre = ob.pre();
+ private int comparePre(Version obj) {
+ Optional<String> oPre = obj.pre();
if (!pre.isPresent()) {
if (oPre.isPresent())
return 1;
@@ -1368,8 +1386,8 @@
return 0;
}
- private int compareBuild(Version ob) {
- Optional<Integer> oBuild = ob.build();
+ private int compareBuild(Version obj) {
+ Optional<Integer> oBuild = obj.build();
if (oBuild.isPresent()) {
return (build.isPresent()
? build.get().compareTo(oBuild.get())
@@ -1380,8 +1398,8 @@
return 0;
}
- private int compareOptional(Version ob) {
- Optional<String> oOpt = ob.optional();
+ private int compareOptional(Version obj) {
+ Optional<String> oOpt = obj.optional();
if (!optional.isPresent()) {
if (oOpt.isPresent())
return -1;
@@ -1427,10 +1445,7 @@
* <p> Two {@code Version}s are equal if and only if they represent the
* same version string.
*
- * <p> This method satisfies the general contract of the {@link
- * Object#equals(Object) Object.equals} method. </p>
- *
- * @param ob
+ * @param obj
* The object to which this {@code Version} is to be compared
*
* @return {@code true} if, and only if, the given object is a {@code
@@ -1438,12 +1453,12 @@
*
*/
@Override
- public boolean equals(Object ob) {
- boolean ret = equalsIgnoreOptional(ob);
+ public boolean equals(Object obj) {
+ boolean ret = equalsIgnoreOptional(obj);
if (!ret)
return false;
- Version that = (Version)ob;
+ Version that = (Version)obj;
return (this.optional().equals(that.optional()));
}
@@ -1454,7 +1469,7 @@
* <p> Two {@code Version}s are equal if and only if they represent the
* same version string disregarding the optional build information.
*
- * @param ob
+ * @param obj
* The object to which this {@code Version} is to be compared
*
* @return {@code true} if, and only if, the given object is a {@code
@@ -1462,13 +1477,13 @@
* ignoring the optional build information
*
*/
- public boolean equalsIgnoreOptional(Object ob) {
- if (this == ob)
+ public boolean equalsIgnoreOptional(Object obj) {
+ if (this == obj)
return true;
- if (!(ob instanceof Version))
+ if (!(obj instanceof Version))
return false;
- Version that = (Version)ob;
+ Version that = (Version)obj;
return (this.version().equals(that.version())
&& this.pre().equals(that.pre())
&& this.build().equals(that.build()));
@@ -1477,9 +1492,6 @@
/**
* Returns the hash code of this version.
*
- * <p> This method satisfies the general contract of the {@link
- * Object#hashCode Object.hashCode} method.
- *
* @return The hashcode of this version
*/
@Override
@@ -1507,8 +1519,7 @@
private static final String BUILD
= "(?:(?<PLUS>\\+)(?<BUILD>0|[1-9][0-9]*)?)?";
private static final String OPT = "(?:-(?<OPT>[-a-zA-Z0-9.]+))?";
- private static final String VSTR_FORMAT
- = "^" + VNUM + PRE + BUILD + OPT + "$";
+ private static final String VSTR_FORMAT = VNUM + PRE + BUILD + OPT;
static final Pattern VSTR_PATTERN = Pattern.compile(VSTR_FORMAT);
--- a/jdk/src/java.base/share/classes/java/lang/VersionProps.java.template Thu Apr 20 22:46:22 2017 -0700
+++ b/jdk/src/java.base/share/classes/java/lang/VersionProps.java.template Fri Apr 21 19:13:47 2017 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -26,7 +26,7 @@
package java.lang;
import java.io.PrintStream;
-import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.Optional;
@@ -77,21 +77,32 @@
// This method is reflectively used by regression tests.
static List<Integer> parseVersionNumbers(String version) {
- List<Integer> verNumbers = new ArrayList<>(4);
+ // Let's find the size of an array required to hold $VNUM components
+ int size = 0;
int prevIndex = 0;
+ do {
+ prevIndex = version.indexOf('.', prevIndex) + 1;
+ size++;
+ } while (prevIndex > 0);
+ Integer[] verNumbers = new Integer[size];
+
+ // Fill in the array with $VNUM components
+ int n = 0;
+ prevIndex = 0;
int index = version.indexOf('.');
- while (index > 0) {
- verNumbers.add(parseVersionNumber(version, prevIndex, index));
+ while (index > -1) {
+ verNumbers[n] = parseVersionNumber(version, prevIndex, index);
prevIndex = index + 1; // Skip the period
index = version.indexOf('.', prevIndex);
+ n++;
}
- verNumbers.add(parseVersionNumber(version, prevIndex, version.length()));
+ verNumbers[n] = parseVersionNumber(version, prevIndex, version.length());
- if (verNumbers.get(0) == 0 || verNumbers.get(verNumbers.size() - 1) == 0)
- throw new IllegalArgumentException("Leading/trailing zeros not supported (" +
- verNumbers + ")");
+ if (verNumbers[0] == 0 || verNumbers[n] == 0)
+ throw new IllegalArgumentException("Leading/trailing zeros not allowed (" +
+ Arrays.toString(verNumbers) + ")");
- return verNumbers;
+ return List.of(verNumbers);
}
static List<Integer> versionNumbers() {
--- a/jdk/test/java/lang/Runtime/Version/Basic.java Thu Apr 20 22:46:22 2017 -0700
+++ b/jdk/test/java/lang/Runtime/Version/Basic.java Fri Apr 21 19:13:47 2017 +0100
@@ -23,19 +23,17 @@
/*
* @test
- * @summary Unit test for java.lang.Runtime.Version.
+ * @summary Unit test for java.lang.Runtime.Version
* @bug 8072379 8144062 8161236 8160956
*/
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
import java.lang.Runtime.Version;
import java.math.BigInteger;
-import java.util.stream.Collectors;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
+import java.util.stream.Collectors;
import static java.lang.System.out;
@@ -46,7 +44,6 @@
= NullPointerException.class;
private static final Class<? extends Throwable> NFE
= NumberFormatException.class;
- private static final Class<?> VERSION = Version.class;
private static final BigInteger TOO_BIG
= (BigInteger.valueOf(Integer.MAX_VALUE)).add(BigInteger.ONE);
@@ -232,7 +229,7 @@
String [] ver = jv[0].split("-");
List<Integer> javaVerVNum
= Arrays.stream(ver[0].split("\\."))
- .map(v -> Integer.parseInt(v))
+ .map(Integer::parseInt)
.collect(Collectors.toList());
if (!javaVerVNum.equals(current.version())) {
fail("Runtime.version()", javaVerVNum.toString(),
@@ -256,7 +253,7 @@
}
private static void testVersion(List<Integer> vnum, String s) {
- List<Integer> svnum = new ArrayList<Integer>();
+ List<Integer> svnum = new ArrayList<>();
StringBuilder sb = new StringBuilder();
for (int i = 0; i < s.length(); i++) {
Character c = s.charAt(i);
@@ -297,21 +294,20 @@
}
private static void testEqualsNO(Version v0, Version v1, boolean eq) {
- if ((eq && !v0.equalsIgnoreOptional(v1))
- || (!eq && v0.equalsIgnoreOptional(v1))) {
+ if (eq == v0.equalsIgnoreOptional(v1)) {
+ pass();
+ } else {
fail("equalsIgnoreOptional() " + Boolean.toString(eq),
v0.toString(), v1.toString());
- } else {
- pass();
}
}
private static void testEquals(Version v0, Version v1, boolean eq) {
- if ((eq && !v0.equals(v1)) || (!eq && v0.equals(v1))) {
+ if (eq == v0.equals(v1)) {
+ pass();
+ } else {
fail("equals() " + Boolean.toString(eq),
v0.toString(), v1.toString());
- } else {
- pass();
}
}
@@ -329,41 +325,24 @@
}
}
- private static void testCompareNO(Version v0, Version v1, int compare)
- {
- try {
- Method m = VERSION.getMethod("compareToIgnoreOptional", VERSION);
- int cmp = (int) m.invoke(v0, v1);
- checkCompare(v0, v1, compare, cmp);
- } catch (IllegalAccessException | InvocationTargetException |
- NoSuchMethodException ex) {
- fail(String.format("compareToIgnoreOptional() invocation: %s",
- ex.getClass()),
- null);
- }
+ private static void testCompareNO(Version v0, Version v1, int compare) {
+ int cmp = v0.compareToIgnoreOptional(v1);
+ checkCompare(v0, v1, compare, cmp);
}
private static void testCompare(Version v0, Version v1, int compare) {
- try {
- Method m = VERSION.getMethod("compareTo", VERSION);
- int cmp = (int) m.invoke(v0, v1);
- checkCompare(v0, v1, compare, cmp);
- } catch (IllegalAccessException | InvocationTargetException |
- NoSuchMethodException ex) {
- fail(String.format("compareTo() invocation: %s", ex.getClass()),
- null);
- }
+ int cmp = v0.compareTo(v1);
+ checkCompare(v0, v1, compare, cmp);
}
private static void checkCompare(Version v0, Version v1,
- int compare, int cmp)
+ int expected, int actual)
{
- if (((cmp == 0) && (compare == 0))
- || (compare == (cmp / Math.abs(cmp == 0 ? 1 : cmp)))) {
+ if (Integer.signum(expected) == Integer.signum(actual)) {
pass();
} else {
- fail(String.format("compare() (cmp = %s) (compare = %s)",
- cmp, compare),
+ fail(String.format("compare() (actual = %s) (expected = %s)",
+ actual, expected),
v0.toString(), v1.toString());
}
}