FasterXML / java-classmate

Library for introspecting generic type information of types, member/static methods, fields. Especially useful for POJO/Bean introspection.
http://fasterxml.com
Apache License 2.0
258 stars 42 forks source link

Allow for subtype resolution with unknown generics #53

Open CarstenWickner opened 4 years ago

CarstenWickner commented 4 years ago

I've been using classmate for type resolutions within my victools/jsonschema-generator library.

Unfortunately, I've hit a snag. It's this particular piece of code in the TypeResolver (lines 288-299):

for (int i = 0; i < paramCount; ++i) {
    ResolvedType t = placeholders[i].actualType();
    // Is it ok for it to be left unassigned? For now let's not allow that
    // 18-Oct-2017, tatu: Highly likely that we'll need to allow this, substitute with "unknown" --
    //    had to do that in Jackson. Occurs when subtype is generic, with "bogus" type declared
    //    but not bound in supertype(s). But leaving checking in for now.
    if (t == null) {
        throw new IllegalArgumentException("Failed to find type parameter #"+(i+1)+"/"
                +paramCount+" for "+subtype.getName());
    }
    typeParams[i] = t;
}

Short term: I'm wondering what exactly I can do on my side short of copy-pasting the whole resolveSubtype() method and leaving out this unfortunate exception being thrown? Can I somehow provide extra type bindings (even if they are just Object.class) to make it work?

Mid term: could there be some kind of configuration option on the type resolver to simply set it to (the resolved version of) Object.class or some lower bounds of the generic (if that's not already happening) instead of throwing an exception? How did you handle this in Jackson (as per the above in-line comment)?

cowtowncoder commented 4 years ago

I think addition of configuration option would be the best option here? Very short-term: if you can figure out a refactoring that would allow simple sub-classing of relevant component (so you can override it), I'd be happy to merge a PR.

In Jackson I think a simple "unknown type" (~= Object.class) is just quietly added in such cases.

takanuva15 commented 10 months ago

I know this is a bit late, but I've been running into an issue where I have some classes that implement Comparator but a few don't provide the generic type parameter (The code is legacy from another jar which I don't have the ability to fix currently.)

So when I go to look up the classes implementing Comparator within my package, it throws an IndexOutOfBoundsException in this method within TypeResolver.java:

    private void _resolveTypePlaceholders(ResolvedType sourceType, ResolvedType actualType) throws IllegalArgumentException {
        List<ResolvedType> expectedTypes = sourceType.getTypeParameters();
        List<ResolvedType> actualTypes = actualType.getTypeParameters(); //exception here because no Comparator type specified
        int i = 0;

        for(int len = expectedTypes.size(); i < len; ++i) {
            ResolvedType exp = (ResolvedType)expectedTypes.get(i);
            ResolvedType act = (ResolvedType)actualTypes.get(i);
            if (!this._verifyAndResolve(exp, act)) {
                throw new IllegalArgumentException("Type parameter #" + (i + 1) + "/" + len + " differs; expected " + exp.getBriefDescription() + ", got " + act.getBriefDescription());
            }
        }

    }

By default, Comparator has a generic parameter of Object, but my model class has no generic type, so this line throws the exception because there is no actualType to get: ResolvedType act = (ResolvedType)actualTypes.get(i);

Does this issue match what you guys are talking about here?

cowtowncoder commented 10 months ago

@takanuva15 Would it be possible to include type declarations needed for reproduction? Definitely there should never be ArrayIndexOutOfBoundsException and it might be an easy enough fix. Not sure if this is the same problem as reported here originally but sounds worth fixing regardless.

takanuva15 commented 10 months ago

It was my own custom class implementing Comparator like class MyComparator implements Comparator, but there was no type specified for the generic parameter of Comparator (it was legacy code). I got rid of the issue by moving the Comparators to a different package so it wasn't included in the subtype lookup code for the package I was operating on. In any case, if it comes up again as a blocker I'll try to post more specific code.

cowtowncoder commented 10 months ago

Thank you @takanuva15 ! Maybe I can reproduce it easily with that.

cowtowncoder commented 10 months ago

@takanuva15 Ok, so I can not quite reproduce exception with type like:

    @SuppressWarnings("rawtypes")
    static abstract class Comparator53 implements Comparator { }

but I do see a problem, since this:

        ResolvedType rt = typeResolver.resolve(Comparator53.class);
        List<ResolvedType> params = rt.typeParametersFor(Comparator.class);

returns an empty List, instead of one with 1 entry (for java.lang.Object). So that at least seems wrong. Although not sure quite easy it'll be to fix.

But I would also like to see the actual failure you see; whether it is due to calling some other method(s) on ResolvedType, or if type declaration is different.

cowtowncoder commented 10 months ago

Added a failing case for problem I did find. Still hoping to get another reproduction.