diff --git a/GsonDesignDocument.md b/GsonDesignDocument.md index 2c3702ee13..a80a81e627 100644 --- a/GsonDesignDocument.md +++ b/GsonDesignDocument.md @@ -1,57 +1,59 @@ # Gson Design Document -This document presents issues that we faced while designing Gson. It is meant for advanced users or developers working on Gson. If you are interested in learning how to use Gson, see its user guide. +This document presents issues that we faced while designing Gson. It is meant for advanced users or developers working on Gson. If you are interested in learning how to use Gson, see its user guide. -**Navigating the Json tree or the target Type Tree while deserializing** +Some information in this document is outdated and does not reflect the current state of Gson. This information can however still be relevant for understanding the history of Gson. -When you are deserializing a Json string into an object of desired type, you can either navigate the tree of the input, or the type tree of the desired type. Gson uses the latter approach of navigating the type of the target object. This keeps you in tight control of instantiating only the type of objects that you are expecting (essentially validating the input against the expected "schema"). By doing this, you also ignore any extra fields that the Json input has but were not expected. +## Navigating the Json tree or the target Type Tree while deserializing -As part of Gson, we wrote a general purpose ObjectNavigator that can take any object and navigate through its fields calling a visitor of your choice. +When you are deserializing a Json string into an object of desired type, you can either navigate the tree of the input, or the type tree of the desired type. Gson uses the latter approach of navigating the type of the target object. This keeps you in tight control of instantiating only the type of objects that you are expecting (essentially validating the input against the expected "schema"). By doing this, you also ignore any extra fields that the Json input has but were not expected. -**Supporting richer serialization semantics than deserialization semantics** +As part of Gson, we wrote a general purpose ObjectNavigator that can take any object and navigate through its fields calling a visitor of your choice. -Gson supports serialization of arbitrary collections, but can only deserialize genericized collections. this means that Gson can, in some cases, fail to deserialize Json that it wrote. This is primarily a limitation of the Java type system since when you encounter a Json array of arbitrary types there is no way to detect the types of individual elements. We could have chosen to restrict the serialization to support only generic collections, but chose not to.This is because often the user of the library are concerned with either serialization or deserialization, but not both. In such cases, there is no need to artificially restrict the serialization capabilities. +## Supporting richer serialization semantics than deserialization semantics -**Supporting serialization and deserialization of classes that are not under your control and hence can not be modified** +Gson supports serialization of arbitrary collections, but can only deserialize genericized collections. this means that Gson can, in some cases, fail to deserialize Json that it wrote. This is primarily a limitation of the Java type system since when you encounter a Json array of arbitrary types there is no way to detect the types of individual elements. We could have chosen to restrict the serialization to support only generic collections, but chose not to. This is because often the user of the library are concerned with either serialization or deserialization, but not both. In such cases, there is no need to artificially restrict the serialization capabilities. -Some Json libraries use annotations on fields or methods to indicate which fields should be used for Json serialization. That approach essentially precludes the use of classes from JDK or third-party libraries. We solved this problem by defining the notion of Custom serializers and deserializers. This approach is not new, and was used by the JAX-RPC technology to solve essentially the same problem. +## Supporting serialization and deserialization of classes that are not under your control and hence can not be modified -**Using Checked vs Unchecked exceptions to indicate a parsing error** +Some Json libraries use annotations on fields or methods to indicate which fields should be used for Json serialization. That approach essentially precludes the use of classes from JDK or third-party libraries. We solved this problem by defining the notion of custom serializers and deserializers. This approach is not new, and was used by the JAX-RPC technology to solve essentially the same problem. -We chose to use unchecked exceptions to indicate a parsing failure. This is primarily done because usually the client can not recover from bad input, and hence forcing them to catch a checked exception results in sloppy code in the catch() block. +## Using Checked vs Unchecked exceptions to indicate a parsing error -**Creating class instances for deserialization** +We chose to use unchecked exceptions to indicate a parsing failure. This is primarily done because usually the client can not recover from bad input, and hence forcing them to catch a checked exception results in sloppy code in the `catch()` block. -Gson needs to create a dummy class instance before it can deserialize Json data into its fields. We could have used Guice to get such an instance, but that would have resulted in a dependency on Guice. Moreover, it probably would have done the wrong thing since Guice is expected to return a valid instance, whereas we need to create a dummy one. Worse, Gson would overwrite the fields of that instance with the incoming data there by modifying the instance for all subsequent Guice injections. This is clearly not a desired behavior. Hence, we create class instances by invoking the parameterless constructor. We also handle the primitive types, enums, collections, sets, maps and trees as a special case. +## Creating class instances for deserialization -To solve the problem of supporting unmodifiable types, we use custom instance creators. So, if you want to use a library types that does not define a default constructor (for example, Money class), then you can register an instance creator that returns a dummy instance when asked. +Gson needs to create a dummy class instance before it can deserialize Json data into its fields. We could have used Guice to get such an instance, but that would have resulted in a dependency on Guice. Moreover, it probably would have done the wrong thing since Guice is expected to return a valid instance, whereas we need to create a dummy one. Worse, Gson would overwrite the fields of that instance with the incoming data thereby modifying the instance for all subsequent Guice injections. This is clearly not a desired behavior. Hence, we create class instances by invoking the parameterless constructor. We also handle the primitive types, enums, collections, sets, maps and trees as a special case. -**Using fields vs getters to indicate Json elements** +To solve the problem of supporting unmodifiable types, we use custom instance creators. So, if you want to use a library type that does not define a default constructor (for example, `Money` class), then you can register an instance creator that returns a dummy instance when asked. -Some Json libraries use the getters of a type to deduce the Json elements. We chose to use all fields (up the inheritance hierarchy) that are not transient, static, or synthetic. We did this because not all classes are written with suitably named getters. Moreover, getXXX or isXXX might be semantic rather than indicating properties. +## Using fields vs getters to indicate Json elements -However, there are good arguments to support properties as well. We intend to enhance Gson in a latter version to support properties as an alternate mapping for indicating Json fields. For now, Gson is fields-based. +Some Json libraries use the getters of a type to deduce the Json elements. We chose to use all fields (up the inheritance hierarchy) that are not transient, static, or synthetic. We did this because not all classes are written with suitably named getters. Moreover, `getXXX` or `isXXX` might be semantic rather than indicating properties. -**Why are most classes in Gson marked as final?** +However, there are good arguments to support properties as well. We intend to enhance Gson in a later version to support properties as an alternate mapping for indicating Json fields. For now, Gson is fields-based. -While Gson provides a fairly extensible architecture by providing pluggable serializers and deserializers, Gson classes were not specifically designed to be extensible. Providing non-final classes would have allowed a user to legitimately extend Gson classes, and then expect that behavior to work in all subsequent revisions. We chose to limit such use-cases by marking classes as final, and waiting until a good use-case emerges to allow extensibility. Marking a class final also has a minor benefit of providing additional optimization opportunities to Java compiler and virtual machine. +## Why are most classes in Gson marked as final? -**Why are inner interfaces and classes used heavily in Gson?** +While Gson provides a fairly extensible architecture by providing pluggable serializers and deserializers, Gson classes were not specifically designed to be extensible. Providing non-final classes would have allowed a user to legitimately extend Gson classes, and then expect that behavior to work in all subsequent revisions. We chose to limit such use-cases by marking classes as final, and waiting until a good use-case emerges to allow extensibility. Marking a class final also has a minor benefit of providing additional optimization opportunities to Java compiler and virtual machine. -Gson uses inner classes substantially. Many of the public interfaces are inner interfaces too (see JsonSerializer.Context or JsonDeserializer.Context as an example). These are primarily done as a matter of style. For example, we could have moved JsonSerializer.Context to be a top-level class JsonSerializerContext, but chose not to do so. However, if you can give us good reasons to rename it alternately, we are open to changing this philosophy. +## Why are inner interfaces and classes used heavily in Gson? -**Why do you provide two ways of constructing Gson?** +Gson uses inner classes substantially. Many of the public interfaces are inner interfaces too (see `JsonSerializer.Context` or `JsonDeserializer.Context` as an example). These are primarily done as a matter of style. For example, we could have moved `JsonSerializer.Context` to be a top-level class `JsonSerializerContext`, but chose not to do so. However, if you can give us good reasons to rename it alternately, we are open to changing this philosophy. -Gson can be constructed in two ways: by invoking new Gson() or by using a GsonBuilder. We chose to provide a simple no-args constructor to handle simple use-cases for Gson where you want to use default options, and quickly want to get going with writing code. For all other situations, where you need to configure Gson with options such as formatters, version controls etc, we use a builder pattern. The builder pattern allows a user to specify multiple optional settings for what essentially become constructor parameters for Gson. +## Why do you provide two ways of constructing Gson? -**Comparing Gson with Alternate Approaches** +Gson can be constructed in two ways: by invoking `new Gson()` or by using a `GsonBuilder`. We chose to provide a simple no-args constructor to handle simple use-cases for Gson where you want to use default options, and quickly want to get going with writing code. For all other situations, where you need to configure Gson with options such as formatters, version controls etc., we use a builder pattern. The builder pattern allows a user to specify multiple optional settings for what essentially become constructor parameters for Gson. + +## Comparing Gson with alternate approaches Note that these comparisons were done while developing Gson so these date back to mid to late 2007. -__Comparing Gson with org.json library__ +### Comparing Gson with org.json library -org.json is a much lower-level library that can be used to write a toJson() method in a class. If you can not use Gson directly (may be because of platform restrictions regarding reflection), you could use org.json to hand-code a toJson method in each object. +org.json is a much lower-level library that can be used to write a `toJson()` method in a class. If you can not use Gson directly (maybe because of platform restrictions regarding reflection), you could use org.json to hand-code a `toJson` method in each object. -__Comparing Gson with org.json.simple library__ +### Comparing Gson with org.json.simple library org.json.simple library is very similar to org.json library and hence fairly low level. The key issue with this library is that it does not handle exceptions very well. In some cases it appeared to just eat the exception while in other cases it throws an "Error" rather than an exception. diff --git a/Troubleshooting.md b/Troubleshooting.md index 94f8391ec5..91fd22110a 100644 --- a/Troubleshooting.md +++ b/Troubleshooting.md @@ -66,7 +66,7 @@ Or in case this occurs for a field in one of your classes which you did not actu **Symptom:** You released a new version of your Android app and it fails to parse JSON data created by the previous version of your app -**Reason:** You probably have not configured ProGuard / R8 correctly; probably the fields names are being obfuscated and their naming changed between the versions of your app +**Reason:** You probably have not configured ProGuard / R8 correctly; probably the field names are being obfuscated and their naming changed between the versions of your app **Solution:** Make sure you have configured ProGuard / R8 correctly to preserve the names of your fields. See the [Android example](examples/android-proguard-example/README.md) for more information. @@ -137,7 +137,7 @@ To spot syntax errors in the JSON data easily you can open it in an editor with **Symptom:** JSON data contains an integral number such as `45` but Gson returns it as `double` -**Reason:** When parsing a JSON number as `Object`, Gson will by default create always return a `double` +**Reason:** When parsing a JSON number as `Object`, Gson will by default always return a `double` **Solution:** Use [`GsonBuilder.setObjectToNumberStrategy`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#setObjectToNumberStrategy(com.google.gson.ToNumberStrategy)) to specify what type of number should be returned diff --git a/UserGuide.md b/UserGuide.md index f519865434..ed2fdf3368 100644 --- a/UserGuide.md +++ b/UserGuide.md @@ -366,7 +366,7 @@ class Event { You can serialize the collection with Gson without doing anything specific: `toJson(collection)` would write out the desired output. -However, deserialization with `fromJson(json, Collection.class)` will not work since Gson has no way of knowing how to map the input to the types. Gson requires that you provide a genericised version of collection type in `fromJson()`. So, you have three options: +However, deserialization with `fromJson(json, Collection.class)` will not work since Gson has no way of knowing how to map the input to the types. Gson requires that you provide a genericized version of the collection type in `fromJson()`. So, you have three options: 1. Use Gson's parser API (low-level streaming parser or the DOM parser JsonParser) to parse the array elements and then use `Gson.fromJson()` on each of the array elements.This is the preferred approach. [Here is an example](extras/src/main/java/com/google/gson/extras/examples/rawcollections/RawCollectionsExample.java) that demonstrates how to do this. @@ -389,7 +389,7 @@ You can also find source code for some commonly used classes such as JodaTime at ### Custom Serialization and Deserialization -Sometimes default representation is not what you want. This is often the case when dealing with library classes (DateTime, etc). +Sometimes the default representation is not what you want. This is often the case when dealing with library classes (DateTime, etc.). Gson allows you to register your own custom serializers and deserializers. This is done by defining two parts: * JSON Serializers: Need to define custom serialization for an object @@ -741,7 +741,7 @@ In addition Gson's object model and data binding, you can use Gson to read from ## Issues in Designing Gson -See the [Gson design document](GsonDesignDocument.md "Gson design document") for a discussion of issues we faced while designing Gson. It also include a comparison of Gson with other Java libraries that can be used for JSON conversion. +See the [Gson design document](GsonDesignDocument.md "Gson design document") for a discussion of issues we faced while designing Gson. It also includes a comparison of Gson with other Java libraries that can be used for JSON conversion. ## Future Enhancements to Gson diff --git a/graal-native-image-test/pom.xml b/graal-native-image-test/pom.xml index f51b60018c..8ce132f1bc 100644 --- a/graal-native-image-test/pom.xml +++ b/graal-native-image-test/pom.xml @@ -150,7 +150,7 @@ org.graalvm.buildtools native-maven-plugin - 0.9.24 + 0.9.27 true diff --git a/graal-native-image-test/src/test/java/com/google/gson/native_test/Java17RecordReflectionTest.java b/graal-native-image-test/src/test/java/com/google/gson/native_test/Java17RecordReflectionTest.java index 3eb9634083..51ea11d199 100644 --- a/graal-native-image-test/src/test/java/com/google/gson/native_test/Java17RecordReflectionTest.java +++ b/graal-native-image-test/src/test/java/com/google/gson/native_test/Java17RecordReflectionTest.java @@ -27,7 +27,6 @@ import java.io.IOException; import org.junit.jupiter.api.Test; -@SuppressWarnings("UnusedVariable") // workaround for https://github.com/google/error-prone/issues/2713 class Java17RecordReflectionTest { public record PublicRecord(int i) { } diff --git a/gson/pom.xml b/gson/pom.xml index 70443acd99..0aa1b0d99e 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -46,7 +46,7 @@ com.google.errorprone error_prone_annotations - 2.21.1 + 2.22.0 diff --git a/gson/src/main/java/com/google/gson/FieldAttributes.java b/gson/src/main/java/com/google/gson/FieldAttributes.java index 5ce710bd1d..4dce2b7cc4 100644 --- a/gson/src/main/java/com/google/gson/FieldAttributes.java +++ b/gson/src/main/java/com/google/gson/FieldAttributes.java @@ -64,6 +64,8 @@ public String getName() { } /** + * Returns the declared generic type of the field. + * *

For example, assume the following class definition: * 

    * public class Foo {
@@ -104,7 +106,7 @@ public Class getDeclaredClass() {
   }
 
   /**
-   * Return the {@code T} annotation object from this field if it exist; otherwise returns
+   * Returns the {@code T} annotation object from this field if it exists; otherwise returns
    * {@code null}.
    *
    * @param annotation the class of the annotation that will be retrieved
@@ -115,7 +117,7 @@ public  T getAnnotation(Class annotation) {
   }
 
   /**
-   * Return the annotations that are present on this field.
+   * Returns the annotations that are present on this field.
    *
    * @return an array of all the annotations set on the field
    * @since 1.4
diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java
index 4ffda1ee7f..1921eca3a5 100644
--- a/gson/src/main/java/com/google/gson/Gson.java
+++ b/gson/src/main/java/com/google/gson/Gson.java
@@ -601,7 +601,7 @@ public  TypeAdapter getAdapter(TypeToken type) {
   /**
    * This method is used to get an alternate type adapter for the specified type. This is used
    * to access a type adapter that is overridden by a {@link TypeAdapterFactory} that you
-   * may have registered. This features is typically used when you want to register a type
+   * may have registered. This feature is typically used when you want to register a type
    * adapter that does a little bit of work but then delegates further processing to the Gson
    * default type adapter. Here is an example:
    * 
Let's say we want to write a type adapter that counts the number of objects being read
@@ -635,7 +635,7 @@ public  TypeAdapter getAdapter(TypeToken type) {
    * System.out.println("Num JSON writes: " + stats.numWrites);
    * }
* Note that this call will skip all factories registered before {@code skipPast}. In case of - * multiple TypeAdapterFactories registered it is up to the caller of this function to insure + * multiple TypeAdapterFactories registered it is up to the caller of this function to ensure * that the order of registration does not prevent this method from reaching a factory they * would expect to reply from this call. * Note that since you can not override the type adapter factories for some types, see @@ -680,7 +680,7 @@ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeTo } if (skipPastFound) { - throw new IllegalArgumentException("GSON cannot serialize " + type); + throw new IllegalArgumentException("GSON cannot serialize or deserialize " + type); } else { // Probably a factory from @JsonAdapter on a field return getAdapter(type); @@ -1019,7 +1019,7 @@ public void toJson(JsonElement jsonElement, JsonWriter writer) throws JsonIOExce * This method deserializes the specified JSON into an object of the specified class. It is not * suitable to use if the specified class is a generic type since it will not have the generic * type information because of the Type Erasure feature of Java. Therefore, this method should not - * be used if the desired type is a generic type. Note that this method works fine if the any of + * be used if the desired type is a generic type. Note that this method works fine if any of * the fields of the specified object are generics, just the object itself should not be a * generic type. For the cases when the object is of generic type, invoke * {@link #fromJson(String, TypeToken)}. If you have the JSON in a {@link Reader} instead of diff --git a/gson/src/main/java/com/google/gson/InstanceCreator.java b/gson/src/main/java/com/google/gson/InstanceCreator.java index b973da07eb..486f250482 100644 --- a/gson/src/main/java/com/google/gson/InstanceCreator.java +++ b/gson/src/main/java/com/google/gson/InstanceCreator.java @@ -73,6 +73,8 @@ * * @param the type of object that will be created by this implementation. * + * @see GsonBuilder#registerTypeAdapter(Type, Object) + * * @author Inderjeet Singh * @author Joel Leitch */ diff --git a/gson/src/main/java/com/google/gson/JsonArray.java b/gson/src/main/java/com/google/gson/JsonArray.java index fbdf80edd2..e8768ef592 100644 --- a/gson/src/main/java/com/google/gson/JsonArray.java +++ b/gson/src/main/java/com/google/gson/JsonArray.java @@ -225,7 +225,7 @@ public Iterator iterator() { * * @param i the index of the element that is being sought. * @return the element present at the i-th index. - * @throws IndexOutOfBoundsException if i is negative or greater than or equal to the + * @throws IndexOutOfBoundsException if {@code i} is negative or greater than or equal to the * {@link #size()} of the array. */ public JsonElement get(int i) { diff --git a/gson/src/main/java/com/google/gson/JsonPrimitive.java b/gson/src/main/java/com/google/gson/JsonPrimitive.java index 827de959b9..7095c05a35 100644 --- a/gson/src/main/java/com/google/gson/JsonPrimitive.java +++ b/gson/src/main/java/com/google/gson/JsonPrimitive.java @@ -285,7 +285,7 @@ public boolean equals(Object obj) { return other.value == null; } if (isIntegral(this) && isIntegral(other)) { - return this.value instanceof BigInteger || other.value instanceof BigInteger + return (this.value instanceof BigInteger || other.value instanceof BigInteger) ? this.getAsBigInteger().equals(other.getAsBigInteger()) : this.getAsNumber().longValue() == other.getAsNumber().longValue(); } diff --git a/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java b/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java index d168575940..5a5da72e13 100644 --- a/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java +++ b/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java @@ -17,6 +17,8 @@ package com.google.gson.annotations; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.InstanceCreator; import com.google.gson.JsonDeserializer; import com.google.gson.JsonSerializer; import com.google.gson.TypeAdapter; @@ -35,11 +37,13 @@ * @JsonAdapter(UserJsonAdapter.class) * public class User { * public final String firstName, lastName; + * * private User(String firstName, String lastName) { * this.firstName = firstName; * this.lastName = lastName; * } * } + * * public class UserJsonAdapter extends TypeAdapter<User> { * @Override public void write(JsonWriter out, User user) throws IOException { * // implement write: combine firstName and lastName into name @@ -47,8 +51,8 @@ * out.name("name"); * out.value(user.firstName + " " + user.lastName); * out.endObject(); - * // implement the write method * } + * * @Override public User read(JsonReader in) throws IOException { * // implement read: split name into firstName and lastName * in.beginObject(); @@ -60,14 +64,15 @@ * } * * - * Since User class specified UserJsonAdapter.class in @JsonAdapter annotation, it - * will automatically be invoked to serialize/deserialize User instances. + * Since {@code User} class specified {@code UserJsonAdapter.class} in {@code @JsonAdapter} + * annotation, it will automatically be invoked to serialize/deserialize {@code User} instances. * - *

Here is an example of how to apply this annotation to a field. + *

Here is an example of how to apply this annotation to a field. * 

  * private static final class Gadget {
- *   @JsonAdapter(UserJsonAdapter2.class)
+ *   @JsonAdapter(UserJsonAdapter.class)
  *   final User user;
+ *
  *   Gadget(User user) {
  *     this.user = user;
  *   }
@@ -75,15 +80,30 @@
  *
* * It's possible to specify different type adapters on a field, that - * field's type, and in the {@link com.google.gson.GsonBuilder}. Field - * annotations take precedence over {@code GsonBuilder}-registered type + * field's type, and in the {@link GsonBuilder}. Field annotations + * take precedence over {@code GsonBuilder}-registered type * adapters, which in turn take precedence over annotated types. * *

The class referenced by this annotation must be either a {@link * TypeAdapter} or a {@link TypeAdapterFactory}, or must implement one * or both of {@link JsonDeserializer} or {@link JsonSerializer}. * Using {@link TypeAdapterFactory} makes it possible to delegate - * to the enclosing {@link Gson} instance. + * to the enclosing {@link Gson} instance. By default the specified + * adapter will not be called for {@code null} values; set {@link #nullSafe()} + * to {@code false} to let the adapter handle {@code null} values itself. + * + *

The type adapter is created in the same way Gson creates instances of + * custom classes during deserialization, that means: + *

    + *
  1. If a custom {@link InstanceCreator} has been registered for the + * adapter class, it will be used to create the instance + *
  2. Otherwise, if the adapter class has a no-args constructor + * (regardless of which visibility), it will be invoked to create + * the instance + *
  3. Otherwise, JDK {@code Unsafe} will be used to create the instance; + * see {@link GsonBuilder#disableJdkUnsafe()} for the unexpected + * side-effects this might have + *
* *

{@code Gson} instances might cache the adapter they create for * a {@code @JsonAdapter} annotation. It is not guaranteed that a new @@ -104,7 +124,13 @@ /** Either a {@link TypeAdapter} or {@link TypeAdapterFactory}, or one or both of {@link JsonDeserializer} or {@link JsonSerializer}. */ Class value(); - /** false, to be able to handle {@code null} values within the adapter, default value is true. */ + /** + * Whether the adapter referenced by {@link #value()} should be made {@linkplain TypeAdapter#nullSafe() null-safe}. + * + *

If {@code true} (the default), it will be made null-safe and Gson will handle {@code null} Java objects + * on serialization and JSON {@code null} on deserialization without calling the adapter. If {@code false}, + * the adapter will have to handle the {@code null} values. + */ boolean nullSafe() default true; } diff --git a/gson/src/main/java/com/google/gson/annotations/Until.java b/gson/src/main/java/com/google/gson/annotations/Until.java index a5fcabd4a4..9fc28f40c0 100644 --- a/gson/src/main/java/com/google/gson/annotations/Until.java +++ b/gson/src/main/java/com/google/gson/annotations/Until.java @@ -63,7 +63,7 @@ public @interface Until { /** - * The value indicating a version number until this member or type should be be included. + * The value indicating a version number until this member or type should be included. * The number is exclusive; annotated elements will be included if {@code gsonVersion < value}. */ double value(); diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java index b444a4bd61..8b528b7ff4 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java @@ -134,6 +134,7 @@ TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gso TypeAdapter tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, skipPast, nullSafe); typeAdapter = tempAdapter; + // TreeTypeAdapter handles nullSafe; don't additionally call `nullSafe()` nullSafe = false; } else { throw new IllegalArgumentException("Invalid attempt to bind an instance of " diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java index b1110f1874..f82257287d 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java @@ -139,14 +139,14 @@ private void put(JsonElement value) { @Override public JsonWriter name(String name) throws IOException { Objects.requireNonNull(name, "name == null"); if (stack.isEmpty() || pendingName != null) { - throw new IllegalStateException(); + throw new IllegalStateException("Did not expect a name"); } JsonElement element = peek(); if (element instanceof JsonObject) { pendingName = name; return this; } - throw new IllegalStateException(); + throw new IllegalStateException("Please begin an object before writing a name."); } @CanIgnoreReturnValue diff --git a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java index f4d6eedca8..0c72d7b73c 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java @@ -36,7 +36,7 @@ /** * Adapts a Gson 1.x tree-style adapter as a streaming TypeAdapter. Since the * tree adapter may be serialization-only or deserialization-only, this class - * has a facility to lookup a delegate type adapter on demand. + * has a facility to look up a delegate type adapter on demand. */ public final class TreeTypeAdapter extends SerializationDelegatingTypeAdapter { private final JsonSerializer serializer; diff --git a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java index 75a991ead7..01cb26bda1 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java +++ b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java @@ -45,7 +45,7 @@ public void write(JsonWriter out, T value) throws IOException { // Order of preference for choosing type adapters // First preference: a type adapter registered for the runtime type // Second preference: a type adapter registered for the declared type - // Third preference: reflective type adapter for the runtime type (if it is a sub class of the declared type) + // Third preference: reflective type adapter for the runtime type (if it is a subclass of the declared type) // Fourth preference: reflective type adapter for the declared type TypeAdapter chosen = delegate; diff --git a/gson/src/main/java/com/google/gson/stream/JsonReader.java b/gson/src/main/java/com/google/gson/stream/JsonReader.java index baad1166ca..b85fba8f7c 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -686,7 +686,7 @@ private int peekKeyword() throws IOException { return PEEKED_NONE; } - // Upper cased keywords are not allowed in STRICT mode + // Uppercased keywords are not allowed in STRICT mode boolean allowsUpperCased = strictness != Strictness.STRICT; // Confirm that chars [0..length) match the keyword. @@ -1745,7 +1745,7 @@ private IllegalStateException unexpectedTokenError(String expected) throws IOExc * Consumes the non-execute prefix if it exists. */ private void consumeNonExecutePrefix() throws IOException { - // fast forward through the leading whitespace + // fast-forward through the leading whitespace int unused = nextNonWhitespace(true); pos--; diff --git a/gson/src/main/java/com/google/gson/stream/JsonWriter.java b/gson/src/main/java/com/google/gson/stream/JsonWriter.java index bb285bd0c1..00ae92fd1b 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonWriter.java +++ b/gson/src/main/java/com/google/gson/stream/JsonWriter.java @@ -495,11 +495,9 @@ public JsonWriter name(String name) throws IOException { if (deferredName != null) { throw new IllegalStateException("Already wrote a name, expecting a value."); } - if (stackSize == 0) { - throw new IllegalStateException("JsonWriter is closed."); - } - if (stackSize == 1 && (peek() == EMPTY_DOCUMENT || peek() == NONEMPTY_DOCUMENT)) { - throw new IllegalStateException("Please begin an object before this."); + int context = peek(); + if (context != EMPTY_OBJECT && context != NONEMPTY_OBJECT) { + throw new IllegalStateException("Please begin an object before writing a name."); } deferredName = name; return this; diff --git a/gson/src/test/java/com/google/gson/functional/EnumTest.java b/gson/src/test/java/com/google/gson/functional/EnumTest.java index 0ca4acdf28..5103a045c0 100644 --- a/gson/src/test/java/com/google/gson/functional/EnumTest.java +++ b/gson/src/test/java/com/google/gson/functional/EnumTest.java @@ -176,7 +176,7 @@ public void testEnumSet() { Type type = new TypeToken>() {}.getType(); EnumSet bar = gson.fromJson(json, type); assertThat(bar).containsExactly(Roshambo.ROCK, Roshambo.PAPER).inOrder(); - assertThat(bar).doesNotContain(Roshambo.SCISSORS);; + assertThat(bar).doesNotContain(Roshambo.SCISSORS); } @Test diff --git a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java index bc9e8f06b6..86ea25d556 100644 --- a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java +++ b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java @@ -72,7 +72,6 @@ public void testMultipleNamesInTheSameString() { .isEqualTo("v3"); } - @SuppressWarnings("unused") private record RecordWithCustomNames( @SerializedName("name") String a, @SerializedName(value = "name1", alternate = {"name2", "name3"}) String b) {} @@ -256,7 +255,6 @@ public void testPrimitiveAdapterNullValue() { .isEqualTo("null is not allowed as value for record component 'aByte' of primitive type; at path $.aByte"); } - @SuppressWarnings("unused") private record RecordWithPrimitives( String aString, byte aByte, short aShort, int anInt, long aLong, float aFloat, double aDouble, char aChar, boolean aBoolean) {} @@ -410,9 +408,7 @@ public void testReflectionFilterBlockInaccessible() { assertThat(gson.fromJson("{\"i\":2}", PublicRecord.class)).isEqualTo(new PublicRecord(2)); } - @SuppressWarnings("unused") private record PrivateRecord(int i) {} - @SuppressWarnings("unused") public record PublicRecord(int i) {} /** diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java index d540f6e7d2..b2b8dc0abf 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java @@ -37,6 +37,7 @@ import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; import java.io.IOException; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.List; import java.util.Locale; @@ -147,10 +148,84 @@ public void testSuperclassTypeAdapterNotInvoked() { } @Test - public void testNullSafeObjectFromJson() { + public void testNullSafeObject() { Gson gson = new Gson(); NullableClass fromJson = gson.fromJson("null", NullableClass.class); assertThat(fromJson).isNull(); + + fromJson = gson.fromJson("\"ignored\"", NullableClass.class); + assertThat(fromJson).isNotNull(); + + String json = gson.toJson(null, NullableClass.class); + assertThat(json).isEqualTo("null"); + + json = gson.toJson(new NullableClass()); + assertThat(json).isEqualTo("\"nullable\""); + } + + /** + * Tests behavior when a {@link TypeAdapterFactory} registered with {@code @JsonAdapter} returns + * {@code null}, indicating that it cannot handle the type and Gson should try a different factory + * instead. + */ + @Test + public void testFactoryReturningNull() { + Gson gson = new Gson(); + + assertThat(gson.fromJson("null", WithNullReturningFactory.class)).isNull(); + assertThat(gson.toJson(null, WithNullReturningFactory.class)).isEqualTo("null"); + + TypeToken> stringTypeArg = new TypeToken>() {}; + WithNullReturningFactory deserialized = gson.fromJson("\"a\"", stringTypeArg); + assertThat(deserialized.t).isEqualTo("custom-read:a"); + assertThat(gson.fromJson("null", stringTypeArg)).isNull(); + assertThat(gson.toJson(new WithNullReturningFactory<>("b"), stringTypeArg.getType())).isEqualTo("\"custom-write:b\""); + assertThat(gson.toJson(null, stringTypeArg.getType())).isEqualTo("null"); + + // Factory should return `null` for this type and Gson should fall back to reflection-based adapter + TypeToken> numberTypeArg = new TypeToken>() {}; + deserialized = gson.fromJson("{\"t\":1}", numberTypeArg); + assertThat(deserialized.t).isEqualTo(1); + assertThat(gson.toJson(new WithNullReturningFactory<>(2), numberTypeArg.getType())).isEqualTo("{\"t\":2}"); + } + // Also set `nullSafe = true` to verify that this does not cause a NullPointerException if the + // factory would accidentally call `nullSafe()` on null adapter + @JsonAdapter(value = WithNullReturningFactory.NullReturningFactory.class, nullSafe = true) + private static class WithNullReturningFactory { + T t; + + public WithNullReturningFactory(T t) { + this.t = t; + } + + static class NullReturningFactory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + // Don't handle raw (non-parameterized) type + if (type.getType() instanceof Class) { + return null; + } + ParameterizedType parameterizedType = (ParameterizedType) type.getType(); + // Makes this test a bit more realistic by only conditionally returning null (instead of always) + if (parameterizedType.getActualTypeArguments()[0] != String.class) { + return null; + } + + @SuppressWarnings("unchecked") + TypeAdapter adapter = (TypeAdapter) new TypeAdapter>() { + @Override + public void write(JsonWriter out, WithNullReturningFactory value) throws IOException { + out.value("custom-write:" + value.t); + } + + @Override + public WithNullReturningFactory read(JsonReader in) throws IOException { + return new WithNullReturningFactory<>("custom-read:" + in.nextString()); + } + }; + return adapter; + } + } } @JsonAdapter(A.JsonAdapter.class) @@ -223,7 +298,6 @@ private static class UserJsonAdapter extends TypeAdapter { out.name("name"); out.value(user.firstName + " " + user.lastName); out.endObject(); - // implement the write method } @Override public User read(JsonReader in) throws IOException { // implement read: split name into firstName and lastName @@ -235,6 +309,7 @@ private static class UserJsonAdapter extends TypeAdapter { } } + // Implicit `nullSafe=true` @JsonAdapter(value = NullableClassJsonAdapter.class) private static class NullableClass { } @@ -606,4 +681,65 @@ public WithJsonDeserializer deserialize(JsonElement json, Type typeOfT, JsonDese } } } + + /** + * Tests creation of the adapter referenced by {@code @JsonAdapter} using an {@link InstanceCreator}. + */ + @Test + public void testAdapterCreatedByInstanceCreator() { + CreatedByInstanceCreator.Serializer serializer = new CreatedByInstanceCreator.Serializer("custom"); + Gson gson = new GsonBuilder() + .registerTypeAdapter(CreatedByInstanceCreator.Serializer.class, (InstanceCreator) t -> serializer) + .create(); + + String json = gson.toJson(new CreatedByInstanceCreator()); + assertThat(json).isEqualTo("\"custom\""); + } + @JsonAdapter(CreatedByInstanceCreator.Serializer.class) + private static class CreatedByInstanceCreator { + static class Serializer implements JsonSerializer { + private final String value; + + @SuppressWarnings("unused") + public Serializer() { + throw new AssertionError("should not be called"); + } + + public Serializer(String value) { + this.value = value; + } + + @Override + public JsonElement serialize(CreatedByInstanceCreator src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(value); + } + } + } + + /** + * Tests creation of the adapter referenced by {@code @JsonAdapter} using JDK Unsafe. + */ + @Test + public void testAdapterCreatedByJdkUnsafe() { + String json = new Gson().toJson(new CreatedByJdkUnsafe()); + assertThat(json).isEqualTo("false"); + } + @JsonAdapter(CreatedByJdkUnsafe.Serializer.class) + private static class CreatedByJdkUnsafe { + static class Serializer implements JsonSerializer { + // JDK Unsafe leaves this at default value `false` + private boolean wasInitialized = true; + + // Explicit constructor with args to remove implicit no-args constructor + @SuppressWarnings("unused") + public Serializer(int i) { + throw new AssertionError("should not be called"); + } + + @Override + public JsonElement serialize(CreatedByJdkUnsafe src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(wasInitialized); + } + } + } } diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java index 20081f68cf..1e4af6a0f9 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java @@ -20,6 +20,7 @@ import com.google.errorprone.annotations.Keep; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import com.google.gson.JsonDeserializationContext; import com.google.gson.JsonDeserializer; import com.google.gson.JsonElement; @@ -27,7 +28,11 @@ import com.google.gson.JsonPrimitive; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; +import com.google.gson.TypeAdapter; import com.google.gson.annotations.JsonAdapter; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; +import java.io.IOException; import java.lang.reflect.Type; import org.junit.Test; @@ -43,7 +48,7 @@ public void testJsonSerializerDeserializerBasedJsonAdapterOnFields() { String json = gson.toJson(new Computer(new User("Inderjeet Singh"), null, new User("Jesse Wilson"))); assertThat(json).isEqualTo("{\"user1\":\"UserSerializer\",\"user3\":\"UserSerializerDeserializer\"}"); Computer computer = gson.fromJson("{'user2':'Jesse Wilson','user3':'Jake Wharton'}", Computer.class); - assertThat(computer.user2.name).isEqualTo("UserSerializer"); + assertThat(computer.user2.name).isEqualTo("UserDeserializer"); assertThat(computer.user3.name).isEqualTo("UserSerializerDeserializer"); } @@ -82,7 +87,7 @@ private static final class UserDeserializer implements JsonDeserializer { @Override public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { - return new User("UserSerializer"); + return new User("UserDeserializer"); } } @@ -178,20 +183,48 @@ private static final class BaseIntegerAdapter implements JsonSerializer() { + @Override + public User read(JsonReader in) throws IOException { + in.nextNull(); + return new User("fallback-read"); + } + @Override + public void write(JsonWriter out, User value) throws IOException { + assertThat(value).isNull(); + out.value("fallback-write"); + } + }) + .serializeNulls() + .create(); + + String json = gson.toJson(new WithNullSafe(null, null, null, null)); + // Only nullSafe=true serializer writes null; for @JsonAdapter with deserializer nullSafe is ignored when serializing + assertThat(json).isEqualTo("{\"userS\":\"UserSerializer\",\"userSN\":null,\"userD\":\"fallback-write\",\"userDN\":\"fallback-write\"}"); + + WithNullSafe deserialized = gson.fromJson("{\"userS\":null,\"userSN\":null,\"userD\":null,\"userDN\":null}", WithNullSafe.class); + // For @JsonAdapter with serializer nullSafe is ignored when deserializing + assertThat(deserialized.userS.name).isEqualTo("fallback-read"); + assertThat(deserialized.userSN.name).isEqualTo("fallback-read"); + assertThat(deserialized.userD.name).isEqualTo("UserDeserializer"); + assertThat(deserialized.userDN).isNull(); + } + + private static final class WithNullSafe { + // "userS..." uses JsonSerializer + @JsonAdapter(value = UserSerializer.class, nullSafe = false) final User userS; + @JsonAdapter(value = UserSerializer.class, nullSafe = true) final User userSN; + + // "userD..." uses JsonDeserializer + @JsonAdapter(value = UserDeserializer.class, nullSafe = false) final User userD; + @JsonAdapter(value = UserDeserializer.class, nullSafe = true) final User userDN; + + WithNullSafe(User userS, User userSN, User userD, User userDN) { + this.userS = userS; + this.userSN = userSN; + this.userD = userD; + this.userDN = userDN; } } } diff --git a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java index 75f50469f7..ae65da9356 100644 --- a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java +++ b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java @@ -17,6 +17,7 @@ package com.google.gson.internal.bind; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.gson.JsonElement; @@ -112,6 +113,45 @@ public void testPrematureClose() throws Exception { } } + @Test + public void testNameAsTopLevelValue() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Did not expect a name"); + + writer.value(12); + writer.close(); + + e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + } + + @Test + public void testNameInArray() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + + writer.beginArray(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + writer.value(12); + e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + writer.endArray(); + + assertThat(writer.get().toString()).isEqualTo("[12]"); + } + + @Test + public void testTwoNames() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + writer.beginObject(); + writer.name("a"); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("a")); + assertThat(e).hasMessageThat().isEqualTo("Did not expect a name"); + } + @Test public void testSerializeNullsFalse() throws IOException { JsonTreeWriter writer = new JsonTreeWriter(); diff --git a/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java b/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java deleted file mode 100644 index 10c9a9a560..0000000000 --- a/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (C) 2016 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.gson.regression; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.gson.Gson; -import com.google.gson.TypeAdapter; -import com.google.gson.TypeAdapterFactory; -import com.google.gson.annotations.JsonAdapter; -import com.google.gson.reflect.TypeToken; -import org.junit.Test; - -public class JsonAdapterNullSafeTest { - private final Gson gson = new Gson(); - - @Test - public void testNullSafeBugSerialize() { - Device device = new Device("ec57803e"); - String unused = gson.toJson(device); - } - - @Test - public void testNullSafeBugDeserialize() { - Device device = gson.fromJson("{'id':'ec57803e2'}", Device.class); - assertThat(device.id).isEqualTo("ec57803e2"); - } - - @JsonAdapter(Device.JsonAdapterFactory.class) - private static final class Device { - String id; - Device(String id) { - this.id = id; - } - - static final class JsonAdapterFactory implements TypeAdapterFactory { - // The recursiveCall in {@link Device.JsonAdapterFactory} is the source of this bug - // because we use it to return a null type adapter on a recursive call. - private static final ThreadLocal recursiveCall = new ThreadLocal<>(); - - @Override public TypeAdapter create(final Gson gson, TypeToken type) { - if (type.getRawType() != Device.class || recursiveCall.get() != null) { - recursiveCall.set(null); // clear for subsequent use - return null; - } - recursiveCall.set(Boolean.TRUE); - return gson.getDelegateAdapter(this, type); - } - } - } -} diff --git a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java index 80bf15f66e..5752876b6f 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java @@ -28,8 +28,6 @@ import java.math.BigDecimal; import java.math.BigInteger; import org.junit.Test; -import java.util.Arrays; -import java.util.List; @SuppressWarnings("resource") public final class JsonWriterTest { @@ -113,20 +111,36 @@ public void testTopLevelValueTypes() throws IOException { } @Test - public void testInvalidTopLevelTypes() throws IOException { + public void testNameAsTopLevelValue() throws IOException { StringWriter stringWriter = new StringWriter(); JsonWriter jsonWriter = new JsonWriter(stringWriter); - assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.value(12); + jsonWriter.close(); + + e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("JsonWriter is closed."); } @Test - public void closeAllObjectsAndTryToAddElements() throws IOException { - JsonWriter jsonWriterForNameAddition = getJsonWriterWithObjects(); - assertThrows(IllegalStateException.class, () -> jsonWriterForNameAddition.name("this_throw_exception_as_all_objects_are_closed")); - jsonWriterForNameAddition.close(); - JsonWriter jsonWriterForValueAddition = getJsonWriterWithObjects(); - assertThrows(IllegalStateException.class, () -> jsonWriterForValueAddition.value("this_throw_exception_as_only_one_top_level_entry")); - jsonWriterForValueAddition.close(); + public void testNameInArray() throws IOException { + StringWriter stringWriter = new StringWriter(); + JsonWriter jsonWriter = new JsonWriter(stringWriter); + + jsonWriter.beginArray(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.value(12); + e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.endArray(); + jsonWriter.close(); + + assertThat(stringWriter.toString()).isEqualTo("[12]"); } @Test @@ -979,33 +993,4 @@ public void testIndentOverwritesFormattingStyle() throws IOException { + "}"; assertThat(stringWriter.toString()).isEqualTo(expected); } - - /** - * This method wites a json object and return a jsonwriter object - * that we can use for the testing purpose - * @return JsonWriter Object with nested object and an array - */ - private JsonWriter getJsonWriterWithObjects() throws IOException { - StringWriter stringWriter = new StringWriter(); - JsonWriter jsonWriter = new JsonWriter(stringWriter); - jsonWriter.beginObject(); - jsonWriter.name("a").value(20); - jsonWriter.name("age").value(30); - - // Start the nested "address" object - jsonWriter.name("address").beginObject(); - jsonWriter.name("city").value("New York"); - jsonWriter.name("country").value("USA"); - jsonWriter.endObject(); // End the nested "address" object - jsonWriter.name("random_prop").value(78); - // Add an array of phone numbers (list of numbers) - List phoneNumbers = Arrays.asList(1234567890, 98989, 9909); - jsonWriter.name("phoneNumbers").beginArray(); - for (Integer phoneNumber : phoneNumbers) { - jsonWriter.value(phoneNumber); - } - jsonWriter.endArray(); // End the array - jsonWriter.endObject(); // End the outer object - return jsonWriter; - } } diff --git a/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java b/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java index bbf541719d..c366a0d129 100644 --- a/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java +++ b/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java @@ -270,7 +270,7 @@ public void parse(char[] data, Document document) throws Exception { } private static class GsonBindParser implements Parser { - private static Gson gson = new GsonBuilder() + private static final Gson gson = new GsonBuilder() .setDateFormat("EEE MMM dd HH:mm:ss Z yyyy") .create(); diff --git a/pom.xml b/pom.xml index 73924ac5e0..3b89b508db 100644 --- a/pom.xml +++ b/pom.xml @@ -100,7 +100,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.3.0 + 3.4.1 enforce-versions @@ -151,7 +151,7 @@ com.google.errorprone error_prone_core - 2.21.1 + 2.22.0 @@ -159,7 +159,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.5.0 + 3.6.0 @@ -299,7 +299,7 @@ com.github.siom79.japicmp japicmp-maven-plugin - 0.17.2 + 0.17.3 diff --git a/shrinker-test/common.pro b/shrinker-test/common.pro index b30faa1329..9995925e9d 100644 --- a/shrinker-test/common.pro +++ b/shrinker-test/common.pro @@ -16,9 +16,9 @@ public static void runTests(...); } -keep class com.example.NoSerializedNameMain { - public static java.lang.String runTest(); + public static java.lang.String runTestNoArgsConstructor(); public static java.lang.String runTestNoJdkUnsafe(); - public static java.lang.String runTestNoDefaultConstructor(); + public static java.lang.String runTestHasArgsConstructor(); } diff --git a/shrinker-test/proguard.pro b/shrinker-test/proguard.pro index ee96073372..7922bc0967 100644 --- a/shrinker-test/proguard.pro +++ b/shrinker-test/proguard.pro @@ -6,12 +6,12 @@ # Unlike R8, ProGuard does not perform aggressive optimization which makes classes abstract, # therefore for ProGuard can successfully perform deserialization, and for that need to # preserve the field names --keepclassmembernames class com.example.NoSerializedNameMain$TestClass { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassNoArgsConstructor { ; } -keepclassmembernames class com.example.NoSerializedNameMain$TestClassNotAbstract { ; } --keepclassmembernames class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassHasArgsConstructor { ; } diff --git a/shrinker-test/r8.pro b/shrinker-test/r8.pro index 01b8c84ee2..642334d56f 100644 --- a/shrinker-test/r8.pro +++ b/shrinker-test/r8.pro @@ -10,8 +10,8 @@ -keep,allowshrinking,allowoptimization,allowobfuscation,allowaccessmodification class com.example.GenericClasses$GenericUsingGenericClass # Don't obfuscate class name, to check it in exception message --keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClass --keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassNoArgsConstructor +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassHasArgsConstructor # This rule has the side-effect that R8 still removes the no-args constructor, but does not make the class abstract -keep class com.example.NoSerializedNameMain$TestClassNotAbstract { diff --git a/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java similarity index 61% rename from shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java rename to shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java index 2af573677a..c65281e08e 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java +++ b/shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java @@ -3,15 +3,15 @@ import com.google.gson.annotations.SerializedName; /** - * Class without no-args default constructor, but with field annotated with + * Class without no-args constructor, but with field annotated with * {@link SerializedName}. */ -public class ClassWithoutDefaultConstructor { +public class ClassWithHasArgsConstructor { @SerializedName("myField") public int i; // Specify explicit constructor with args to remove implicit no-args default constructor - public ClassWithoutDefaultConstructor(int i) { + public ClassWithHasArgsConstructor(int i) { this.i = i; } } diff --git a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java similarity index 52% rename from shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java rename to shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java index 6cff119011..f211daf91c 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java +++ b/shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java @@ -3,14 +3,14 @@ import com.google.gson.annotations.SerializedName; /** - * Class with no-args default constructor and with field annotated with + * Class with no-args constructor and with field annotated with * {@link SerializedName}. */ -public class ClassWithDefaultConstructor { +public class ClassWithNoArgsConstructor { @SerializedName("myField") public int i; - public ClassWithDefaultConstructor() { + public ClassWithNoArgsConstructor() { i = -3; } } diff --git a/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java new file mode 100644 index 0000000000..b0178bf613 --- /dev/null +++ b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java @@ -0,0 +1,19 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class without no-args constructor, but with field annotated with + * {@link SerializedName}. The constructor should not actually be used in the + * code, but this shouldn't lead to R8 concluding that values of the type are + * not constructible and therefore must be null. + */ +public class ClassWithUnreferencedHasArgsConstructor { + @SerializedName("myField") + public int i; + + // Specify explicit constructor with args to remove implicit no-args default constructor + public ClassWithUnreferencedHasArgsConstructor(int i) { + this.i = i; + } +} diff --git a/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java new file mode 100644 index 0000000000..2d43032140 --- /dev/null +++ b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java @@ -0,0 +1,18 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class with no-args constructor and with field annotated with + * {@link SerializedName}. The constructor should not actually be used in the + * code, but this shouldn't lead to R8 concluding that values of the type are + * not constructible and therefore must be null. + */ +public class ClassWithUnreferencedNoArgsConstructor { + @SerializedName("myField") + public int i; + + public ClassWithUnreferencedNoArgsConstructor() { + i = -3; + } +} diff --git a/shrinker-test/src/main/java/com/example/Main.java b/shrinker-test/src/main/java/com/example/Main.java index 488bc03b82..d2d88021e1 100644 --- a/shrinker-test/src/main/java/com/example/Main.java +++ b/shrinker-test/src/main/java/com/example/Main.java @@ -32,7 +32,11 @@ public static void runTests(BiConsumer outputConsumer) { testNamedFields(outputConsumer); testSerializedName(outputConsumer); - testNoDefaultConstructor(outputConsumer); + testConstructorNoArgs(outputConsumer); + testConstructorHasArgs(outputConsumer); + testUnreferencedConstructorNoArgs(outputConsumer); + testUnreferencedConstructorHasArgs(outputConsumer); + testNoJdkUnsafe(outputConsumer); testEnum(outputConsumer); @@ -90,12 +94,57 @@ private static void testSerializedName(BiConsumer outputConsumer }); } - private static void testNoDefaultConstructor(BiConsumer outputConsumer) { + private static void testConstructorNoArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + TestExecutor.run(outputConsumer, "Write: No args constructor", () -> toJson( + gson, new ClassWithNoArgsConstructor())); + TestExecutor.run(outputConsumer, "Read: No args constructor; initial constructor value", () -> { + ClassWithNoArgsConstructor deserialized = fromJson(gson, "{}", ClassWithNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + TestExecutor.run(outputConsumer, "Read: No args constructor; custom value", () -> { + ClassWithNoArgsConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testConstructorHasArgs(BiConsumer outputConsumer) { Gson gson = new GsonBuilder().setPrettyPrinting().create(); - TestExecutor.run(outputConsumer, "Write: No default constructor", () -> toJson(gson, new ClassWithoutDefaultConstructor(2))); + TestExecutor.run(outputConsumer, "Write: Constructor with args", () -> toJson( + gson, new ClassWithHasArgsConstructor(2))); + // This most likely relies on JDK Unsafe (unless the shrinker rewrites the constructor in some way) + TestExecutor.run(outputConsumer, "Read: Constructor with args", () -> { + ClassWithHasArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithHasArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testUnreferencedConstructorNoArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + // No write because we're not referencing this class's constructor. + + // This runs the no-args constructor. + TestExecutor.run(outputConsumer, "Read: Unreferenced no args constructor; initial constructor value", () -> { + ClassWithUnreferencedNoArgsConstructor deserialized = fromJson( + gson, "{}", ClassWithUnreferencedNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + TestExecutor.run(outputConsumer, "Read: Unreferenced no args constructor; custom value", () -> { + ClassWithUnreferencedNoArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithUnreferencedNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testUnreferencedConstructorHasArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + // No write because we're not referencing this class's constructor. + // This most likely relies on JDK Unsafe (unless the shrinker rewrites the constructor in some way) - TestExecutor.run(outputConsumer, "Read: No default constructor", () -> { - ClassWithoutDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithoutDefaultConstructor.class); + TestExecutor.run(outputConsumer, "Read: Unreferenced constructor with args", () -> { + ClassWithUnreferencedHasArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithUnreferencedHasArgsConstructor.class); return Integer.toString(deserialized.i); }); } @@ -103,11 +152,13 @@ private static void testNoDefaultConstructor(BiConsumer outputCo private static void testNoJdkUnsafe(BiConsumer outputConsumer) { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); TestExecutor.run(outputConsumer, "Read: No JDK Unsafe; initial constructor value", () -> { - ClassWithDefaultConstructor deserialized = fromJson(gson, "{}", ClassWithDefaultConstructor.class); + ClassWithNoArgsConstructor deserialized = fromJson( + gson, "{}", ClassWithNoArgsConstructor.class); return Integer.toString(deserialized.i); }); TestExecutor.run(outputConsumer, "Read: No JDK Unsafe; custom value", () -> { - ClassWithDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithDefaultConstructor.class); + ClassWithNoArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithNoArgsConstructor.class); return Integer.toString(deserialized.i); }); } diff --git a/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java index cd70af34a7..a43fc0bdef 100644 --- a/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java +++ b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java @@ -10,7 +10,8 @@ * therefore not matched by the default {@code gson.pro} rules. */ public class NoSerializedNameMain { - static class TestClass { + static class TestClassNoArgsConstructor { + // Has a no-args default constructor. public String s; } @@ -19,37 +20,40 @@ static class TestClassNotAbstract { public String s; } - static class TestClassWithoutDefaultConstructor { + static class TestClassHasArgsConstructor { public String s; // Specify explicit constructor with args to remove implicit no-args default constructor - public TestClassWithoutDefaultConstructor(String s) { + public TestClassHasArgsConstructor(String s) { this.s = s; } } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoArgsConstructor()}. */ - public static String runTest() { - TestClass deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClass.class)); + public static String runTestNoArgsConstructor() { + TestClassNoArgsConstructor deserialized = new Gson().fromJson( + "{\"s\":\"value\"}", same(TestClassNoArgsConstructor.class)); return deserialized.s; } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructorNoJdkUnsafe()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoArgsConstructorNoJdkUnsafe()}. */ public static String runTestNoJdkUnsafe() { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); - TestClassNotAbstract deserialized = gson.fromJson("{\"s\": \"value\"}", same(TestClassNotAbstract.class)); + TestClassNotAbstract deserialized = gson.fromJson( + "{\"s\": \"value\"}", same(TestClassNotAbstract.class)); return deserialized.s; } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoDefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_HasArgsConstructor()}. */ - public static String runTestNoDefaultConstructor() { - TestClassWithoutDefaultConstructor deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassWithoutDefaultConstructor.class)); + public static String runTestHasArgsConstructor() { + TestClassHasArgsConstructor deserialized = new Gson().fromJson( + "{\"s\":\"value\"}", same(TestClassHasArgsConstructor.class)); return deserialized.s; } } diff --git a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java index be5a6213f7..a017c2c33b 100644 --- a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java +++ b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java @@ -129,12 +129,32 @@ public void test() throws Exception { "Read: SerializedName", "3", "===", - "Write: No default constructor", + "Write: No args constructor", + "{", + " \"myField\": -3", + "}", + "===", + "Read: No args constructor; initial constructor value", + "-3", + "===", + "Read: No args constructor; custom value", + "3", + "===", + "Write: Constructor with args", "{", " \"myField\": 2", "}", "===", - "Read: No default constructor", + "Read: Constructor with args", + "3", + "===", + "Read: Unreferenced no args constructor; initial constructor value", + "-3", + "===", + "Read: Unreferenced no args constructor; custom value", + "3", + "===", + "Read: Unreferenced constructor with args", "3", "===", "Read: No JDK Unsafe; initial constructor value", @@ -191,9 +211,9 @@ public void test() throws Exception { } @Test - public void testNoSerializedName_DefaultConstructor() throws Exception { + public void testNoSerializedName_NoArgsConstructor() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { - Method m = c.getMethod("runTest"); + Method m = c.getMethod("runTestNoArgsConstructor"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { Object result = m.invoke(null); @@ -203,7 +223,7 @@ public void testNoSerializedName_DefaultConstructor() throws Exception { Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( "Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" - + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClass" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassNoArgsConstructor" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); } @@ -211,7 +231,7 @@ public void testNoSerializedName_DefaultConstructor() throws Exception { } @Test - public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exception { + public void testNoSerializedName_NoArgsConstructorNoJdkUnsafe() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { Method m = c.getMethod("runTestNoJdkUnsafe"); @@ -232,9 +252,9 @@ public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exceptio } @Test - public void testNoSerializedName_NoDefaultConstructor() throws Exception { + public void testNoSerializedName_HasArgsConstructor() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { - Method m = c.getMethod("runTestNoDefaultConstructor"); + Method m = c.getMethod("runTestHasArgsConstructor"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { Object result = m.invoke(null); @@ -244,7 +264,7 @@ public void testNoSerializedName_NoDefaultConstructor() throws Exception { Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( "Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" - + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassHasArgsConstructor" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); }