Skip to content

Commit 47b9eb2

Browse files
mbelladesebersole
mbellade
authored and
sebersole
committed
#145 Fix nested generic type variable argument resolution
Also fix bytebuddy and jandex type switches not setting the correct `declaringType` when resolving parameterized generic-supertype's type variables
1 parent d4f2413 commit 47b9eb2

File tree

6 files changed

+83
-44
lines changed

6 files changed

+83
-44
lines changed

‎hibernate-models-bytebuddy/src/main/java/org/hibernate/models/bytebuddy/internal/ClassDetailsImpl.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public List<TypeDetails> getImplementedInterfaces() {
121121
@Override
122122
public List<TypeVariableDetails> getTypeParameters() {
123123
if ( typeParameters == null ) {
124-
this.typeParameters = determineTypeParameters( typeDescription, this, getModelContext() );
124+
this.typeParameters = determineTypeParameters( typeDescription, getModelContext() );
125125
}
126126
return typeParameters;
127127
}
@@ -227,15 +227,15 @@ private static ClassDetails determineSuperType(
227227
.resolveClassDetails( typeDescription.getSuperClass().asRawType().getTypeName() );
228228
}
229229

230-
private static TypeDetails determineGenericSuperType(TypeDescription typeDescription, ModelsContext modelsContext) {
230+
private TypeDetails determineGenericSuperType(TypeDescription typeDescription, ModelsContext modelsContext) {
231231
if ( typeDescription.getSuperClass() == null ) {
232232
return null;
233233
}
234234

235-
return TypeSwitchStandard.switchType( typeDescription.getSuperClass(), modelsContext );
235+
return TypeSwitchStandard.switchType( typeDescription.getSuperClass(), this, modelsContext );
236236
}
237237

238-
private static List<TypeDetails> determineInterfaces(
238+
private List<TypeDetails> determineInterfaces(
239239
TypeDescription typeDescription,
240240
ModelsContext modelsContext) {
241241
final TypeList.Generic interfaceTypes = typeDescription.getInterfaces();
@@ -247,16 +247,16 @@ private static List<TypeDetails> determineInterfaces(
247247
for ( TypeDescription.Generic interfaceType : interfaceTypes ) {
248248
final TypeDetails switchedType = TypeSwitchStandard.switchType(
249249
interfaceType,
250+
this,
250251
modelsContext
251252
);
252253
result.add( switchedType );
253254
}
254255
return result;
255256
}
256257

257-
private static List<TypeVariableDetails> determineTypeParameters(
258+
private List<TypeVariableDetails> determineTypeParameters(
258259
TypeDescription typeDescription,
259-
ClassDetailsImpl current,
260260
ModelsContext modelsContext) {
261261
final TypeList.Generic typeArguments = typeDescription.getTypeVariables();
262262
if ( CollectionHelper.isEmpty( typeArguments ) ) {
@@ -265,7 +265,7 @@ private static List<TypeVariableDetails> determineTypeParameters(
265265

266266
final ArrayList<TypeVariableDetails> result = arrayList( typeArguments.size() );
267267
for ( TypeDescription.Generic typeArgument : typeArguments ) {
268-
result.add( (TypeVariableDetails) TypeSwitchStandard.switchType( typeArgument, current, modelsContext ) );
268+
result.add( (TypeVariableDetails) TypeSwitchStandard.switchType( typeArgument, this, modelsContext ) );
269269
}
270270
return result;
271271
}

‎hibernate-models-bytebuddy/src/main/java/org/hibernate/models/bytebuddy/internal/TypeSwitchStandard.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@
3535
*/
3636
public class TypeSwitchStandard implements TypeSwitch<TypeDetails> {
3737

38-
public static TypeDetails switchType(TypeDefinition typeDescription, ModelsContext modelsContext) {
39-
return switchType( typeDescription, null, modelsContext );
40-
}
41-
4238
public static TypeDetails switchType(TypeDefinition typeDescription, ClassDetails declaringType, ModelsContext modelsContext) {
4339
final TypeSwitchStandard switchImpl = new TypeSwitchStandard( declaringType );
4440
return TypeSwitcher.switchType( typeDescription, switchImpl, modelsContext );
@@ -47,7 +43,7 @@ public static TypeDetails switchType(TypeDefinition typeDescription, ClassDetail
4743

4844
private final ClassDetails declaringType;
4945

50-
public TypeSwitchStandard(ClassDetails declaringType) {
46+
private TypeSwitchStandard(ClassDetails declaringType) {
5147
this.declaringType = declaringType;
5248
}
5349

‎hibernate-models-jandex/src/main/java/org/hibernate/models/jandex/internal/JandexClassDetails.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public List<TypeDetails> getImplementedInterfaces() {
121121
@Override
122122
public List<TypeVariableDetails> getTypeParameters() {
123123
if ( typeParameters == null ) {
124-
this.typeParameters = determineTypeParameters( classInfo, this, getModelContext() );
124+
this.typeParameters = determineTypeParameters( classInfo, getModelContext() );
125125
}
126126
return typeParameters;
127127
}
@@ -246,15 +246,15 @@ private static ClassDetails determineSuperType(
246246
.resolveClassDetails( classInfo.superClassType().name().toString() );
247247
}
248248

249-
private static TypeDetails determineGenericSuperType(ClassInfo classInfo, ModelsContext modelsContext) {
249+
private TypeDetails determineGenericSuperType(ClassInfo classInfo, ModelsContext modelsContext) {
250250
if ( classInfo.superClassType() == null ) {
251251
return null;
252252
}
253253

254-
return JandexTypeSwitchStandard.switchType( classInfo.superClassType(), modelsContext );
254+
return JandexTypeSwitchStandard.switchType( classInfo.superClassType(), this, modelsContext );
255255
}
256256

257-
private static List<TypeDetails> determineInterfaces(
257+
private List<TypeDetails> determineInterfaces(
258258
ClassInfo classInfo,
259259
ModelsContext modelsContext) {
260260
final List<Type> interfaceTypes = classInfo.interfaceTypes();
@@ -266,22 +266,23 @@ private static List<TypeDetails> determineInterfaces(
266266
for ( Type interfaceType : interfaceTypes ) {
267267
final TypeDetails switchedType = JandexTypeSwitchStandard.switchType(
268268
interfaceType,
269+
this,
269270
modelsContext
270271
);
271272
result.add( switchedType );
272273
}
273274
return result;
274275
}
275276

276-
private static List<TypeVariableDetails> determineTypeParameters(ClassInfo classInfo, JandexClassDetails current, ModelsContext modelsContext) {
277+
private List<TypeVariableDetails> determineTypeParameters(ClassInfo classInfo, ModelsContext modelsContext) {
277278
final List<TypeVariable> jandexTypeVariables = classInfo.typeParameters();
278279
if ( CollectionHelper.isEmpty( jandexTypeVariables ) ) {
279280
return emptyList();
280281
}
281282

282283
final ArrayList<TypeVariableDetails> result = arrayList( jandexTypeVariables.size() );
283284
for ( TypeVariable jandexTypeVariable : jandexTypeVariables ) {
284-
result.add( (TypeVariableDetails) JandexTypeSwitchStandard.switchType( jandexTypeVariable, current, modelsContext ) );
285+
result.add( (TypeVariableDetails) JandexTypeSwitchStandard.switchType( jandexTypeVariable, this, modelsContext ) );
285286
}
286287
return result;
287288
}

‎hibernate-models-jandex/src/main/java/org/hibernate/models/jandex/internal/JandexTypeSwitchStandard.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@
4343
* @author Steve Ebersole
4444
*/
4545
public class JandexTypeSwitchStandard implements JandexTypeSwitch<TypeDetails> {
46-
public static TypeDetails switchType(Type type, ModelsContext modelsContext) {
47-
assert type.kind() != Type.Kind.TYPE_VARIABLE;
48-
return switchType( type, null, modelsContext );
49-
}
5046

5147
public static TypeDetails switchType(Type type, ClassDetails declaringType, ModelsContext modelsContext) {
5248
final JandexTypeSwitchStandard genericVariableSwitch = new JandexTypeSwitchStandard( declaringType );
@@ -55,7 +51,7 @@ public static TypeDetails switchType(Type type, ClassDetails declaringType, Mode
5551

5652
private final ClassDetails declaringType;
5753

58-
public JandexTypeSwitchStandard(ClassDetails declaringType) {
54+
private JandexTypeSwitchStandard(ClassDetails declaringType) {
5955
this.declaringType = declaringType;
6056
}
6157

‎hibernate-models/src/main/java/org/hibernate/models/spi/TypeDetailsHelper.java‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
*/
55
package org.hibernate.models.spi;
66

7-
import java.util.Collections;
8-
import java.util.List;
9-
107
import org.hibernate.models.internal.ArrayTypeDetailsImpl;
118
import org.hibernate.models.internal.ParameterizedTypeDetailsImpl;
129
import org.hibernate.models.internal.PrimitiveKind;
1310
import org.hibernate.models.internal.util.CollectionHelper;
1411

12+
import java.util.Collections;
13+
import java.util.List;
14+
1515
import static org.hibernate.models.internal.util.CollectionHelper.arrayList;
1616
import static org.hibernate.models.spi.StandardTypeDetails.OBJECT_TYPE_DETAILS;
1717

@@ -185,35 +185,37 @@ public static ClassDetails resolveRawClass(TypeDetails typeDetails) {
185185
* @param parameterizedType the parameterized type used to resolve the type variable's relative type
186186
* @param typeVariable the type variable to resolve
187187
*
188-
* @return the type variable's relative type, or {@code null} if not resolved
188+
* @return the type variable's relative type
189189
*/
190190
public static TypeDetails resolveTypeVariableFromParameterizedType(
191191
ParameterizedTypeDetails parameterizedType,
192192
TypeVariableDetails typeVariable) {
193193
final ClassDetails classDetails = parameterizedType.getRawClassDetails();
194-
final TypeDetails typeArgument = findMatchingTypeArgument(
195-
classDetails.getTypeParameters(),
196-
parameterizedType.getArguments(),
197-
typeVariable.getIdentifier()
198-
);
199-
200-
// If no type argument is found, or the type variable is defined by another
201-
// class, try resolving it in the generic super type if present
202-
if ( typeArgument == null || classDetails != typeVariable.getDeclaringType() ) {
194+
if ( classDetails == typeVariable.getDeclaringType() ) {
195+
// If the type variable is defined by the parameterized class, try to find the matching type argument
196+
return findMatchingTypeArgument(
197+
classDetails.getTypeParameters(),
198+
parameterizedType.getArguments(),
199+
typeVariable.getIdentifier()
200+
);
201+
}
202+
else {
203+
// Try resolving the type variable in the generic super type
203204
final TypeDetails genericSuper = classDetails.getGenericSuperType();
204205
final TypeDetails resolvedType = genericSuper != null ?
205206
genericSuper.resolveTypeVariable( typeVariable ) :
206207
null;
207-
if ( typeArgument == null || resolvedType != null
208-
&& resolvedType.getTypeKind() != TypeDetails.Kind.TYPE_VARIABLE ) {
209-
return resolvedType;
208+
if ( resolvedType != null ) {
209+
return resolvedType.getTypeKind() == TypeDetails.Kind.TYPE_VARIABLE ?
210+
parameterizedType.resolveTypeVariable( resolvedType.asTypeVariable() ) :
211+
resolvedType;
210212
}
211213
}
212214

213-
// Either we found the exact type variable's argument, or the parameterized class redefines
214-
// a type variable with the same identifier as a supertype, and it should be ignored.
215-
// Return the matching generic type argument
216-
return typeArgument;
215+
throw new IllegalArgumentException(
216+
"Unable to resolve type variable [" + typeVariable.getIdentifier() + "] from parameterized type ["
217+
+ parameterizedType.getName() + "]"
218+
);
217219
}
218220

219221
private static TypeDetails findMatchingTypeArgument(

‎hibernate-models/src/test/java/org/hibernate/models/testing/tests/generics/InheritanceTypeVariableTests.java‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,16 @@ void testParameterizedHierarchy() {
2727
final ModelsContext modelsContext = createModelContext(
2828
Root.class,
2929
Base1.class,
30-
Base2.class
30+
Base2.class,
31+
Level1.class,
32+
Level2.class,
33+
Base3.class
3134
);
3235

3336
final ClassDetails rootClassDetails = modelsContext.getClassDetailsRegistry().getClassDetails( Root.class.getName() );
3437
final ClassDetails base1ClassDetails = modelsContext.getClassDetailsRegistry().getClassDetails( Base1.class.getName() );
3538
final ClassDetails base2ClassDetails = modelsContext.getClassDetailsRegistry().getClassDetails( Base2.class.getName() );
39+
final ClassDetails base3ClassDetails = modelsContext.getClassDetailsRegistry().getClassDetails( Base3.class.getName() );
3640

3741
final FieldDetails idField = rootClassDetails.findFieldByName( "id" );
3842
final TypeDetails idFieldType = idField.getType();
@@ -80,6 +84,35 @@ void testParameterizedHierarchy() {
8084
assertThat( resolvedClassType.isResolved() ).isTrue();
8185
}
8286

87+
{
88+
final TypeDetails concreteType = idField.resolveRelativeType( base3ClassDetails );
89+
assertThat( concreteType ).isInstanceOf( ClassTypeDetails.class );
90+
final ClassDetails concreteClassDetails = ( (ClassTypeDetails) concreteType ).getClassDetails();
91+
assertThat( concreteClassDetails.toJavaClass() ).isEqualTo( Long.class );
92+
93+
final ClassBasedTypeDetails resolvedClassType = idField.resolveRelativeClassType( base3ClassDetails );
94+
assertThat( resolvedClassType.isResolved() ).isTrue();
95+
assertThat( resolvedClassType.getClassDetails().toJavaClass() ).isEqualTo( Long.class );
96+
assertThat( resolvedClassType.isResolved() ).isTrue();
97+
}
98+
99+
{
100+
final ClassDetails level1Class = modelsContext.getClassDetailsRegistry().getClassDetails( Level1.class.getName() );
101+
final FieldDetails middleField = level1Class.findFieldByName( "middle" );
102+
final TypeDetails middleFieldType = middleField.getType();
103+
assertThat( middleFieldType.getTypeKind() ).isEqualTo( TypeDetails.Kind.TYPE_VARIABLE );
104+
assertThat( middleFieldType.isResolved() ).isFalse();
105+
106+
final TypeDetails concreteType = middleField.resolveRelativeType( base3ClassDetails );
107+
assertThat( concreteType ).isInstanceOf( ClassTypeDetails.class );
108+
final ClassDetails concreteClassDetails = ( (ClassTypeDetails) concreteType ).getClassDetails();
109+
assertThat( concreteClassDetails.toJavaClass() ).isEqualTo( Short.class );
110+
111+
final ClassBasedTypeDetails resolvedClassType = middleField.resolveRelativeClassType( base3ClassDetails );
112+
assertThat( resolvedClassType.isResolved() ).isTrue();
113+
assertThat( resolvedClassType.getClassDetails().toJavaClass() ).isEqualTo( Short.class );
114+
assertThat( resolvedClassType.isResolved() ).isTrue();
115+
}
83116
}
84117

85118
@SuppressWarnings("unused")
@@ -92,4 +125,15 @@ static class Base1 extends Root<Integer> {
92125

93126
static class Base2 extends Root<String> {
94127
}
128+
129+
// reuse same type parameter identifier as Root
130+
static class Level1<J, I> extends Root<J> {
131+
I middle;
132+
}
133+
134+
static class Level2<K> extends Level1<K, Short> {
135+
}
136+
137+
static class Base3<K> extends Level2<Long> {
138+
}
95139
}

0 commit comments

Comments
 (0)