java.lang.reflect: Use only relevant modifiers when converting to string

Some modifiers not relevant might be used to encode internal information
(eg bridge methods) that shouldn't be displayed when converting to string

bug: 18488857

Change-Id: Ie82ed513b58083a795549a708197f1db52ffb796

6 files changed, 102 insertions, 6 deletions

diff --git a/libart/src/main/java/java/lang/reflect/Field.java b/libart/src/main/java/java/lang/reflect/Field.java
index 11e8a6e..cad876b 100644
--- a/libart/src/main/java/java/lang/reflect/Field.java
+++ b/libart/src/main/java/java/lang/reflect/Field.java
@@ -182,10 +182,12 @@ public final class Field extends AccessibleObject implements Member {
      */
     public String toGenericString() {
         StringBuilder sb = new StringBuilder(80);
+        // Limit modifier bits to the ones that toStringGeneric should return for fields.
+
+        String modifiers = Modifier.getDeclarationFieldModifiers(getModifiers());
         // append modifiers if any
-        int modifier = getModifiers();
-        if (modifier != 0) {
-            sb.append(Modifier.toString(modifier)).append(' ');
+        if (!modifiers.isEmpty()) {
+            sb.append(modifiers).append(' ');
         }
         // append generic type
         Types.appendGenericType(sb, getGenericType());
@@ -861,7 +863,8 @@ public final class Field extends AccessibleObject implements Member {
      */
     @Override
     public String toString() {
-        StringBuilder result = new StringBuilder(Modifier.toString(getModifiers()));
+        StringBuilder result = new StringBuilder(
+                Modifier.getDeclarationFieldModifiers(getModifiers()));
         if (result.length() != 0) {
             result.append(' ');
         }
diff --git a/libart/src/main/java/java/lang/reflect/Method.java b/libart/src/main/java/java/lang/reflect/Method.java
index 058fb96..f0e4f5c 100644
--- a/libart/src/main/java/java/lang/reflect/Method.java
+++ b/libart/src/main/java/java/lang/reflect/Method.java
@@ -398,7 +398,8 @@ public final class Method extends AbstractMethod implements GenericDeclaration,
      */
     @Override
     public String toString() {
-        StringBuilder result = new StringBuilder(Modifier.toString(getModifiers()));
+        StringBuilder result = new StringBuilder(
+                Modifier.getDeclarationMethodModifiers(getModifiers()));
 
         if (result.length() != 0) {
             result.append(' ');
diff --git a/luni/src/main/java/java/lang/reflect/Modifier.java b/luni/src/main/java/java/lang/reflect/Modifier.java
index 257064e..0480b8b 100644
--- a/luni/src/main/java/java/lang/reflect/Modifier.java
+++ b/luni/src/main/java/java/lang/reflect/Modifier.java
@@ -302,4 +302,23 @@ public class Modifier {
         buf.setLength(buf.length() - 1);
         return buf.toString();
     }
+
+    /**
+     * Returns the modifiers for fields that can be present in a declaration.
+     * @hide
+     */
+    static String getDeclarationFieldModifiers(int modifiers) {
+        return Modifier.toString(modifiers & fieldModifiers());
+    }
+
+    /**
+     * Returns the modifiers for methods that can be present in a declaration.
+     * @hide
+     */
+    static String getDeclarationMethodModifiers(int modifiers) {
+        return Modifier.toString(modifiers & (
+                Modifier.isConstructor(modifiers)
+                        ? Modifier.constructorModifiers()
+                        : Modifier.methodModifiers()));
+    }
 }
diff --git a/luni/src/test/java/libcore/java/lang/reflect/FieldTest.java b/luni/src/test/java/libcore/java/lang/reflect/FieldTest.java
index b60d984..75665db 100644
--- a/luni/src/test/java/libcore/java/lang/reflect/FieldTest.java
+++ b/luni/src/test/java/libcore/java/lang/reflect/FieldTest.java
@@ -17,6 +17,8 @@
 package libcore.java.lang.reflect;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
 import junit.framework.TestCase;
 
 public final class FieldTest extends TestCase {
@@ -46,6 +48,56 @@ public final class FieldTest extends TestCase {
         assertFalse(f1.equals(f2));
     }
 
+    // Tests that the "synthetic" modifier is handled correctly.
+    // It's supposed to be present but not shown in toString.
+    public void testSyntheticModifier() throws NoSuchFieldException {
+        Field valuesField = Thread.State.class.getDeclaredField("$VALUES");
+        // Check that this test makes sense.
+        assertTrue(valuesField.isSynthetic());
+        assertEquals(Modifier.SYNTHETIC, valuesField.getModifiers() & Modifier.SYNTHETIC);
+        assertEquals("private static final java.lang.Thread$State[] java.lang.Thread$State.$VALUES",
+                valuesField.toString());
+    }
+
+    // Ensure that the "enum constant" bit is not returned in toString.
+    public void testEnumValueField() throws NoSuchFieldException {
+        Field blockedField = Thread.State.class.getDeclaredField("BLOCKED");
+        assertTrue(Thread.State.class.getDeclaredField("BLOCKED").isEnumConstant());
+        assertEquals("public static final", Modifier.toString(blockedField.getModifiers()));
+        assertEquals(
+                "public static final java.lang.Thread$State java.lang.Thread$State.BLOCKED",
+                blockedField.toString());
+    }
+
+    class ClassWithATransientField {
+        private transient Class<String> transientField = String.class;
+    }
+
+    // Tests that the "transient" modifier is handled correctly.
+    // The underlying constant value for it is the same as for the "varargs" method modifier.
+    // http://b/18488857
+    public void testTransientModifier() throws NoSuchFieldException {
+        Field transientField = ClassWithATransientField.class.getDeclaredField("transientField");
+        // Check that this test makes sense.
+        assertEquals(Modifier.TRANSIENT, transientField.getModifiers() & Modifier.TRANSIENT);
+        assertEquals(
+                "private transient java.lang.Class "
+                        + "libcore.java.lang.reflect.FieldTest$ClassWithATransientField"
+                        + ".transientField",
+                transientField.toString());
+    }
+
+    public void testToGenericString() throws NoSuchFieldException {
+        Field transientField = ClassWithATransientField.class.getDeclaredField("transientField");
+        // Check that this test makes sense.
+        assertEquals(Modifier.TRANSIENT, transientField.getModifiers() & Modifier.TRANSIENT);
+        assertEquals(
+                "private transient java.lang.Class<java.lang.String> "
+                        + "libcore.java.lang.reflect.FieldTest$ClassWithATransientField"
+                        + ".transientField",
+                transientField.toGenericString());
+    }
+
     static class FieldTestHelper {
         public String a;
         public Object b;
diff --git a/luni/src/test/java/libcore/java/lang/reflect/MethodTest.java b/luni/src/test/java/libcore/java/lang/reflect/MethodTest.java
index c3a436c..a3f9065 100644
--- a/luni/src/test/java/libcore/java/lang/reflect/MethodTest.java
+++ b/luni/src/test/java/libcore/java/lang/reflect/MethodTest.java
@@ -17,6 +17,7 @@
 package libcore.java.lang.reflect;
 
 import java.lang.reflect.Method;
+
 import junit.framework.TestCase;
 
 public final class MethodTest extends TestCase {
@@ -197,6 +198,23 @@ public final class MethodTest extends TestCase {
         assertEquals( "public java.lang.Process java.lang.Runtime.exec(java.lang.String[])"
                 + " throws java.io.IOException",
                 Runtime.class.getMethod("exec", new Class[] { String[].class }).toString());
+        // http://b/18488857
+        assertEquals(
+                "public int java.lang.String.compareTo(java.lang.Object)",
+                String.class.getMethod("compareTo", Object.class).toString());
+    }
+
+    // Tests that the "varargs" modifier is handled correctly.
+    // The underlying constant value for it is the same as for the "transient" field modifier.
+    // http://b/18488857
+    public void testVarargsModifier() throws NoSuchMethodException {
+        Method stringFormatMethod = String.class.getMethod(
+                "format", new Class[] { String.class, Object[].class });
+        assertTrue(stringFormatMethod.isVarArgs());
+        assertEquals(
+                "public static java.lang.String java.lang.String.format("
+                        + "java.lang.String,java.lang.Object[])",
+                stringFormatMethod.toString());
     }
 
     public static class MethodTestHelper {
diff --git a/luni/src/test/java/libcore/java/lang/reflect/ModifierTest.java b/luni/src/test/java/libcore/java/lang/reflect/ModifierTest.java
index 1bde157..0505f2f 100644
--- a/luni/src/test/java/libcore/java/lang/reflect/ModifierTest.java
+++ b/luni/src/test/java/libcore/java/lang/reflect/ModifierTest.java
@@ -100,6 +100,9 @@ public class ModifierTest extends junit.framework.TestCase {
     }
 
     public void test_toStringI() {
-        assertEquals("public abstract", Modifier.toString(Modifier.PUBLIC | Modifier.ABSTRACT));
+        // Note that it checks that "STRICT" is rendered as "strictfp" (for other modifiers,
+        // the displayed name is the same as the lowercase constant name).
+        assertEquals("public abstract strictfp",
+                Modifier.toString(Modifier.PUBLIC | Modifier.ABSTRACT | Modifier.STRICT));
     }
 }