Throw exception when serializing or deserializing anonymous or local classes #2189
Conversation
…classes Backward incompatible changes: - Previously Gson did not throw an exception but simply used null; this was pretty error-prone and difficult to debug for users - Previously Gson ignored whether the type had a custom adapter registered; now Gson only throws an exception when no adapter has been registered and reflection would be used
| @@ -155,7 +155,8 @@ BagOfPrimitives obj2 = gson.fromJson(json, BagOfPrimitives.class); | |||
| * While serializing, a null field is omitted from the output. | |||
| * While deserializing, a missing entry in JSON results in setting the corresponding field in the object to its default value: null for object types, zero for numeric types, and false for booleans. | |||
| * If a field is _synthetic_, it is ignored and not included in JSON serialization or deserialization. | |||
| * Fields corresponding to the outer classes in inner classes, anonymous classes, and local classes are ignored and not included in serialization or deserialization. | |||
| * Fields corresponding to the outer classes in inner classes are ignored and not included in serialization or deserialization. | |||
| * Reflection-based serialization and deserialization of anonymous and local classes is not supported; an exception will be thrown. Convert the classes to `static` nested classes or register a custom `TypeAdapter` for them to enable serialization and deserialization. | |||
|
|
|||
| ### <a name="TOC-Nested-Classes-including-Inner-Classes-"></a>Nested Classes (including Inner Classes) | |||
|
|
|||
There was a problem hiding this comment.
The notes below for non-static inner classes a few lines below are a bit incorrect, should they be adjusted?
| @@ -291,7 +291,20 @@ public GsonBuilder enableComplexMapKeySerialization() { | |||
| } | |||
|
|
|||
| /** | |||
| * Configures Gson to exclude inner classes during serialization. | |||
| * Configures Gson to exclude inner classes (= non-{@code static} nested classes) during serialization | |||
There was a problem hiding this comment.
Interestingly this method affects both serialization and deserialization.
| } | ||
|
|
||
| @Override public void write(JsonWriter out, T value) throws IOException { | ||
| throw new UnsupportedOperationException("Serialization of anonymous or local class " + type.getName() + " is not supported. " |
There was a problem hiding this comment.
Not sure if UnsupportedOperationException fits well here. Ideally we should really create a proper exception hierarchy for Gson at some point.
At least the exact exception type is not specified so I guess we can change it later, if necessary.
| public class FieldExclusionTest extends TestCase { | ||
| public class InnerClassesTest extends TestCase { |
There was a problem hiding this comment.
Renamed this because it only seems to cover inner classes.
| } catch (JsonSyntaxException e) { | ||
| fail("Unexpected exception; test has to be run with `--illegal-access=deny`"); |
There was a problem hiding this comment.
Not directly related to this PR, but it makes test failures for this test method when not running with --illegal-access=deny easier to understand.
|
Thanks for this! I commented at length in #1510. In short, I don't think we'll be able to make this change as is. |
Resolves #1510 (but does not implement that feature request)
Backward incompatible changes:
null; this was pretty error-prone and difficult to debug for users.With these changes it is still possible to achieve this behavior by registering a custom
ExclusionStrategywhich excludes anonymous and local classes.This behavior seems reasonable because the issue with anonymous and local classes exists only for reflection-based serialization and deserialization. For what it's worth, this now also supports serializing anonymous classes created with "double brace initialization" (let's not discuss here though whether that concept is a good idea).
Note that this differs from the intended behavior shown by the tests added by b0531e1. Maybe it wasn't even explicitly intended this way back then, but just happened to behave like this because annonymous and local class handling was implemented for the
Excluder.