[11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

[Langer, Christoph]

Hi Matthias,

thanks for your careful review of this tedious change.

I think the change in jdk.internal.module.Checks.java is ok to backport. It is no public interface and regression tests show no issue. So I wouldn't modify the patch in this place.

Any other opinions?

Best regards
Christoph

From: Baesken, Matthias
Sent: Dienstag, 18. Juni 2019 15:16
To: Langer, Christoph <christoph.langer at sap.com>; jdk-updates-dev at openjdk.java.net
Cc: Java Core Libs <core-libs-dev at openjdk.java.net>
Subject: RE: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

Hello Christoph,   looks good   with  the exception of

src/java.base/share/classes/jdk/internal/module/Checks.java

http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/src/java.base/share/classes/jdk/internal/module/Checks.java.frames.html


     /**
-     * Returns {@code true} if the given name is a legal module name.
-     */
-    public static boolean isModuleName(String name) {

   ....

-    private static boolean isJavaIdentifier(CharSequence cs) {
-        if (cs.length() == 0 || RESERVED.contains(cs))
+    private static boolean isJavaIdentifier(String str) {
+        if (str.isEmpty() || RESERVED.contains(str))


 ... where I am a bit  unsure - should we really  remove a method in jdk11  (and change signature of another) ?  I know ,  it is  from  jdk/internal   , but still  I have a bad feeling about this .
This was probably fine for  the higher release  but for jdk11  it might be better   to keep what we have .

Best regards, Matthias


From: Langer, Christoph
Sent: Dienstag, 18. Juni 2019 10:59
To: jdk-updates-dev at openjdk.java.net<mailto:jdk-updates-dev at openjdk.java.net>
Cc: Java Core Libs <core-libs-dev at openjdk.java.net<mailto:core-libs-dev at openjdk.java.net>>; Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>
Subject: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

Hi,

please review the backport of the String.isEmpty() cleanups in java.base.

The main reason to bring this down to JDK11u is that it'll ease backporting of later changes which otherwise run into conflicts and won't resolve. Furthermore, the original change is a nice cleanup and improves performance.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215281
Change in jdk/jdk: http://hg.openjdk.java.net/jdk/jdk/rev/8bf9268df0e2

The original patch merely applies (with a little fuzz in some places). There were 4 rejects:

src/java.base/share/classes/java/lang/VersionProps.java.template
--- VersionProps.java.template
+++ VersionProps.java.template
@@ -95,7 +95,7 @@
         props.put("java.version.date", java_version_date);
         props.put("java.runtime.version", java_runtime_version);
         props.put("java.runtime.name", java_runtime_name);
-        if (VENDOR_VERSION_STRING.length() > 0)
+        if (!VENDOR_VERSION_STRING.isEmpty())
             props.put("java.vendor.version", VENDOR_VERSION_STRING);

         props.put("java.class.version", CLASSFILE_MAJOR_MINOR);

-> manually resolved that to the right location

src/java.base/share/classes/java/text/CompactNumberFormat.java

-> file does not exist in 11u, so dropping

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckMethodAdapter.java
--- CheckMethodAdapter.java
+++ CheckMethodAdapter.java
@@ -1314,7 +1314,7 @@
       * @param message the message to use in case of error.
       */
     static void checkMethodIdentifier(final int version, final String name, final String message) {
-        if (name == null || name.length() == 0) {
+        if (name == null || name.isEmpty()) {
             throw new IllegalArgumentException(INVALID + message + MUST_NOT_BE_NULL_OR_EMPTY);
         }
         if ((version & 0xFFFF) >= Opcodes.V1_5) {
@@ -1347,7 +1347,7 @@
       * @param message the message to use in case of error.
       */
     static void checkInternalName(final int version, final String name, final String message) {
-        if (name == null || name.length() == 0) {
+        if (name == null || name.isEmpty()) {
             throw new IllegalArgumentException(INVALID + message + MUST_NOT_BE_NULL_OR_EMPTY);
         }
         if (name.charAt(0) == '[') {
@@ -1457,7 +1457,7 @@
       * @param descriptor the string to be checked.
       */
     static void checkMethodDescriptor(final int version, final String descriptor) {
-        if (descriptor == null || descriptor.length() == 0) {
+        if (descriptor == null || descriptor.isEmpty()) {
             throw new IllegalArgumentException("Invalid method descriptor (must not be null or empty)");
         }
         if (descriptor.charAt(0) != '(' || descriptor.length() < 3) {

-> file looks different in 11u due to a missing change. So, modified the right places in the 11u version

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckSignatureAdapter.java
--- CheckSignatureAdapter.java
+++ CheckSignatureAdapter.java
@@ -365,7 +365,7 @@
     }

     private void checkClassName(final String name, final String message) {
-        if (name == null || name.length() == 0) {
+        if (name == null || name.isEmpty()) {
             throw new IllegalArgumentException(INVALID + message + " (must not be null or empty)");
         }
         for (int i = 0; i < name.length(); ++i) {
@@ -377,7 +377,7 @@
     }

     private void checkIdentifier(final String name, final String message) {
-        if (name == null || name.length() == 0) {
+        if (name == null || name.isEmpty()) {
             throw new IllegalArgumentException(INVALID + message + " (must not be null or empty)");
         }
         for (int i = 0; i < name.length(); ++i) {

-> file looks different in 11u due to a missing change. So, modified the right places in the 11u version

Thanks
Christoph