Very early versions of Gson actually supported inner classes.
What do you expect to happen to the outer class reference? Ignore it silently or serialize it as well?

Note that the topic of this issue is not inner classes but anonymous classes.

Anyway, ignoring the references to the outer class seems fine (imagine a circular dependency otherwise). Gson's documentation already explicitly mentions the behaviour to fields corresponding to outer classes:

Fields corresponding to the outer classes in inner classes, anonymous classes, and local classes are ignored and not included in serialization or deserialization.

If you don't want to implement this, fine, but at least give an exception instead of silently serializing to null. I just lost way too much time debugging some weird NullPointerExceptions until I realized it was due to the use of anonymous classes.

I think deserialization should be possible as an opt-in through the use of custom type adapters.

There should be some option to tell Gson to include the fields of anonymous classes, something like toJson(obj, withAnonymousStuff = true).

Besides general support for serialization of anonymous classes, there are also these special cases for which it would be even more reasonable to allow serialization or deserialization:

• When the user has registered a custom TypeAdapter / TypeAdapterFactory for the type (or a supertype)
• When the user has registered a custom InstanceCreator, and therefore the risk of a NullPointerException after deserialization when accessing the enclosing class does not exist

Personally, I find the proposal mentioned in #1510 (comment) to throw an exception on deserialization quite compelling, but I think it can unfortunately not be easily implemented:

• When implemented with a custom GsonBuilder setting, it might be necessary to differentiate between inner class, anonymous class and local class, and would therefore bloat the API
• When implemented as built-in TypeAdapterFactory which throws the exception on deserialization, the user could not overwrite this behavior by registering a TypeAdapterFactory which delegates to reflection-based deserialization (because delegation would delegate to that throwing adapter factory)
• When implemented with a built-in instance creator (or as part of the internal ConstructorConstructor), it would be somewhat redundant, the proper solution might be to use GsonBuilder.disableJdkUnsafe(). And users could also not disable this behavior / delegate to Unsafe instance creation without a new GsonBuilder setting.
• When implemented with a ReflectionAccessFilter it would not be possible to differentiate between serialization and deserialization

Also, on deserialization trying to detect whether a local or anonymous class uses the enclosing instance does not seem to be possible (until recently, see JDK-8271623), because even when the enclosing instance is not used, the compiler seems to generate a field storing the enclosing instance anyways.

So maybe Gson should just remove restriction for local and anonymous classes, assuming that the users knows what they are doing. This would be backward incompatible, but the question is how many users really rely on Gson ignoring the values for anonymous and local classes.

Users could still achieve the old behavior by specifying an ExclusionStrategy, and as shown above, if users want Gson to throw exceptions, they could use multiple approaches to achieve this.

@eamonnmcmanus, what do you think about dropping this restriction for anonymous and local classes?
(we might need to find out though if there are use cases relying on this behavior)

I'm not terribly motivated to support serializing anonymous classes. It's easy enough to make a named class if you need to serialize it, and I'm concerned that the serialization could turn out to be a can of worms.

I'm more sympathetic to the notion of throwing an exception rather than serializing null. It seems we should be able to tell when we are about to do that? It's potentially incompatible but only for people who have been seeing these mysterious null values in their JSON and not cared. We could maybe have a GsonBuilder option or even system property for people like that.

We could maybe have a GsonBuilder option or even system property for people like that.

That might not even be necessary, they can probably just register an ExclusionStrategy which behaves like the current built-in exclusion logic for local and anonymous classes.

Thanks for PR #2189! That enabled me to see the effect of throwing an exception here, by running the change against all of Google's internal tests. (We have thousands of non-test source files that reference the Gson API.) I found one place where a field with this anonymous Ticker subclass newly causes an exception; two places where tests were explicitly expecting null for an anonymous class; one place where the new exception was caught and serialized as a potentiallyIncompleteData property in the containing JSON object, triggering a golden-file comparison failure; and one place where a serialized listener caused hundreds of test failures. While all of these are fixable, it makes me concerned that we could be breaking users in hard-to-diagnose ways. It should be possible for people to upgrade to the latest Gson without having to solve this sort of problem.

The main alternative seems to be to support serializing anonymous classes in at least some cases, as alluded to by @Marcono1234 above. I'm afraid that would also cause similar problems, though, unless we introduced yet another GsonBuilder option.

We have seen a number of cases like this where we want to do the right thing but are hampered by the risk of breaking existing users. I am wondering if we should maybe introduce the notion of a "compatibility level". It might be an enum, with constants like L1, L2, and a special value LATEST. In your GsonBuilder you could say that you want the compatibility level that is current at the time you write your code, say L2. Then you benefit from any potentially-incompatible fixes that were current in L2, but not any later ones. Or you could say LATEST if you are prepared to fix any problems from future incompatible changes. If you say nothing then you get L0, which means keeping all the ugly behaviours we have today. (Perhaps the constants could have names like V2_9_1 instead or as well.) This scheme would avoid the combinatorial explosion induced by having one GsonBuilder option for every incompatible change.

I found one place where a field with this anonymous Ticker subclass newly causes an exception

This also highlights other flaws with the current behavior:

• Users relying on fields being excluded because they have anonymous or local class values might unintentionally serialize data when they refactor their code to not use anonymous or local classes anymore
• The field is only excluded for serialization; it is (unless the field type is a local class) still deserialized

I am wondering if we should maybe introduce the notion of a "compatibility level"

I guess that would be possible, but I am a bit afraid that this could turn into a maintainability and troubleshooting nightmare. The effect of the current GsonBuilder methods is limited to a certain area, whereas these compatibility levels might affect all kinds of areas (and would still have to work properly in combination with the other GsonBuilder methods?).
Maybe it is a good idea to solve this, but I don't really know.

Its interesting to see why the decision to not serialize anonymous classes was taken in the first place.

As many others I just found this issue indirectly, when I used nimbus-jose-jwt library that uses Gson internally to serialize JWT token claims.

It happened that my claims contained scope claim that is Set<String>. Nothing special, right? Imagine my confusion when I seen that the serialization result is null. After some hours of debugging I realized that the reason is that some code in a different place uses Guava's Sets.intersection(..) utility to create a Set. Like this:

Set<String> filteredScopes = Sets.intersection(requestedScopes, allowedScopes);

As soon as I added the result to a new HashSet<String>, the serialization started to work as expected.

The main problem I see here is that the serialization result depends on the conditions not always controllable by a calling party. E.g. I didn't know until now that the result of Sets.intersection is anonymous class. And why should I care? It is Collection and serialization of collections is supported. It is very strange that supported depends on the implementation of the Collection. As a library user, I cannot always know/control what implementation of the Collection I'm passing to the library. Especially if I do not use the library directly, like it was in my case.

The fact that when collection implementation is not supported, this error is silently ignored, was also surprising, but I don't think this problem should be solved by changing the behavior to throwing an exception. I think the solution should be to allow serialization of any implementation of supported interfaces by default.

It could be considered as a breaking change, but here we are returning to my first question: why the decision to not serialize anonymous classes was taken in the first place? Is it really a deliberate decision or just overlooking? Are there any reason for people to rely on this specific behavior? If not, then this breaking change will only "break" some very rare cases anyway and I would rather consider it as a "bug fix".

The comment from @xak2000 is interesting. We probably want to continue not trying to serialize anonymous classes via reflection. But in cases where another TypeAdapterFactory takes precedence over ReflectiveTypeAdapterFactory, we shouldn't care whether the class is anonymous. In the initial example (Sets.union) and this one (Sets.intersection), CollectionTypeAdapterFactory would handle those anonymous classes perfectly well. So perhaps we can find a way to reorganize the logic so this can happen. We'd still want deserialization to exclude anonymous classes.