refactor: implement check for recursive type bound without using Object (#3865)

Author: andrewbwogi

src/main/java/spoon/support/visitor/java/JavaReflectionTreeBuilder.java

@@ -142,9 +142,9 @@ public void addConstructor(CtConstructor<?> ctConstructor) {
 			@Override
 			public void addTypeReference(CtRole role, CtTypeReference<?> typeReference) {
 				switch (role) {
-				case SUPER_TYPE:
-					ctClass.setSuperclass(typeReference);
-					return;
+					case SUPER_TYPE:
+						ctClass.setSuperclass(typeReference);
+						return;
 				}
 				super.addTypeReference(role, typeReference);
 			}
@@ -291,8 +291,7 @@ public void visitField(Field field) {
 			Set<ModifierKind> modifiers = RtHelper.getModifiers(field.getModifiers());
 			if (modifiers.contains(ModifierKind.STATIC)
 					&& modifiers.contains(ModifierKind.PUBLIC)
-					&& (field.getType().isPrimitive() || String.class.isAssignableFrom(field.getType()))
-				) {
+					&& (field.getType().isPrimitive() || String.class.isAssignableFrom(field.getType()))) {
 				CtLiteral<Object> defaultExpression = factory.createLiteral(field.get(null));
 				ctField.setDefaultExpression(defaultExpression);
 			}
@@ -356,13 +355,13 @@ public <T extends GenericDeclaration> void visitTypeParameter(TypeVariable<T> pa
 			@Override
 			public void addTypeReference(CtRole role, CtTypeReference<?> typeReference) {
 				switch (role) {
-				case SUPER_TYPE:
-					if (typeParameter.getSuperclass() != null) {
-						typeParameter.setSuperclass(typeParameter.getFactory().createIntersectionTypeReferenceWithBounds(Arrays.asList(typeParameter.getSuperclass(), typeReference)));
-					} else {
-						typeParameter.setSuperclass(typeReference);
-					}
-					return;
+					case SUPER_TYPE:
+						if (typeParameter.getSuperclass() != null) {
+							typeParameter.setSuperclass(typeParameter.getFactory().createIntersectionTypeReferenceWithBounds(Arrays.asList(typeParameter.getSuperclass(), typeReference)));
+						} else {
+							typeParameter.setSuperclass(typeReference);
+						}
+						return;
 				}
 				super.addTypeReference(role, typeReference);
 			}
@@ -379,13 +378,6 @@ public <T extends GenericDeclaration> void visitTypeParameterReference(CtRole ro
 		typeParameterReference.setSimpleName(parameter.getName());
 
 		RuntimeBuilderContext runtimeBuilderContext = new TypeReferenceRuntimeBuilderContext(parameter, typeParameterReference);
-		if (contexts.contains(runtimeBuilderContext)) {
-			// we are in the case of a loop
-			exit();
-			enter(new TypeReferenceRuntimeBuilderContext(Object.class, factory.Type().OBJECT));
-			return;
-		}
-
 		GenericDeclaration genericDeclaration = parameter.getGenericDeclaration();
 		for (RuntimeBuilderContext context : contexts) {
 			CtTypeParameter typeParameter = context.getTypeParameter(genericDeclaration, parameter.getName());
@@ -404,9 +396,14 @@ public <T extends GenericDeclaration> void visitTypeParameterReference(CtRole ro
 
 	@Override
 	public void visitTypeReference(CtRole role, ParameterizedType type) {
+		Type[] typeArguments = type.getActualTypeArguments();
+		if (role == CtRole.SUPER_TYPE && typeArguments.length > 0) {
+			if (hasProcessedRecursiveBound(typeArguments)) {
+				return;
+			}
+		}
 		final CtTypeReference<?> ctTypeReference = factory.Core().createTypeReference();
 		ctTypeReference.setSimpleName(((Class) type.getRawType()).getSimpleName());
-
 		RuntimeBuilderContext context = new TypeReferenceRuntimeBuilderContext(type, ctTypeReference) {
 
 			@Override
@@ -418,27 +415,26 @@ public void addType(CtType<?> aType) {
 
 		enter(context);
 		super.visitTypeReference(role, type);
-
-		// in case of a loop we have replaced a context:
-		// we do not want to addTypeName then
-		// and we have to rely on the instance reference to check that
-		boolean contextStillExisting = false;
-		for (RuntimeBuilderContext context1 : contexts) {
-			contextStillExisting = contextStillExisting || (context1 == context);
-		}
 		exit();
 
-		if (contextStillExisting) {
-			contexts.peek().addTypeReference(role, ctTypeReference);
-		}
+		contexts.peek().addTypeReference(role, ctTypeReference);
 	}
 
 	@Override
 	public void visitTypeReference(CtRole role, WildcardType type) {
 		final CtWildcardReference wildcard = factory.Core().createWildcardReference();
-		//looks like type.getUpperBounds() always returns single value array with Object.class
+		//type.getUpperBounds() returns at least a single value array with Object.class
 		//so we cannot distinguish between <? extends Object> and <?>, which must be upper==true too!
-		wildcard.setUpper((type.getLowerBounds() != null && type.getLowerBounds().length > 0) == false);
+		wildcard.setUpper((type.getLowerBounds().length > 0) == false);
+
+		if (!type.getUpperBounds()[0].equals(Object.class)) {
+			if (hasProcessedRecursiveBound(type.getUpperBounds())) {
+				return;
+			}
+		}
+		if (hasProcessedRecursiveBound(type.getLowerBounds())) {
+			return;
+		}
 
 		enter(new TypeReferenceRuntimeBuilderContext(type, wildcard));
 		super.visitTypeReference(role, type);
@@ -447,7 +443,21 @@ public void visitTypeReference(CtRole role, WildcardType type) {
 		contexts.peek().addTypeReference(role, wildcard);
 	}
 
-
+	// check if a type parameter that is bounded by some expression involving itself has already been processed
+	private boolean hasProcessedRecursiveBound(Type[] types) {
+		for (Type type : types) {
+			if (type instanceof TypeVariable) {
+				TypeVariable t = (TypeVariable) type;
+				final CtTypeParameterReference typeParameterReference = factory.Core().createTypeParameterReference();
+				typeParameterReference.setSimpleName(t.getName());
+				RuntimeBuilderContext runtimeBuilderContext = new TypeReferenceRuntimeBuilderContext(t, typeParameterReference);
+				if (contexts.contains(runtimeBuilderContext)) {
+					return true;
+				}
+			}
+		}
+		return false;
+	}
 
 	@Override
 	public <T> void visitArrayReference(CtRole role, final Type typeArray) {
@@ -457,9 +467,9 @@ public <T> void visitArrayReference(CtRole role, final Type typeArray) {
 			@Override
 			public void addTypeReference(CtRole role, CtTypeReference<?> typeReference) {
 				switch (role) {
-				case DECLARING_TYPE:
-					arrayTypeReference.setDeclaringType(typeReference);
-					return;
+					case DECLARING_TYPE:
+						arrayTypeReference.setDeclaringType(typeReference);
+						return;
 				}
 				arrayTypeReference.setComponentType(typeReference);
 			}

src/main/java/spoon/support/visitor/java/JavaReflectionVisitorImpl.java

@@ -461,7 +461,7 @@ public void visitTypeReference(CtRole role, ParameterizedType type) {
 
 	@Override
 	public void visitTypeReference(CtRole role, WildcardType type) {
-		if (type.getUpperBounds() != null && type.getUpperBounds().length > 0 && !type.getUpperBounds()[0].equals(Object.class)) {
+		if (!type.getUpperBounds()[0].equals(Object.class)) {
 			for (Type upper : type.getUpperBounds()) {
 				visitTypeReference(CtRole.BOUNDING_TYPE, upper);
 			}

src/test/java/spoon/test/generics/GenericsTest.java

@@ -335,6 +335,15 @@ public void testBugCommonCollection() throws Exception {
 		}
 	}
 
+	@Test
+	public void testRecursiveUpperBound() throws Exception {
+		// contract: wildcards with a recursive upper bound are properly parsed with runtime reflection
+		CtClass<?> type = build("spoon.test.generics.testclasses3", "UpperBoundedWildcardUser");
+		CtField field = type.getElements(new TypeFilter<>(CtField.class)).get(0);
+		assertTrue(field.getType().getTypeDeclaration().toString()
+				.startsWith("public class UpperBoundedWildcard<Q extends java.lang.Comparable<? extends Q>>"));
+	}
+
 	@Test
 	public void testInstanceOfMapEntryGeneric() throws Exception {
 		CtClass<?> type = build("spoon.test.generics.testclasses3", "InstanceOfMapEntryGeneric");

src/test/java/spoon/test/generics/testclasses3/UpperBoundedWildcard.java

@@ -0,0 +1,3 @@
+package spoon.test.generics.testclasses3;
+
+public class UpperBoundedWildcard<Q extends Comparable<? extends Q>>  { }

src/test/java/spoon/test/generics/testclasses3/UpperBoundedWildcardUser.java

@@ -0,0 +1,5 @@
+package spoon.test.generics.testclasses3;
+
+public class UpperBoundedWildcardUser {
+    UpperBoundedWildcard x;
+}