Skip to content

Conversation

@isaric
Copy link
Contributor

@isaric isaric commented Dec 30, 2025

Fix TypeUtils.isAssignable for Class parameterized types

Summary

This PR fixes a bug in TypeUtils.isAssignable where it incorrectly returned true for certain ParameterizedType assignments involving java.lang.Class. Specifically, it was allowing assignments like Class<TestIF> to Class<? extends TestIF<?>>, which are prohibited by the Java compiler due to the invariance of the Class<T> type.

Problem

In Java, Class<T> is invariant. This means that even if TestImpl extends TestIF, Class<TestImpl> is NOT assignable to Class<TestIF>. Furthermore, Class<TestIF> is not assignable to Class<? extends TestIF<?>> because the type argument of the latter is a wildcard that requires a specific capturing behavior that a raw-ish or differently parameterized Class does not satisfy in the same way.

The previous implementation of TypeUtils.isAssignable(Type, ParameterizedType, Map) was too permissive when handling Class.class as the raw type, treating it like any other parameterized interface or class instead of enforcing the strict invariance required for Class<T>.

Solution

The fix introduces a specialized check within isAssignable(Type, ParameterizedType, Map) when the target raw type is java.lang.Class.

  1. It identifies if the target ParameterizedType represents a Class<?>.
  2. It extracts the type argument from both the source and target.
  3. It ensures that the source type argument matches the target type argument exactly, or correctly satisfies the target's wildcard bounds if applicable, adhering to the strict rules of the Java language for the Class class.

Changes

  • org.apache.commons.lang3.reflect.TypeUtils:
    • Added a logic block in isAssignable(Type, ParameterizedType, Map) specifically for Class.class.
    • Implemented strict comparison for Class type arguments using unrollVariableAssignments to resolve any type variables in the current context.
    • Ensures that even if the target is a wildcard (e.g., Class<? extends Foo>), the source must match the captured type exactly or satisfy the bound without violating invariance.
  • org.apache.commons.lang3.reflect.TypeUtilsTest:
    • Added testIsAssignable_ClassWithParameterizedType which includes:
      • Class<TestIF> to Class<? extends TestIF<?>> (Expected: false)
      • Class<TestImpl> to Class<? extends TestIF<?>> (Expected: false)
      • Class<TestImpl2> to Class<? extends TestIF<Number>> (Expected: false)
    • Added helper internal interfaces and classes (TestIF<T>, TestImpl<T>, TestImpl2<R>) to support these test cases.

Note: Used AI tools in the writing of this PR - JetBrains AI. There are no copyright issues with the submitted code.

@isaric isaric closed this Dec 30, 2025
@garydgregory
Copy link
Member

garydgregory commented Dec 30, 2025

Opened, then closed, what's up? Why the churn?

@isaric
Copy link
Contributor Author

isaric commented Dec 30, 2025

Opened, then closed, what's up? Why the churn?

I did not check the previous PR until pushing my changes. It's basically a repeat.

The last comment in the Jira issue mentioned the test cases should be changed from the ones in the description.

It's unclear if this issue still has Merit.

@isaric isaric reopened this Dec 30, 2025
@isaric
Copy link
Contributor Author

isaric commented Dec 30, 2025

@garydgregory I re-examined both issues (LANG-1749 and LANG-1700), comments and the first PR and I decided to reopen this one.
I will also open another PR for LANG-1700.
Please review.

@garydgregory garydgregory merged commit bcad40a into apache:master Dec 31, 2025
19 of 20 checks passed
@garydgregory
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants