Fix type resolution for static methods (regression in 2.11.3)

[lukasz-walkiewicz]

Updating Jackson (2.10.4 => 2.11.3) in https://github.com/prestosql/presto introduced a regression in types resolution for factory methods, ie. type reference is being dropped for static methods.
Given simplified scenario:

class Wrapper<T> {
  List<T> values;

  static <T> Wrapper<T> fromValues(List<T> values) {
    return new Wrapper<>(values);
  }
}

class Value {
  int x;
}

Wrapper<Value> src = new Wrapper<>(Arrays.asList(new Value(1), new Value(2)));
Wrapper<Value> output = MAPPER.readValue(MAPPER.writeValueAsString(src), new TypeReference<Wrapper<Value>>() {});

out contains values of type LinkedHashMap produced by UntypedObjectDeserializer.

I think it's due to changes introduced in 1014493.

[cowtowncoder]

Please note that T here is NOT related to type variable T for class, and as such can not realistically be used as a creator method. It just happens to have same name, but being static method is not part of the class or bindings in that sense.

[cowtowncoder]

Unfortunately usage as indicated is wrong: static methods type variable is not bound to class at all, even if name happens to be same. For static methods T would come from caller and bound effectively by compiler, but not be available at run time.

If this has seemingly worked before that is unfortunate but I don't think such usage such "work" in the sense indicated.

[cowtowncoder]

Hmmmh. Actually, thinking about this bit more -- while it is, fundamentally, assuming something that Java does not do (implicit linkage), the fact that behavior has been such that it has worked means that usage may be widespread.
One big, big challenge is just that even if it was used, nothing in databind test suite tests for it so it is not supported usage.

Still, I'll have to think about this as I think it has been reported wrt Immutables as well.

[carterkozak]

Hi @lukasz-walkiewicz, I've attempted a fix which does not reintroduce bugs here: #2895. Any chance you could take a look to make sure I haven't missed relevant test cases?

[cowtowncoder]

I resolved the issue with bit more localized patch, added modified unit test.
Will hopefully find a better resolution for 2.12 but trying to minimize changes for 2.11.x at this point.

[lukasz-walkiewicz]

@cowtowncoder Thank you very much for taking care of this. As you said, this "misuse" of type resolution mechanism is pretty widespread (eg. in Presto) so dropping it without prior notice would refrain many users from updating.

[cowtowncoder]

@lukasz-walkiewicz yes, agreed. It's just too bad there were no unit tests for this usage, leading to failures.

[carterkozak]

I'd be happy to submit additional test cases for this behavior based on uses in a few internal codebases :-)

[cowtowncoder]

@carterkozak that'd be great! As I said, for 2.11 I'd hope to keep things that worked working; and we can work on #2895 wrt 2.12 improvements.