Fix TypeUtils.toString() recursion and bound handling on recursive generic types

[Alwaysgaurav1]

Problem

TypeUtils.toString(Type) suffered from several interrelated issues when handling recursive generic types and type bounds:

1. StackOverflowError on Recursive Generic Types: Formatting recursive generic types like class MySuperClass<T>, class MyClass<U extends MySuperClass<? super U>>, or class MultiBoundClass<U extends Number & Comparable<? super U>> resulted in StackOverflowError.
2. Stripping of Valid Interface Bounds: A workaround previously introduced for LANG-1698 unconditionally skipped bounds if the raw type was an interface. This stripped all valid interface bounds (e.g. <T extends List<String>> formatted as "T" instead of "T extends java.util.List<java.lang.String>"), while still failing with a StackOverflowError on class bounds.
3. Corrupted Multi-Parameter Recursive Types: Workaround methods findRecursiveTypes and appendRecursiveTypes corrupted type parameter formatting on multi-parameter recursive types (such as TwoParams<T extends TwoParams<T, U>, U>), outputting <T><U><U>.
4. Incomplete Multi-Bound Wildcard Inspection: TypeUtils.containsTypeVariables(WildcardType) only inspected the bound at index 0, ignoring subsequent bounds for multi-bounded wildcards.

Root Cause

In Java grammar (JLS Chapter 4), a TypeVariable is a declaration only when declaring a type parameter (e.g. <U extends Number>). Everywhere else—such as inside type arguments (List<U>), wildcard bounds (? extends U, ? super U), generic arrays (U[]), or other type variable bounds (<S extends T>)—it is a type reference (ReferenceType) and should be formatted only by its identifier name U. Treating embedded type variables as declarations led to recursive expansion of bounds.

Solution

1. Differentiate Type Declarations and References: Added toReferenceString(Type) which formats TypeVariable instances by name only and delegates other types to toString(Type). Configured AMP_JOINER, TYPE_ARG_JOINER, and genericArrayTypeToString to format embedded types as references.
2. Preserve Interface Bounds: Removed the interface-check workaround from typeVariableToString, properly preserving all interface bounds without recursion.
3. Remove Workaround Heuristics: Cleanly removed findRecursiveTypes, appendRecursiveTypes, and containsVariableTypeSameParametrizedTypeBound.
4. Inspect All Wildcard Bounds: Updated containsTypeVariables(WildcardType) to check all implicit upper and lower bounds.
5. ThreadLocal Cycle Guard: Added VISITING ThreadLocal<Set<TypeVariable<?>>> with guaranteed VISITING.remove() in finally to guard against arbitrary cyclical type graphs without leaking memory.

Verification

• All 430 tests in TypeUtilsTest passed (including 6 new tests covering recursive class bounds, multi-bounds, interface preservation, and multi-bound wildcards).
• All 590 tests across org.apache.commons.lang3.reflect.* passed without regressions.

[Alwaysgaurav1]

Hi @garydgregory, could you please review..

[Copilot]

🟡 Changes recommended

The recursion guard does not cover all cyclic Type graphs, leaving a critical StackOverflowError path.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request fixes recursive generic type formatting and preserves valid bounds in TypeUtils.

Changes:

• Separates type declarations from references.
• Preserves interface and multi-bounded wildcard bounds.
• Replaces recursive-formatting workarounds with cycle protection.
• Adds regression coverage.

File summaries

File	Summary
src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java	Adds tests for recursive, multi-bound, interface, and wildcard cases.
src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java	Updates type formatting, bound handling, wildcard inspection, and recursion protection.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

No unresolved blocking issues were identified.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[garydgregory]

Hello @Alwaysgaurav1
Thank you for the update.
I'm not sure the new tests do not cover every change:

• Bounded T[], List, and <T extends Number, S extends T> formatting.
• A bounded variable passed to toLongString().
• A parameterized owner with a non-generic inner class, exercising removal of <>.
• Defensive copying of lower bounds; only upper bounds are tested.
• Owner cycles, repeated sibling references, and cleanup after an exception.

[Alwaysgaurav1]

Hello @garydgregory,

Thank you for the thorough review! I have addressed all five points in the latest push (commit df5c62c):

1. Bounded T[], List<T>, and <T extends Number, S extends T> formatting: Added dedicated tests (testBoundedGenericArrayTypeToString, testBoundedParameterizedTypeArgumentToString, and testDependentBoundsClassAndTypeParametersToString) verifying that type variables in arrays, type arguments, and dependent type bounds format as type references without expanding recursively.
2. Bounded variable passed to toLongString(): Updated toLongString to route through toString(typeVariable) for cycle safety, and added testToLongStringBoundedTypeVariable covering single bounds, dependent bounds, multi-bounds, interface bounds, and recursive class bounds.
3. Parameterized owner with a non-generic inner class: Added testParameterizedOwnerWithNonGenericInnerClassToString exercising removal of <> for both parameterizeWithOwner(...) and reflective method return types.
4. Defensive copying of lower bounds: Updated testWildcardTypeBuilderDefensiveCopy to test input array and getter return defensive copying for both upper and lower bounds.
5. Owner cycles, repeated sibling references, and exception cleanup:
   • Routed non-Class owner formatting through toString(useOwner) so owner cycles are guarded, adding testCyclicOwnerParameterizedTypeToString (self-cycles and mutual owner cycles).
   • Added testRepeatedSiblingReferencesToString verifying that identical sibling type instances in a graph are not falsely flagged as cycles.
   • Added testThreadLocalCleanupAfterException verifying clean unwind of the ThreadLocal guard after exceptions (unsupported types, custom exceptions, nested exceptions) and correct subsequent behavior.

All 442 tests in TypeUtilsTest pass, and Checkstyle reports 0 violations.