From 6c27553c8375515e9522facf5ea82b8161d5ffb2 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Mon, 5 Dec 2022 02:27:43 +0100 Subject: [PATCH] Improve exception message for duplicate field names (#2251) --- .../bind/ReflectiveTypeAdapterFactory.java | 16 ++++++------ .../internal/reflect/ReflectionHelper.java | 11 ++++++-- .../gson/functional/NamingPolicyTest.java | 10 +++++--- .../google/gson/functional/ObjectTest.java | 25 +++++++++++++++++++ 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 5ddac50e25..4db9bd72f1 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -139,7 +139,7 @@ private static void checkAccessible(Object } } - private ReflectiveTypeAdapterFactory.BoundField createBoundField( + private BoundField createBoundField( final Gson context, final Field field, final Method accessor, final String name, final TypeToken fieldType, boolean serialize, boolean deserialize, final boolean blockInaccessible) { @@ -161,7 +161,7 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField( @SuppressWarnings("unchecked") final TypeAdapter typeAdapter = (TypeAdapter) mapped; - return new ReflectiveTypeAdapterFactory.BoundField(name, field.getName(), serialize, deserialize) { + return new BoundField(name, field, serialize, deserialize) { @Override void write(JsonWriter writer, Object source) throws IOException, IllegalAccessException { if (!serialized) return; @@ -232,7 +232,6 @@ private Map getBoundFields(Gson context, TypeToken type, return result; } - Type declaredType = type.getType(); Class originalRaw = raw; while (raw != Object.class) { Field[] fields = raw.getDeclaredFields(); @@ -298,8 +297,9 @@ private Map getBoundFields(Gson context, TypeToken type, if (previous == null) previous = replaced; } if (previous != null) { - throw new IllegalArgumentException(declaredType - + " declares multiple JSON fields named " + previous.name); + throw new IllegalArgumentException("Class " + originalRaw.getName() + + " declares multiple JSON fields named '" + previous.name + "'; conflict is caused" + + " by fields " + ReflectionHelper.fieldToString(previous.field) + " and " + ReflectionHelper.fieldToString(field)); } } type = TypeToken.get($Gson$Types.resolve(type.getType(), raw, raw.getGenericSuperclass())); @@ -310,14 +310,16 @@ private Map getBoundFields(Gson context, TypeToken type, static abstract class BoundField { final String name; + final Field field; /** Name of the underlying field */ final String fieldName; final boolean serialized; final boolean deserialized; - protected BoundField(String name, String fieldName, boolean serialized, boolean deserialized) { + protected BoundField(String name, Field field, boolean serialized, boolean deserialized) { this.name = name; - this.fieldName = fieldName; + this.field = field; + this.fieldName = field.getName(); this.serialized = serialized; this.deserialized = deserialized; } diff --git a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java index ac06121241..737bb5d495 100644 --- a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java +++ b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java @@ -53,8 +53,7 @@ public static String getAccessibleObjectDescription(AccessibleObject object, boo String description; if (object instanceof Field) { - Field field = (Field) object; - description = "field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'"; + description = "field '" + fieldToString((Field) object) + "'"; } else if (object instanceof Method) { Method method = (Method) object; @@ -75,6 +74,14 @@ public static String getAccessibleObjectDescription(AccessibleObject object, boo return description; } + /** + * Creates a string representation for a field, omitting modifiers and + * the field type. + */ + public static String fieldToString(Field field) { + return field.getDeclaringClass().getName() + "#" + field.getName(); + } + /** * Creates a string representation for a constructor. * E.g.: {@code java.lang.String(char[], int, int)} diff --git a/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java b/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java index ab76e64918..c19ee46063 100644 --- a/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java +++ b/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java @@ -22,10 +22,8 @@ import com.google.gson.annotations.SerializedName; import com.google.gson.common.TestTypes.ClassWithSerializedNameFields; import com.google.gson.common.TestTypes.StringWrapper; - -import junit.framework.TestCase; - import java.lang.reflect.Field; +import junit.framework.TestCase; /** * Functional tests for naming policies. @@ -122,6 +120,12 @@ public void testGsonDuplicateNameUsingSerializedNameFieldNamingPolicySerializati gson.toJson(target); fail(); } catch (IllegalArgumentException expected) { + assertEquals( + "Class com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields declares multiple JSON fields named 'a';" + + " conflict is caused by fields com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields#a and" + + " com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields#b", + expected.getMessage() + ); } } diff --git a/gson/src/test/java/com/google/gson/functional/ObjectTest.java b/gson/src/test/java/com/google/gson/functional/ObjectTest.java index bed5b59859..cf851e060a 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -145,6 +145,31 @@ public void testClassWithNoFieldsDeserialization() throws Exception { assertEquals(expected, target); } + private static class Subclass extends Superclass1 { + } + private static class Superclass1 extends Superclass2 { + @SuppressWarnings("unused") + String s; + } + private static class Superclass2 { + @SuppressWarnings("unused") + String s; + } + + public void testClassWithDuplicateFields() { + try { + gson.getAdapter(Subclass.class); + fail(); + } catch (IllegalArgumentException e) { + assertEquals( + "Class com.google.gson.functional.ObjectTest$Subclass declares multiple JSON fields named 's';" + + " conflict is caused by fields com.google.gson.functional.ObjectTest$Superclass1#s and" + + " com.google.gson.functional.ObjectTest$Superclass2#s", + e.getMessage() + ); + } + } + public void testNestedSerialization() throws Exception { Nested target = new Nested(new BagOfPrimitives(10, 20, false, "stringValue"), new BagOfPrimitives(30, 40, true, "stringValue"));