Don't silently ignore missing type information from TypeTokens.

[JornVernee]

I'm active on Stack Overflow, and often see questions with code more or less like this:

public class Main {
		
	public static void main(String[] args) {
            // should work right?
	    MyClass mc = new Gson().fromJson("{}", Main.<MyClass>typeTokenHelper());
	}
	
	static class MyClass {}
	
	public static <T> Type typeTokenHelper() {
		return new TypeToken<T>() {}.getType();
	}

}

Of course this fails with:

java.lang.ClassCastException: com.google.gson.internal.LinkedTreeMap cannot be cast to test.Main$MyClass

Since there is not actually any type information available from the TypeToken. The Type that you get is the generic type T:

Type t = Main.<MyClass>typeTokenHelper();
System.out.println(t); // prints 'T'. instead of the naïvely expected 'MyClass'

This is confusing. The missing type information should not be silently ignored only to get a ClassCastException later. The missing type information is caused by a design time error and should be flagged as early as possible.

At first glance a check like this:

// where 'typeOfT' is the type returned by TypeToken::getType
if(typeOfT instanceof TypeVariable) {  // java.lang.reflect.TypeVariable
    throw new RuntimeExcepiton(...);
}

Somewhere might fix this issue. Perhaps in TypeToken::getSuperclassTypeParameter or in Gson::fromJson. (I don't know where the use of TypeTokens with TypeVariables might be required though)

[Marcono1234]

@eamonnmcmanus, can this please be revisited (maybe using the original code from #2072) in one of the next major versions? The author of this issue is right, people keep running into this issue somewhat frequently, and as pointed out in #2072 (comment) this can also cause issues for serialization.

Here is an incomplete list of Stack Overflow questions and answers where users encountered issues due to this:

• https://stackoverflow.com/q/74321188
• https://stackoverflow.com/q/74357747
• https://stackoverflow.com/q/75567439
• https://stackoverflow.com/a/67921091
  Currently this answer erroneously suggests using TypeToken<List<T>>() {}
• https://stackoverflow.com/q/62147697
• https://stackoverflow.com/q/47618965
• https://stackoverflow.com/q/28123978
• https://stackoverflow.com/q/45554148

[eamonnmcmanus]

As I mentioned in the previous discussion, I am concerned that this would invalidate theoretically-incorrect but working code. The scenario I'm imagining is where the original developer is long gone and someone just wants to update the Gson dependency. All of a sudden they get this new exception and they have no idea how to go about fixing it.

As an alternative, could we detect this situation during deserialization and produce a more helpful exception? The exception could even indicate the line where the faulty TypeToken was created.

[Marcono1234]

All of a sudden they get this new exception and they have no idea how to go about fixing it.

We could at least assist the developers by creating an entry in the troubleshooting guide explaining which alternatives exist to solve this (this is already covered to some extent). Maybe we could even directly add the troubleshooting guide URL in the exception message. (I have been thinking about proposing this for multiple Gson exceptions; I might create a separate issue for that.)

We could also add a "Gson unsafe features system property" which allows users to enable unsafe feature again, such as type variable capturing here with gson-typetoken-allows-type-variable=true. Though setting system properties can be annoying sometimes, especially if you have to set it before the Gson classes are loaded (we could avoid that by checking the system property every time) or if other unrelated classes clear the value (should hopefully not occur because these feature properties would all be opt-in; we could also document that clearing the system property is discouraged).

As an alternative, could we detect this situation during deserialization and produce a more helpful exception? The exception could even indicate the line where the faulty TypeToken was created.

Maybe, but we would probably have to rewrite adapter retrieval for that and maybe even introduce a wrapper adapter (subclassing SerializationDelegatingTypeAdapter) which allows serialization but blocks deserialization.
Will capturing the stack trace be expensive (especially if users don't cache the TypeToken)? java.lang.StackWalker was only added in Java 9.

[eamonnmcmanus]

Yeah, I think capturing the stack trace is probably not a good idea. I'm hoping it will usually be fairly obvious where the problem arose, especially since this should usually only happen just after someone has written the problematic code and is testing it.

A system property to turn off the new exception could be an alternative. We could mention the system property in the exception message. I'm a bit concerned that we may start to accumulate a whole bunch of these random system properties, which is why I've proposed the notion of a compatibility level in the past. But if we have to check that level in the TypeToken constructor then it would still have to be a system property or other static state.

Linking to the troubleshooting guide from exception messages seems like an excellent idea in general.