Fix $Gson$Types equals method for TypeVariable when its generic declaration is not a Class

[d-william]

Purpose

When $Gson$Types equals method check two TypeVariable, it can return false while Objects#equals(Object, Object) returns true.

Description

public class Main {

    private static class MyClass {

        public <T> T method() { return null; }

    }

    public static void main(String[] args) throws NoSuchMethodException {
        Method m1 = MyClass.class.getMethod("method");
        Method m2 = MyClass.class.getMethod("method");

        Type rt1 = m1.getGenericReturnType();
        Type rt2 = m2.getGenericReturnType();

        TypeToken<?> t1 = TypeToken.get(rt1);
        TypeToken<?> t2 = TypeToken.get(rt2);

        System.out.println(Objects.equals(rt1, rt2)); // true
        System.out.println(Objects.equals(t1, t2));  // false
    }

}

The root cause is here.
For Class generic declaration, reference equality is enough because there is one reference of each class.
But for Methodgeneric declaration, it is not, Class#getMethod(String, Class<?>...) returns a new instance of the Method per call.

Checklist

• New code follows the Google Java Style Guide
  This is automatically checked by mvn verify, but can also be checked on its own using mvn spotless:check.
  Style violations can be fixed using mvn spotless:apply; this can be done in a separate commit to verify that it did not cause undesired changes.
• If necessary, new public API validates arguments, for example rejects null
• New public API has Javadoc
  • Javadoc uses @since $next-version$
    ($next-version$ is a special placeholder which is automatically replaced during release)
• If necessary, new unit tests have been added
  • Assertions in unit tests use Truth, see existing tests
  • No JUnit 3 features are used (such as extending class TestCase)
  • If this pull request fixes a bug, a new test was added for a situation which failed previously and is now fixed
• mvn clean verify javadoc:jar passes without errors

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Marcono1234]

Thanks for this pull request, and for fixing this bug!

Could you please add a unit test similar to what you have shown in your example code (probably either to GsonTypesTest or TypeTokenTest)? I assume you can add this dummy class with generic method as local class within the test method, so that it does not have to be directly inside the enclosing test class.

Also, out of curiosity, how did you notice this bug? Creating TypeTokens which capture type variables is discouraged and will be disallowed in future Gson versions (see #2376). But obtaining it in the way shown in your example will still work (because it provides no false sense of type safety).

[d-william]

I should be able to add the tests you request in a few days :)

To answer your question, I am implementing JSON objects representing method signature with the type information of a potentially declaring generic class.

class SimpleClass {

    public void method1(String str) {...} 

} 

Will give :

{
    "name" : "method1", 
    "parameters" : [
        {
            "name" : "str",
            "type" : "java.lang.String"
        } 
    ] 
} 

class GenericClass<T> {

    public void method2(T value) {...} 

} 

GenericClass<Number> will give :

{
    "name" : "method2", 
    "parameters" : [
        {
            "name" : "value",
            "type" : "java.lang.Number"
        } 
    ]  
} 

class OtherClass {

    public <T> void method3(T param) {...} 

} 

Will give :

{
    "name" : "method3", 
    "parameters" : [
        {
            "name" : "param",
            "type" : { "name" : "T", "declaring" : "package.OtherClass#method3..." }
        } 
    ] 
} 

And with POJO of this JSON objects, I am led to check equality between them.
I encountered this problem when trying with two objects from scenario 3, because internally, parameter types use TypeToken.

[Marcono1234]

Thanks for the explanation. I guess Gson's TypeToken can be used for that use case, but it is mainly designed to be used with the Gson API. Maybe it would be better to use a more general "type token" class, such as Guava's com.google.common.reflect.TypeToken which also offers more functionality. What do you think?

Nonetheless, the bug here with the Gson TypeToken should still be fixed.

[d-william]

Test added :)

[Marcono1234]

Thanks a lot! The changes look good to me.

It might take some time though until this is merged because I think the direct members of this project are quite busy currently.

[Marcono1234]

Could you please address the build failures though? It probably suffices to place @SuppressWarnings({"unused", "TypeParameterUnusedInFormals"}) (or {"UnusedMethod", "UnusedVariable", "TypeParameterUnusedInFormals}") on TypeVariableTest respectively its constructor and method.