Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Draft: Issue #69 | Add ResolvedTypeMapper to convert ResolvedType into Type #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Luc-Veldhuis wants to merge 4 commits into FasterXML:main
base: main
Choose a base branch
Loading
from Luc-Veldhuis:luc-veldhuis/69-resolvedtype-to-java-type

Conversation

@Luc-Veldhuis
Copy link

@Luc-Veldhuis Luc-Veldhuis commented Aug 26, 2025

Attempt at getting a converter working from ResolvedType -> java.lang.reflect.Type
However, this required me to create a custom implementation for ParameterizedType and GenericArrayType, which is not so great :/

If anybody has any suggestions, I would be glad to hear them!

@Luc-Veldhuis Luc-Veldhuis changed the title (削除) Draft: Issue#69 | Add ResolvedTypeMapper to convert ResolvedType into Type (削除ここまで) (追記) Draft: Issue #69 | Add ResolvedTypeMapper to convert ResolvedType into Type (追記ここまで) Aug 26, 2025
Copy link
Member

Makes sense overall. Not sure what to think TBH, but would be willing to merge in in some form.

@garretwilson WDYT?


/**
* Utility for converting a {@link ResolvedType} into a standard Java {@link Type}
* that can be used with the reflection API.
Copy link
Member

@cowtowncoder cowtowncoder Dec 5, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably list specific Types produced (GenericArrayType, ParameterizedType).

And also, without generics, Classes too?

Copy link

garretwilson commented Dec 18, 2025
edited
Loading

Makes sense overall. Not sure what to think TBH, but would be willing to merge in in some form.

@garretwilson WDYT?

Let me be honest—my original issue in #69 was several years and I don't readily remember all intricacies of the issue. However I'm now deep into LLMs, so just now I worked with the two top coding LLMs to analyze this pull request. The conclusion below is LLM generated, but it is a summary of a conversation that I guided, going over my original post, analyzing the pull request, looking for bugs, considering limitations, and discussing alternatives. I think you'll find it useful; be sure to check the limitations and potential bugs it points out. I provide my own note below after the summary.


Context

This PR addresses a problem I raised in issue #69 back in October 2023: the inability to convert ResolvedType back to java.lang.reflect.Type for use with libraries like Jackson. I've been reviewing this PR with LLM assistance to get back up to speed and evaluate the approach.

Alternative: Preserving the Original Type in ClassMate

One question worth considering: rather than reconstructing a Type externally, could ClassMate preserve the original during resolution?

// In ResolvedType
private final Type originalType; // null for programmatic construction
public Optional<Type> findOriginalType() {
 return Optional.ofNullable(originalType);
}

Trade-offs: Not all ResolvedType instances originate from a Type (some are constructed programmatically), so this would require nullable/Optional handling. There's also the philosophical question of whether ClassMate should carry unresolved type information. That said, it would avoid custom Type implementations entirely.

The mapper approach in this PR is a reasonable pragmatic alternative.

Potential Issues Found

1. Multi-dimensional generic arrays (High)List<String>[][] loses its type parameter:

// Element type is GenericArrayType, not ParameterizedType, so falls through
if (elementType instanceof ParameterizedType) { ... }
return arrayType.getErasedType(); // Returns List[][] — String is lost

Fix: if (elementType instanceof ParameterizedType || elementType instanceof GenericArrayType)

2. Nested parameterized inner classes (Medium) — Owner type is raw:

// For Outer<String>.Inner<Integer>, owner becomes Outer.class, not Outer<String>
new ParameterizedTypeImpl(erasedType, ..., erasedType.getEnclosingClass());

The reconstructed type won't equals() a JVM-provided type in this case.

3. Array mutability (Low)getActualTypeArguments() returns the internal array directly; JDK implementations return a clone.

Inherent Limitations (Not Bugs)

These stem from ClassMate's design, not this PR:

Input Reconstructed As
List<? extends Number> List<Number>
List<? super Integer> List<Object>
Map<K, V> (unbound) Map<Object, Object>

Minor Observations

  • ResolvedTypeMapper is stateless—map() could be static. Is instance-based design intentional?
  • Tests don't cover multi-dimensional generic arrays or nested parameterized inner classes.

Overall, this PR solves a real need. The multi-dimensional array issue should probably be addressed before merging.


Even though I pushed both LLMs to find problems, it looks like overall they both came away with a positive evaluation. I recommend checking into the bugs they mention, reviewing the limitations, and reconsidering whether you might want to just integrate this into ClassMate. In particular note the doubt about whether the facility should be stateless. I hope this summary is useful!

(Note that LLMs aren't all that innovative, so although they didn't find major errors with the PR, that doesn't indicate whether there are alternative creative solutions. Sorry I wasn't up-to-speed enough to provide more of my own direct evaluation at the moment.)

I'll keep the LLM conversation history around, so if you have any follow-up questions let me know and I'll report back the responses.

Copy link

I finally got a chance to dig deep into the original ideas and remember the context of all this. I came away with what I feel to be some remarkable insights, which I propose in #111.

As related to this pull request, the takeaway is that this functionality is useful under the current ResolvedType design, and the implementation here is reasonable (although the potential issues I brought up should be looked at). However the core issue is that ResolvedType needs to be redesigned and slightly repurposes/repositioned, with a toType() method that will either attempt to reconstruct a Type (in cases where the original type is not available) or provide the original type (if the ResolvedType were created in a context in which the original Type was available).

Copy link
Member

My first gut reaction to preserving source Type -- which is interesting idea, obvious in hindsight -- is that it leads to too uneven handling and somehow seems bit wrong. But it has its benefits for sure.

Biggest question really is, I think, intended usage; and from that determine which approach (keep original Type, return when one exists; generate only if not -- vs -- always re-generate) works best.

Good points on issues; I will file separate ones.

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

Reviewers

@cowtowncoder cowtowncoder cowtowncoder left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /