Adjust Record adapter and extend test coverage

gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java

@@ -261,14 +261,17 @@ public T construct() {
           @SuppressWarnings("unchecked") // T is the same raw type as is requested
           T newInstance = (T) constructor.newInstance();
           return newInstance;
-        } catch (InstantiationException e) {
-          // TODO: JsonParseException ?
-          throw new RuntimeException("Failed to invoke " + constructor + " with no args", e);
+        }
+        // Note: InstantiationException should be impossible because check at start of method made sure
+        //   that class is not abstract
+        catch (InstantiationException e) {
+          throw new RuntimeException("Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+              + " with no args", e);
         } catch (InvocationTargetException e) {
-          // TODO: don't wrap if cause is unchecked!
+          // TODO: don't wrap if cause is unchecked?
           // TODO: JsonParseException ?
-          throw new RuntimeException("Failed to invoke " + constructor + " with no args",
-              e.getTargetException());
+          throw new RuntimeException("Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+              + " with no args", e.getCause());
         } catch (IllegalAccessException e) {
           throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
         }

gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java

@@ -24,65 +24,83 @@ public class ReflectionHelper {
 
   private ReflectionHelper() {}
 
-  /**
-   * Tries making the field accessible, wrapping any thrown exception in a {@link JsonIOException}
-   * with descriptive message.
-   *
-   * @param field field to make accessible
-   * @throws JsonIOException if making the field accessible fails
-   */
-  public static void makeAccessible(Field field) throws JsonIOException {
-    makeAccessible("field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'", field);
-  }
-
-  /**
-   * Tries making the constructor accessible, wrapping any thrown exception in a {@link JsonIOException}
-   * with descriptive message.
-   *
-   * @param constructor constructor to make accessible
-   * @throws JsonIOException if making the constructor accessible fails
-   */
-  public static void makeAccessible(Constructor<?> constructor) throws JsonIOException {
-    makeAccessible(
-            "constructor " + constructor + " in " + constructor.getDeclaringClass().getName(),
-            constructor
-    );
-  }
-
   /**
    * Internal implementation of making an {@link AccessibleObject} accessible.
    *
-   * @param description describe what we are attempting to make accessible
    * @param object the object that {@link AccessibleObject#setAccessible(boolean)} should be called on.
    * @throws JsonIOException if making the object accessible fails
    */
-  private static void makeAccessible(String description, AccessibleObject object) throws JsonIOException {
+  public static void makeAccessible(AccessibleObject object) throws JsonIOException {
     try {
       object.setAccessible(true);
     } catch (Exception exception) {
-      throw new JsonIOException("Failed making " + description + "' accessible; either change its visibility "
-              + "or write a custom TypeAdapter for its declaring type", exception);
+      String description = getAccessibleObjectDescription(object, false);
+      throw new JsonIOException("Failed making " + description + " accessible; either increase its visibility"
+              + " or write a custom TypeAdapter for its declaring type.", exception);
     }
   }
 
+  /**
+   * Returns a short string describing the {@link AccessibleObject} in a human-readable way.
+   * The result is normally shorter than {@link AccessibleObject#toString()} because it omits
+   * modifiers (e.g. {@code final}) and uses simple names for constructor and method parameter
+   * types.
+   *
+   * @param object object to describe
+   * @param uppercaseFirstLetter whether the first letter of the description should be uppercased
+   */
+  public static String getAccessibleObjectDescription(AccessibleObject object, boolean uppercaseFirstLetter) {
+    String description;
+
+    if (object instanceof Field) {
+      Field field = (Field) object;
+      description = "field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'";
+    } else if (object instanceof Method) {
+      Method method = (Method) object;
+
+      StringBuilder methodSignatureBuilder = new StringBuilder(method.getName());
+      appendExecutableParameters(method, methodSignatureBuilder);
+      String methodSignature = methodSignatureBuilder.toString();
+
+      description = "method '" + method.getDeclaringClass().getName() + "#" + methodSignature + "'";
+    } else if (object instanceof Constructor) {
+      description = "constructor '" + constructorToString((Constructor<?>) object) + "'";
+    } else {
+      description = "<unknown AccessibleObject> " + object.toString();
+    }
+
+    if (uppercaseFirstLetter && Character.isLowerCase(description.charAt(0))) {
+      description = Character.toUpperCase(description.charAt(0)) + description.substring(1);
+    }
+    return description;
+  }
+
   /**
    * Creates a string representation for a constructor.
-   * E.g.: {@code java.lang.String#String(char[], int, int)}
+   * E.g.: {@code java.lang.String(char[], int, int)}
    */
-  private static String constructorToString(Constructor<?> constructor) {
-    StringBuilder stringBuilder = new StringBuilder(constructor.getDeclaringClass().getName())
-      .append('#')
-      .append(constructor.getDeclaringClass().getSimpleName())
-      .append('(');
-    Class<?>[] parameters = constructor.getParameterTypes();
+  public static String constructorToString(Constructor<?> constructor) {
+    StringBuilder stringBuilder = new StringBuilder(constructor.getDeclaringClass().getName());
+    appendExecutableParameters(constructor, stringBuilder);
+
+    return stringBuilder.toString();
+  }
+
+  // Note: Ideally parameter type would be java.lang.reflect.Executable, but that was added in Java 8
+  private static void appendExecutableParameters(AccessibleObject executable, StringBuilder stringBuilder) {
+    stringBuilder.append('(');
+
+    Class<?>[] parameters = (executable instanceof Method)
+        ? ((Method) executable).getParameterTypes()
+        : ((Constructor<?>) executable).getParameterTypes();
     for (int i = 0; i < parameters.length; i++) {
       if (i > 0) {
         stringBuilder.append(", ");
       }
       stringBuilder.append(parameters[i].getSimpleName());
     }
 
-    return stringBuilder.append(')').toString();
+    stringBuilder.append(')');
   }
 
   /**
@@ -98,10 +116,10 @@ public static String tryMakeAccessible(Constructor<?> constructor) {
       constructor.setAccessible(true);
       return null;
     } catch (Exception exception) {
-      return "Failed making constructor '" + constructorToString(constructor) + "' accessible; "
-          + "either change its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type: "
+      return "Failed making constructor '" + constructorToString(constructor) + "' accessible;"
+          + " either increase its visibility or write a custom InstanceCreator or TypeAdapter for"
           // Include the message since it might contain more detailed information
-          + exception.getMessage();
+          + " its declaring type: " + exception.getMessage();
     }
   }
 
@@ -125,20 +143,20 @@ public static <T> Constructor<T> getCanonicalRecordConstructor(Class<T> raw) {
 
   public static RuntimeException createExceptionForUnexpectedIllegalAccess(
       IllegalAccessException exception) {
-    throw new RuntimeException("Unexpected IllegalAccessException occurred (Gson " + GsonBuildConfig.VERSION + "). "
-        + "Certain ReflectionAccessFilter features require Java >= 9 to work correctly. If you are not using "
-        + "ReflectionAccessFilter, report this to the Gson maintainers.",
+    throw new RuntimeException("Unexpected IllegalAccessException occurred (Gson " + GsonBuildConfig.VERSION + ")."
+        + " Certain ReflectionAccessFilter features require Java >= 9 to work correctly. If you are not using"
+        + " ReflectionAccessFilter, report this to the Gson maintainers.",
         exception);
   }
 
 
-  public static RuntimeException createExceptionForRecordReflectionException(
+  private static RuntimeException createExceptionForRecordReflectionException(
           ReflectiveOperationException exception) {
-    throw new RuntimeException("Unexpected ReflectiveOperationException occurred "
-            + "(Gson " + GsonBuildConfig.VERSION + "). "
-            + "To support Java records, reflection is utilized to read out information "
-            + "about records. All these invocations happens after it is established "
-            + "that records exists in the JVM. This exception is unexpected behaviour.",
+    throw new RuntimeException("Unexpected ReflectiveOperationException occurred"
+            + " (Gson " + GsonBuildConfig.VERSION + ")."
+            + " To support Java records, reflection is utilized to read out information"
+            + " about records. All these invocations happens after it is established"
+            + " that records exist in the JVM. This exception is unexpected behavior.",
             exception);
   }
 
@@ -164,9 +182,10 @@ private static class RecordSupportedHelper extends RecordHelper {
     private RecordSupportedHelper() throws NoSuchMethodException {
       isRecord = Class.class.getMethod("isRecord");
       getRecordComponents = Class.class.getMethod("getRecordComponents");
-      Class<?> recordComponentType = getRecordComponents.getReturnType().getComponentType();
-      getName = recordComponentType.getMethod("getName");
-      getType = recordComponentType.getMethod("getType");
+      // Class java.lang.reflect.RecordComponent
+      Class<?> classRecordComponent = getRecordComponents.getReturnType().getComponentType();
+      getName = classRecordComponent.getMethod("getName");
+      getType = classRecordComponent.getMethod("getType");
     }
 
     @Override