Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved type handling #1753

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Marcono1234
Copy link
Collaborator

@Marcono1234 Marcono1234 commented Aug 14, 2020

Resolves #1219 (by preventing to capture type variables using TypeToken anonymous subclass)

Tries to improve type handling of Gson. Related to #1741 (comment).
Currently still a draft because there are certain further improvements which I am not sure how to implement (listed below). This would probably also have to include the changes done by #1391.

TODO

Type variables

The following type variable logic should also apply to type variables declared by methods and constructors.

Serialization

When serializing should get the bounds of the type variable:

  • if one bound: Should try to use its type. For type variables being part of the bound (e.g. T extends List<U>) apply this logic recursively. However, if there is a cycle, e.g. X extends Comparable<X>, should throw exception.
  • if multiple bounds: Should throw exception. Intersection types cannot be represented using Gson's TypeToken and even if they could it might result in conflicts between different libraries when for example multiple libraries register different CharSequence & Number adapters which behave differently.

Deserialization

When deserializing and the type is not raw should always throw exception because at runtime the type argument for that type variable is likely a subtype of the bound. This would then at unrelated places cause unexpected ClassCastExceptions so it would be better if Gson already failed on deserialization.

If the type is raw then the same logic for resolving the type variable as described in the "Serialization" section above should be used.

Implementation notes

Throwing the exceptions directly on calls to $Gson$Types.resolve(Type, Class<?>, Type) should not be done because the problematic type variable might not be considered by the type adapter. For example for a class MyGenericClass<T>, if the custom adapter for MyGenericClass does not even consider the type variable then it is not an issue that an unresolved type variable exists. So maybe the exception should only be thrown on calls to Gson.getAdapter(TypeToken).

However, the other problem is then to determine when the caller is serializing and when it is deserializing. This could be solved in a somewhat reliable way by having the ReflectiveTypeAdapterFactory resolve the type twice, once for serialization and a second time for deserialization. $Gson$Types.resolve could then return different resolved types which are then detected by Gson.getAdapter and cause it to throw an exception.

Wildcards

Wildcards without or with lower bounds

Wildcards without bounds or with lower bounds should be treated as if the class declaring the type variable for which the wildcard is a type argument is raw, i.e. private List<?> f should be treated like private List f (raw type) as described in the "Type variables" section above.
However, the bound of that wildcard should be resolved with the respective type arguments for the other type variables. For example:

class GenericClass<T, U extends T> { }

...

// Resolved type should be GenericClass<String, String>
private GenericClass<String, ?> f;

Wildcards with upper bounds

Due to JDK-8250936 handling of wildcards with upper bounds is slightly more complicated. First the wildcard should be treated like "Wildcards without or with lower bounds" described above. The resolved type should then be compared with the resolved type of the wildcard bound and the more specific one of them should be used as resolved type.
For example:

class BaseClass { }
class SubClass extends BaseClass { }
class SubSubClass extends SubClass { }
class GenericClass<T extends SubClass> { }

...

// Illogical wildcard bound (JDK-8250936); resolved type should be GenericClass<SubClass>
private GenericClass<? extends BaseClass> f;
// Resolved type should be GenericClass<SubSubClass>
private GenericClass<? extends SubSubClass> f;

class OtherGenericClass<T> {
    private GenericClass<? extends T> f;
}
// Resolved type of OtherGenericClass.f should be GenericClass<String>
private OtherGenericClass<String> f2;

The returned Type is never a wildcard due to the changes made to getSupertype
by commit b1fb9ca.
getRawType(TypeToken.getType()) is the same as calling TypeToken.getRawType().
It is possible to use wildcards as part of the type argument, e.g.:
`new TypeToken<List<? extends CharSequence>>() {}`
Previously subclasses of subclasses (...) of TypeToken were allowed which
can behave incorrectly when retrieving the type argument, e.g.:

  class SubTypeToken<T> extends TypeToken<Integer> {}
  new SubTypeToken<String>() {}.getType()

This returned `String` despite the class extending TypeToken<Integer>.
Due to type erasure the runtime type argument for a type variable is not
available. Therefore there is no point in capturing a type variable and it
might even give a false sense of type-safety.
Rename the method parameter to match the documentation of the method and
to be similar to getSupertype(...).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't silently ignore missing type information from TypeTokens.
1 participant