some cleanup

Author: christophstrobl

src/main/java/org/springframework/data/repository/query/QueryMethod.java

@@ -299,7 +299,7 @@ private static void assertReturnTypeAssignable(Method method, Set<Class<?>> type
 		Assert.notNull(method, "Method must not be null!");
 		Assert.notEmpty(types, "Types must not be null or empty!");
 
-		// TODO: to resolve generics we'd need the actual repository interface here
+		// TODO: to resolve generics fully we'd need the actual repository interface here
 		TypeInformation<?> returnType = ClassTypeInformation.fromReturnTypeOf(method);
 
 		returnType = QueryExecutionConverters.isSingleValue(returnType.getType()) //

src/main/java/org/springframework/data/repository/util/ClassUtils.java

@@ -21,7 +21,6 @@
 import java.util.Collection;
 import java.util.function.Consumer;
 
-import org.springframework.core.ResolvableType;
 import org.springframework.data.repository.Repository;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.TypeInformation;
@@ -185,7 +184,7 @@ public static void unwrapReflectionException(Exception ex) throws Throwable {
 		throw ex;
 	}
 
-	// todo owning type
+	// TODO: we should also consider having the owning type here so we can resolve generics better.
 	private static TypeInformation<?> getEffectivelyReturnedTypeFrom(Method method) {
 
 		TypeInformation<?> returnType = ClassTypeInformation.fromReturnTypeOf(method);

src/main/java/org/springframework/data/util/ClassTypeInformation.java

@@ -23,13 +23,16 @@
 import java.util.Set;
 
 import org.springframework.core.ResolvableType;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ConcurrentReferenceHashMap;
 import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType;
 
 /**
+ * {@link TypeInformation} for a plain {@link Class}.
+ *
+ * @author Oliver Gierke
  * @author Christoph Strobl
- * @since 2021/11
  */
 public class ClassTypeInformation<S> extends TypeDiscoverer<S> {
 
@@ -42,15 +45,29 @@ public class ClassTypeInformation<S> extends TypeDiscoverer<S> {
 	private static final Map<Class<?>, ClassTypeInformation<?>> cache = new ConcurrentReferenceHashMap<>(64,
 			ReferenceType.WEAK);
 
+	/**
+	 * Warning: Does not fully resolve generic arguments.
+	 * @param method
+	 * @return
+	 * @deprecated since 3.0 Use {@link #fromReturnTypeOf(Method, Class)} instead.
+	 */
+	@Deprecated
 	public static TypeInformation<?> fromReturnTypeOf(Method method) {
 		return new TypeDiscoverer<>(ResolvableType.forMethodReturnType(method));
 	}
-//
-//	public static TypeInformation<?> fromReturnTypeOf(Method method, Class<?> actualType) {
-//		// todo open issue in FW for ResolvableType.forMethod(method)
-//		//return new NewTypeDiscoverer(ResolvableType.forType(method.getGenericReturnType(), ResolvableType.forClass(method.getDeclaringClass())));
-//		return new NewTypeDiscoverer<>(ResolvableType.forMethodReturnType(method, actualType));
-//	}
+
+	/**
+	 * @param method
+	 * @param actualType can be {@literal null}.
+	 * @return
+	 */
+	public static TypeInformation<?> fromReturnTypeOf(Method method, @Nullable Class<?> actualType) {
+
+		if(actualType == null) {
+			return new TypeDiscoverer<>(ResolvableType.forMethodReturnType(method));
+		}
+		return new TypeDiscoverer<>(ResolvableType.forMethodReturnType(method, actualType));
+	}
 
 	Class<?> type;
 

src/main/java/org/springframework/data/util/TypeDiscoverer.java

@@ -40,8 +40,11 @@
 import org.springframework.util.ReflectionUtils;
 
 /**
+ * Basic {@link TypeDiscoverer} that contains basic functionality to discover property types.
+ *
+ * @author Oliver Gierke
  * @author Christoph Strobl
- * @since 2021/11
+ * @author Mark Paluch
  */
 public class TypeDiscoverer<S> implements TypeInformation<S> {
 
@@ -135,7 +138,6 @@ private Optional<TypeInformation<?>> getPropertyInformation(String fieldname) {
 
 			if(it.getReadMethod() != null) {
 				return new TypeDiscoverer(ResolvableType.forMethodReturnType(it.getReadMethod(), rawType));
-//				return ClassTypeInformation.fromReturnTypeOf(it.getReadMethod());
 			}
 			if(it.getWriteMethod() != null) {
 				return new TypeDiscoverer(ResolvableType.forMethodParameter(it.getWriteMethod(), 0, rawType));
@@ -222,20 +224,6 @@ protected TypeInformation<?> doGetComponentType() {
 			}
 
 			return mapValueType.resolve() != null ? new TypeDiscoverer<>(mapValueType) :null;
-
-//			return Arrays.stream(resolvableType.getInterfaces()).filter(ResolvableType::hasGenerics)
-//					.findFirst()
-//					.map(it -> it.getGeneric(0))
-//					.filter(it -> !ResolvableType.NONE.equals(it))
-//					.map(NewTypeDiscoverer::new)
-//					.orElse(null);
-
-//			if(type.getComponentType().equals(ResolvableType.NONE)) {
-//				if(!type.hasGenerics()) {
-//					return null;
-//				}
-//			}
-//			return mapValueType != null ? new NewTypeDiscoverer(mapValueType) : new ClassTypeInformation<>(Object.class);
 		}
 
 		if (isNullableWrapper()) {
@@ -377,8 +365,6 @@ public TypeInformation<?> getSuperTypeInformation(Class<?> superType) {
 			candidates.add(genericSuperclass);
 		}
 
-		// todo try raw type interfaces //
-
 		candidates.addAll(Arrays.asList(resolvableType.getInterfaces()));
 
 		for (var candidate : candidates) {
@@ -393,12 +379,7 @@ public TypeInformation<?> getSuperTypeInformation(Class<?> superType) {
 							return new TypeDiscoverer<>(ResolvableType.forRawClass(superType));
 						}
 					}
-//					return new NewTypeDiscoverer<>(ResolvableType.forClassWithGenerics(candidate.toClass(), classes));
 				}
-//				return new NewTypeDiscoverer(candidate);
-//				if(ObjectUtils.isEmpty(superType.getTypeParameters())) {
-//					new NewTypeDiscoverer(ResolvableType.forRawClass(superType));
-//				}
 				return new TypeDiscoverer(ResolvableType.forClass(superType, getType()));
 			} else {
 				var sup = candidate.getSuperType();

src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java

@@ -68,12 +68,6 @@ void setUp() {
 
 	@Test // DATACMNS-68
 	void discoversComponentTypeCorrectly() {
-
-		// TestClassSet testClassSet;
-		// class TestClassSet extends TreeSet<Object> {}
-
-		// type -> collection -> generics
-
 		assertThat(getProperty(TestClassComplex.class, "testClassSet").getComponentType()).isEqualTo(Object.class);
 	}
 

src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java

@@ -191,7 +191,6 @@ void returnsSingleElementCollectionForTargetThatReturnsNonCollection() throws Th
 	}
 
 	@Test // DATACMNS-1598
-	// todo: component type must find the enum
 	void returnsEnumSet() throws Throwable {
 
 		MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,

src/test/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadataUnitTests.java

@@ -21,7 +21,6 @@
 import java.util.Collection;
 import java.util.Optional;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.data.domain.Page;
@@ -55,7 +54,6 @@ void rejectsNonRepositoryInterface() {
 	}
 
 	@Test // DATACMNS-406
-//	@Disabled("F*****!")
 	void rejectsUnparameterizedRepositoryInterface() {
 		assertThatIllegalArgumentException().isThrownBy(() -> new DefaultRepositoryMetadata(RepoWithoutArgs.class));
 	}

src/test/java/org/springframework/data/repository/util/ClassUtilsUnitTests.java

@@ -52,8 +52,6 @@ void determinesValidFieldsCorrectly() {
 
 	@Test // DATACMNS-769
 	void unwrapsWrapperTypesBeforeAssignmentCheck() throws Exception {
-
-		// Future<Page<User>> findAsync(Pageable pageable);
 		assertReturnTypeAssignable(UserRepository.class.getMethod("findAsync", Pageable.class), Page.class);
 	}
 

src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java

@@ -34,7 +34,6 @@
 import org.springframework.aop.TargetSource;
 import org.springframework.aop.framework.Advised;
 import org.springframework.aop.framework.AopConfigException;
-import org.springframework.aop.framework.ProxyFactory;
 
 import org.springframework.data.mapping.Person;
 import org.springframework.lang.Nullable;
@@ -486,6 +485,16 @@ public void proxyTypeInformationShouldNotEqualUserClassTypeInfo () {
 		assertThat(typeInfoLeaf).isNotEqualTo(typeInformationLeafProxy);
 	}
 
+	@Test  // GH-2312
+	void typeInfoShouldPreserveGenericParameter() {
+
+		TypeInformation<Wrapper> wrapperTypeInfo = ClassTypeInformation.from(Wrapper.class);
+		TypeInformation<?> fieldTypeInfo = wrapperTypeInfo.getProperty("field");
+		TypeInformation<?> valueTypeInfo = fieldTypeInfo.getProperty("value");
+
+		assertThat(valueTypeInfo.getType()).isEqualTo(Leaf.class);
+	}
+
 	static class StringMapContainer extends MapContainer<String> {
 
 	}
@@ -697,6 +706,12 @@ static class SomeGeneric<T> {
 
 	static class SomeConcrete extends SomeGeneric<String> {}
 
+	static class GenericExtendingSomeGeneric<T> extends SomeGeneric<T> { }
+
+	static class Wrapper {
+		GenericExtendingSomeGeneric<Leaf> field;
+	}
+
 	static class WildcardedWrapper {
 		SomeGeneric<?> wildcarded;
 	}