Include array element types in equality and hashing

VERSION.txt

@@ -3,6 +3,25 @@ Java ClassMate project: licensed under Apache License 2.0
 
 Release notes:
 
+1.8.0 (not yet released)
+
+#125: Generic array types compare equal and collide in the resolver cache
+ (reported, fix contributed by 박재홍, @PHJ2000)
+- Array types no longer retain type bindings of the enclosing type; equality
+  of `ResolvedArrayType` now includes element type (self-referential element
+  types are compared by erased type and type bindings)
+- Raw types (like raw `Map` field, or raw supertype) no longer retain type bindings
+  of the enclosing type: type parameters resolve to their bounds instead
+  (f.ex raw `Map` within `Holder<String>` is now `Map<Object,Object>`, not `Map<String>`)
+- `TypeResolver.resolve(TypeBindings, Type)` no longer applies given bindings
+  to a `Class` passed as `Type` (f.ex bindings of `List<String>` with `List.class`
+  now result in `List<Object>`); use `resolve(Type, Type...)` instead
+- Array types are cached by `TypeResolver` using element type as part of the key
+- Types with `TypePlaceHolder`s (from `resolveSubtype()`) or self-references
+  (`ResolvedRecursiveType`) in their type parameters (at any level of nesting,
+  including array element types) are no longer cached (but see #128 for
+  self-references via supertypes)
+
 1.7.3 (02-Jan-2026)
 
 #117: Regression in 1.7.2 (wrt #53) causes `StackOverflowError` for some recursive types

src/main/java/com/fasterxml/classmate/TypeBindings.java

@@ -3,6 +3,9 @@
 import java.lang.reflect.TypeVariable;
 import java.util.*;
 
+import com.fasterxml.classmate.types.ResolvedRecursiveType;
+import com.fasterxml.classmate.types.TypePlaceHolder;
+
 /**
  * Helper class used for storing binding of local type variables to
  * matching resolved types, in context of a single class.
@@ -33,6 +36,14 @@ public final class TypeBindings
     private final String[] _unboundVariables;
 
     private final int _hashCode;
+
+    /**
+     * Whether any of bound types is (or contains) a type only valid within
+     * resolution context: {@link TypePlaceHolder} or {@link ResolvedRecursiveType}.
+     *
+     * @since 1.8
+     */
+    private final boolean _hasContextualTypes;
 
     /*
     /**********************************************************************
@@ -48,11 +59,24 @@ private TypeBindings(String[] names, ResolvedType[] types, String[] uvars)
             throw new IllegalArgumentException("Mismatching names ("+_names.length+"), types ("+_types.length+")");
         }
         int h = 1;
+        boolean contextual = false;
         for (int i = 0, len = _types.length; i < len; ++i) {
             h += _types[i].hashCode();
+            contextual = contextual || isContextual(_types[i]);
         }
         _unboundVariables = uvars;
         _hashCode = h;
+        _hasContextualTypes = contextual;
+    }
+
+    // Copy-constructor for changing unbound variables only
+    private TypeBindings(TypeBindings base, String[] uvars)
+    {
+        _names = base._names;
+        _types = base._types;
+        _unboundVariables = uvars;
+        _hashCode = base._hashCode;
+        _hasContextualTypes = base._hasContextualTypes;
     }
 
     public static TypeBindings emptyBindings() {
@@ -108,7 +132,7 @@ public TypeBindings withUnboundVariable(String name)
         String[] names =  (len == 0)
                 ? new String[1] : Arrays.copyOf(_unboundVariables, len+1);
         names[len] = name;
-        return new TypeBindings(_names, _types, names);
+        return new TypeBindings(this, names);
     }
 
     /*
@@ -130,6 +154,41 @@ public ResolvedType findBoundType(String name)
         return null;
     }
 
+    /**
+     * Method for checking whether any of bound types is, or contains (as type
+     * parameter or array element type, at any level of nesting), a type that is only
+     * valid within resolution context ({@link TypePlaceHolder} for sub-type resolution,
+     * {@link ResolvedRecursiveType} for self-references): types with such bindings
+     * should not be cached.
+     *
+     * @since 1.8
+     */
+    boolean hasContextualTypes() {
+        return _hasContextualTypes;
+    }
+
+    /**
+     * Helper method for checking whether given type is, or contains, a type only
+     * valid within resolution context; see {@link #hasContextualTypes()}.
+     *
+     * @since 1.8
+     */
+    static boolean isContextual(ResolvedType type)
+    {
+        while (type != null) {
+            if ((type instanceof TypePlaceHolder) || (type instanceof ResolvedRecursiveType)) {
+                return true;
+            }
+            // Array types have no bindings of their own, need to check element type
+            if (!type.isArray()) {
+                // nested bindings were checked when constructed so no need to recurse
+                return type.getTypeBindings().hasContextualTypes();
+            }
+            type = type.getArrayElementType();
+        }
+        return false;
+    }
+
     public boolean isEmpty() {
         return (_types.length == 0);
     }

src/main/java/com/fasterxml/classmate/TypeResolver.java

@@ -3,6 +3,7 @@
 import java.io.Serializable;
 import java.lang.reflect.*;
 import java.util.*;
+import java.util.function.Supplier;
 
 import com.fasterxml.classmate.types.*;
 import com.fasterxml.classmate.util.ClassKey;
@@ -172,11 +173,7 @@ public ResolvedType resolve(Type type, Type... typeParameters)
     public ResolvedArrayType arrayType(Type elementType)
     {
         ResolvedType resolvedElementType = resolve(TypeBindings.emptyBindings(), elementType);
-        // Arrays are cumbersome for some reason:
-        Object emptyArray = Array.newInstance(resolvedElementType.getErasedType(), 0);
-        // Should we try to use cache? It's bit tricky, so let's not bother yet
-        return new ResolvedArrayType(emptyArray.getClass(), TypeBindings.emptyBindings(),
-                resolvedElementType);
+        return _arrayOf(_arrayClassFor(resolvedElementType), resolvedElementType);
     }
 
     /**
@@ -187,6 +184,13 @@ public ResolvedArrayType arrayType(Type elementType)
      * are doing!); but if used, type bindings passed should come from {@link ResolvedType}
      * instance of declaring class (or interface).
      *<p>
+     * NOTE: bindings are only used for resolving type variables (like {@code T}
+     * or {@code List<T>}); they are NOT applied to {@link java.lang.Class} (raw type)
+     * passed as {@code jdkType} itself: so passing bindings of {@code List<String>} with
+     * {@code List.class} results in {@code List<Object>}. To construct parameterized
+     * types, use {@link #resolve(Type, Type...)} instead.
+     * (behavior changed in 1.8, see [classmate#125])
+     *<p>
      * NOTE: order of arguments was reversed for 0.8, to avoid problems with
      * overload varargs method.
      */
@@ -327,7 +331,10 @@ public static boolean isSelfReference(ResolvedType type)
     private ResolvedType _fromAny(ClassStack context, Type mainType, TypeBindings typeBindings)
     {
         if (mainType instanceof Class<?>) {
-            return _fromClass(context, (Class<?>) mainType, typeBindings);
+            // [classmate#125]: a Class here is a raw (or non-generic) type reference;
+            // bindings of the enclosing context are not its own and must not be used
+            // (otherwise raw `Map` within `Holder<String>` would become `Map<String>`)
+            return _fromClass(context, (Class<?>) mainType, TypeBindings.emptyBindings());
         }
         if (mainType instanceof ParameterizedType) {
             return _fromParamType(context, (ParameterizedType) mainType, typeBindings);
@@ -358,6 +365,12 @@ private ResolvedType _fromClass(ClassStack context, Class<?> rawType, TypeBindin
         if (type != null) {
             return type;
         }
+        // [classmate#125]: Arrays have no type parameters of their own, so bindings
+        // must not be retained (nor used for element type)
+        if (rawType.isArray()) {
+            return _arrayOf(rawType, _fromClass(context, rawType.getComponentType(),
+                    TypeBindings.emptyBindings()));
+        }
         // Second: recursive reference?
         if (context == null) {
             context = new ClassStack(rawType);
@@ -374,20 +387,31 @@ private ResolvedType _fromClass(ClassStack context, Class<?> rawType, TypeBindin
         }
 
         // If not, already recently resolved?
-        ResolvedType[] typeParameters = typeBindings.typeParameterArray();
-        ResolvedTypeKey key = _resolvedTypes.key(rawType, typeParameters);
         // 25-Oct-2015, tatu: one twist; if any TypePlaceHolders included, key will NOT be created,
         //   which means that caching should not be used (since type is mutable)
-        if (key == null) {
-            type = _constructType(context, rawType, typeBindings);
-        } else {
-            type = _resolvedTypes.find(key);
-            if (type == null) {
-                type = _constructType(context, rawType, typeBindings);
+        // [classmate#125]: same for nested placeholders, self-references (only valid
+        //   within resolution context)
+        ResolvedTypeKey key = typeBindings.hasContextualTypes() ? null
+                : _resolvedTypes.key(rawType, typeBindings.typeParameterArray());
+        final ClassStack typeContext = context;
+        type = _findOrConstruct(key, () -> _constructType(typeContext, rawType, typeBindings));
+        context.resolveSelfReferences(type);
+        return type;
+    }
+
+    /**
+     * Helper method for finding cached type with given key, if any; or if not,
+     * constructing and caching it. If key is null, type is constructed but not cached.
+     */
+    private ResolvedType _findOrConstruct(ResolvedTypeKey key, Supplier<ResolvedType> constructor)
+    {
+        ResolvedType type = (key == null) ? null : _resolvedTypes.find(key);
+        if (type == null) {
+            type = constructor.get();
+            if (key != null) {
                 _resolvedTypes.put(key, type);
             }
         }
-        context.resolveSelfReferences(type);
         return type;
     }
 
@@ -402,7 +426,8 @@ private ResolvedType _fromGenericType(ClassStack context, GenericType<?> generic
          * we better resolve the whole thing; then dig out
          * type parameterization...
          */
-        ResolvedType type = _fromClass(context, generic.getClass(), typeBindings);
+        // [classmate#125]: GenericType sub-class does not use bindings of enclosing context
+        ResolvedType type = _fromClass(context, generic.getClass(), TypeBindings.emptyBindings());
         ResolvedType genType = type.findSupertype(GenericType.class);
         if (genType == null) { // sanity check; shouldn't occur
             throw new IllegalArgumentException("Unparameterized GenericType instance ("+generic.getClass().getName()+")");
@@ -418,32 +443,23 @@ private ResolvedType _fromGenericType(ClassStack context, GenericType<?> generic
     private ResolvedType _constructType(ClassStack context, Class<?> rawType, TypeBindings typeBindings)
     {
         // Ok: no easy shortcut, let's figure out type of type...
-        if (rawType.isArray()) {
-            ResolvedType elementType = _fromAny(context, rawType.getComponentType(), typeBindings);
-            return new ResolvedArrayType(rawType, typeBindings, elementType);
-        }
+        // (note: array types handled by `_fromClass()`)
         final TypeVariable<?>[] rawTypeParameters = rawType.getTypeParameters();
         // [classmate#53]: Handle raw generic types - resolve type parameters to their bounds
-        if (typeBindings.isEmpty()) {
-            if (rawTypeParameters.length > 0) {
-                ResolvedType[] types = new ResolvedType[rawTypeParameters.length];
-                for (int i = 0; i < rawTypeParameters.length; ++i) {
-                    // Resolve each type parameter to its bound (similar to _fromVariable)
-                    TypeVariable<?> var = rawTypeParameters[i];
-                    String name = var.getName();
-                    // Avoid self-reference cycles by marking as unbound during resolution
-                    TypeBindings tempBindings = typeBindings.withUnboundVariable(name);
-                    Type[] bounds = var.getBounds();
-                    types[i] = _fromAny(context, bounds[0], tempBindings);
-                }
-                typeBindings = TypeBindings.create(rawType, types);
-            }
-        } else {
-            // Work-around/fix for [classmate#33]: if the type has no type parameters,
-            // don't include typeBindings in the ResolvedType
-            if (rawTypeParameters.length == 0) {
-                typeBindings = TypeBindings.emptyBindings();
+        // (note: [classmate#33] work-around for non-empty bindings of non-generic types
+        // no longer needed as of [classmate#125]: such bindings are never passed)
+        if (typeBindings.isEmpty() && (rawTypeParameters.length > 0)) {
+            ResolvedType[] types = new ResolvedType[rawTypeParameters.length];
+            for (int i = 0; i < rawTypeParameters.length; ++i) {
+                // Resolve each type parameter to its bound (similar to _fromVariable)
+                TypeVariable<?> var = rawTypeParameters[i];
+                String name = var.getName();
+                // Avoid self-reference cycles by marking as unbound during resolution
+                TypeBindings tempBindings = typeBindings.withUnboundVariable(name);
+                Type[] bounds = var.getBounds();
+                types[i] = _fromAny(context, bounds[0], tempBindings);
             }
+            typeBindings = TypeBindings.create(rawType, types);
         }
         // For other types super interfaces are needed...
         if (rawType.isInterface()) {
@@ -508,10 +524,29 @@ private ResolvedType _fromParamType(ClassStack context, ParameterizedType ptype,
 
     private ResolvedType _fromArrayType(ClassStack context, GenericArrayType arrayType, TypeBindings typeBindings)
     {
+        // [classmate#125]: bindings only needed for element type, not retained by array
         ResolvedType elementType = _fromAny(context, arrayType.getGenericComponentType(), typeBindings);
+        return _arrayOf(_arrayClassFor(elementType), elementType);
+    }
+
+    private static Class<?> _arrayClassFor(ResolvedType elementType) {
         // Figuring out raw class for generic array is actually bit tricky...
-        Object emptyArray = Array.newInstance(elementType.getErasedType(), 0);
-        return new ResolvedArrayType(emptyArray.getClass(), typeBindings, elementType);
+        return Array.newInstance(elementType.getErasedType(), 0).getClass();
+    }
+
+    /**
+     * Helper method for constructing (or finding cached) array type with given
+     * element type. Arrays are cached using element type as the "type parameter"
+     * of the key, so that differently parameterized element types do not collide.
+     * Arrays with element types only valid within resolution context (self-references,
+     * placeholders) are not cached (cache key will be null).
+     */
+    private ResolvedArrayType _arrayOf(Class<?> arrayClass, ResolvedType elementType)
+    {
+        ResolvedTypeKey key = TypeBindings.isContextual(elementType) ? null
+                : _resolvedTypes.key(arrayClass, new ResolvedType[] { elementType });
+        return (ResolvedArrayType) _findOrConstruct(key,
+                () -> new ResolvedArrayType(arrayClass, TypeBindings.emptyBindings(), elementType));
     }
 
     private ResolvedType _fromWildcard(ClassStack context, WildcardType wildType, TypeBindings typeBindings)

src/main/java/com/fasterxml/classmate/types/ResolvedArrayType.java

@@ -35,6 +35,32 @@ public ResolvedArrayType(Class<?> erased, TypeBindings bindings,
         _elementType = elementType;
     }
 
+    @Override
+    public boolean equals(Object other) {
+        return super.equals(other)
+                && _elementsEqual(_elementType, ((ResolvedArrayType) other)._elementType);
+    }
+
+    // Self-reference to element type (like `Node[]` within `Node extends Base<Node[]>`)
+    // must be equal to fully resolved type, so compare erased type and bindings for those
+    private static boolean _elementsEqual(ResolvedType elem1, ResolvedType elem2) {
+        if ((elem1 == null) || (elem2 == null)) {
+            return (elem1 == elem2);
+        }
+        if ((elem1 instanceof ResolvedRecursiveType) || (elem2 instanceof ResolvedRecursiveType)) {
+            return (elem1.getErasedType() == elem2.getErasedType())
+                    && elem1.getTypeBindings().equals(elem2.getTypeBindings());
+        }
+        return Objects.equals(elem1, elem2);
+    }
+
+    // NOTE: consistent with `equals()` since hash code of all types (including
+    // ResolvedRecursiveType) is based on erased type and bindings
+    @Override
+    public int hashCode() {
+        return 31 * super.hashCode() + Objects.hashCode(_elementType);
+    }
+
     @Override
     public boolean canCreateSubtypes() {
         return false;

src/main/java/com/fasterxml/classmate/util/ResolvedTypeCache.java

@@ -46,6 +46,8 @@ public ResolvedTypeKey key(Class<?> simpleType, ResolvedType[] tp) {
         }
         // 25-Oct-2015, tatu: Need to prevent caching of anything with TypePlaceHolder;
         //   can cause problems otherwise as those are ephemeral/mutable containers
+        // [classmate#125]: NOTE: `TypeResolver` also checks for nested placeholders
+        //   and self-references before calling this method
         for (int i = 0; i < len; ++i) {
             if (tp[i] instanceof TypePlaceHolder) {
                 return null;

src/test/java/com/fasterxml/classmate/TestSubtypeResolution.java

@@ -1,5 +1,7 @@
 package com.fasterxml.classmate;
 
+import com.fasterxml.classmate.util.ResolvedTypeCache;
+
 import java.util.*;
 
 /**
@@ -56,6 +58,23 @@ protected void setUp()
     /**********************************************************************
      */
 
+    // Types with nested placeholders (like `Wrapper<List<P0>>`) are not to be cached
+    public void testSubtypeWithNestedPlaceholderNotCached()
+    {
+        ResolvedTypeCache cache = ResolvedTypeCache.lruCache(200);
+        TypeResolver resolver = new TypeResolver(cache);
+        ResolvedType supertype = resolver.resolve(Wrapper.class,
+                resolver.resolve(List.class, String.class));
+        ResolvedType subtype = resolver.resolveSubtype(supertype, ListWrapper.class);
+        assertEquals(supertype, subtype.getParentClass());
+        int size = cache.size();
+
+        for (int i = 0; i < 3; ++i) {
+            resolver.resolveSubtype(supertype, ListWrapper.class);
+        }
+        assertEquals(size, cache.size());
+    }
+
     /**
      * Test to ensure a properly parameterized {@link List} can be be made
      * more specific while still keeping parameterization.