corrected TypeUtils.isAssignable() failure to return true in certain valid wildcard bounding scenarios (SPR-6850)

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3171 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.core/src/main/java/org/springframework/util/TypeUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,12 +21,15 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.WildcardType;
 
+import org.springframework.util.ClassUtils;
+
 /**
  * Utility to work with Java 5 generic type parameters.
  * Mainly for internal use within the framework.
  *
  * @author Ramnivas Laddad
  * @author Juergen Hoeller
+ * @author Chris Beams
  * @since 2.0.7
  */
 public abstract class TypeUtils {
@@ -38,56 +41,78 @@ public abstract class TypeUtils {
 	 * @param rhsType the value type that should be assigned to the target type
 	 * @return true if rhs is assignable to lhs
 	 */
-	@SuppressWarnings("unchecked")
 	public static boolean isAssignable(Type lhsType, Type rhsType) {
-		Assert.notNull(lhsType, "Left-hand side type must not be null");
-		Assert.notNull(rhsType, "Right-hand side type must not be null");
+		if (rhsType == null) {
+			return true;
+		}
+
+		if (lhsType == null) {
+			return false;
+		}
+
+		// all types are assignable to themselves and to class Object
 		if (lhsType.equals(rhsType) || lhsType.equals(Object.class)) {
 			return true;
 		}
-		if (lhsType instanceof Class) {
-			Class lhsClass = (Class) lhsType;
-			if (rhsType instanceof Class) {
-				return ClassUtils.isAssignable(lhsClass, (Class) rhsType);
+
+		if (lhsType instanceof Class<?>) {
+			Class<?> lhsClass = (Class<?>) lhsType;
+
+			// just comparing two classes
+			if (rhsType instanceof Class<?>) {
+				return ClassUtils.isAssignable(lhsClass, (Class<?>) rhsType);
 			}
-			else if (rhsType instanceof ParameterizedType){
+
+			if (rhsType instanceof ParameterizedType) {
 				Type rhsRaw = ((ParameterizedType) rhsType).getRawType();
-				if (rhsRaw instanceof Class) {
-					return ClassUtils.isAssignable(lhsClass, (Class) rhsRaw);
+
+				// a parameterized type is always assignable to its raw class type
+				if (rhsRaw instanceof Class<?>) {
+					return ClassUtils.isAssignable(lhsClass, (Class<?>) rhsRaw);
 				}
 			}
-			else if (lhsClass.isArray() && rhsType instanceof GenericArrayType){
+			else if (lhsClass.isArray() && rhsType instanceof GenericArrayType) {
 				Type rhsComponent = ((GenericArrayType) rhsType).getGenericComponentType();
+
 				return isAssignable(lhsClass.getComponentType(), rhsComponent);
 			}
 		}
+
+		// parameterized types are only assignable to other parameterized types and class types
 		if (lhsType instanceof ParameterizedType) {
-			if (rhsType instanceof Class) {
+			if (rhsType instanceof Class<?>) {
 				Type lhsRaw = ((ParameterizedType) lhsType).getRawType();
-				if (lhsRaw instanceof Class) {
-					return ClassUtils.isAssignable((Class) lhsRaw, (Class) rhsType);
+
+				if (lhsRaw instanceof Class<?>) {
+					return ClassUtils.isAssignable((Class<?>) lhsRaw, (Class<?>) rhsType);
 				}
 			}
 			else if (rhsType instanceof ParameterizedType) {
 				return isAssignable((ParameterizedType) lhsType, (ParameterizedType) rhsType);
 			}
 		}
+
 		if (lhsType instanceof GenericArrayType) {
 			Type lhsComponent = ((GenericArrayType) lhsType).getGenericComponentType();
-			if (rhsType instanceof Class) {
-				Class rhsClass = (Class) rhsType;
+
+			if (rhsType instanceof Class<?>) {
+				Class<?> rhsClass = (Class<?>) rhsType;
+
 				if (rhsClass.isArray()) {
 					return isAssignable(lhsComponent, rhsClass.getComponentType());
 				}
 			}
 			else if (rhsType instanceof GenericArrayType) {
 				Type rhsComponent = ((GenericArrayType) rhsType).getGenericComponentType();
+
 				return isAssignable(lhsComponent, rhsComponent);
 			}
 		}
+
 		if (lhsType instanceof WildcardType) {
 			return isAssignable((WildcardType) lhsType, rhsType);
 		}
+
 		return false;
 	}
 
@@ -95,36 +120,101 @@ public abstract class TypeUtils {
 		if (lhsType.equals(rhsType)) {
 			return true;
 		}
+
 		Type[] lhsTypeArguments = lhsType.getActualTypeArguments();
 		Type[] rhsTypeArguments = rhsType.getActualTypeArguments();
+
 		if (lhsTypeArguments.length != rhsTypeArguments.length) {
 			return false;
 		}
+
 		for (int size = lhsTypeArguments.length, i = 0; i < size; ++i) {
 			Type lhsArg = lhsTypeArguments[i];
 			Type rhsArg = rhsTypeArguments[i];
+
 			if (!lhsArg.equals(rhsArg) &&
 					!(lhsArg instanceof WildcardType && isAssignable((WildcardType) lhsArg, rhsArg))) {
 				return false;
 			}
 		}
+
 		return true;
 	}
 
 	private static boolean isAssignable(WildcardType lhsType, Type rhsType) {
-		Type[] upperBounds = lhsType.getUpperBounds();
-		Type[] lowerBounds = lhsType.getLowerBounds();
-		for (int size = upperBounds.length, i = 0; i < size; ++i) {
-			if (!isAssignable(upperBounds[i], rhsType)) {
-				return false;
+		Type[] lUpperBounds = lhsType.getUpperBounds();
+
+		// supply the implicit upper bound if none are specified
+		if (lUpperBounds.length == 0) {
+			lUpperBounds = new Type[] { Object.class };
+		}
+
+		Type[] lLowerBounds = lhsType.getLowerBounds();
+
+		// supply the implicit lower bound if none are specified
+		if (lLowerBounds.length == 0) {
+			lLowerBounds = new Type[] { null };
+		}
+
+		if (rhsType instanceof WildcardType) {
+			// both the upper and lower bounds of the right-hand side must be
+			// completely enclosed in the upper and lower bounds of the left-
+			// hand side.
+			WildcardType rhsWcType = (WildcardType) rhsType;
+			Type[] rUpperBounds = rhsWcType.getUpperBounds();
+
+			if (rUpperBounds.length == 0) {
+				rUpperBounds = new Type[] { Object.class };
+			}
+
+			Type[] rLowerBounds = rhsWcType.getLowerBounds();
+
+			if (rLowerBounds.length == 0) {
+				rLowerBounds = new Type[] { null };
+			}
+
+			for (Type lBound : lUpperBounds) {
+				for (Type rBound : rUpperBounds) {
+					if (!isAssignable(lBound, rBound)) {
+						return false;
+					}
+				}
+
+				for (Type rBound : rLowerBounds) {
+					if (!isAssignable(lBound, rBound)) {
+						return false;
+					}
+				}
+			}
+
+			for (Type lBound : lLowerBounds) {
+				for (Type rBound : rUpperBounds) {
+					if (!isAssignable(rBound, lBound)) {
+						return false;
+					}
+				}
+
+				for (Type rBound : rLowerBounds) {
+					if (!isAssignable(rBound, lBound)) {
+						return false;
+					}
+				}
 			}
 		}
-		for (int size = lowerBounds.length, i = 0; i < size; ++i) {
-			if (!isAssignable(rhsType, lowerBounds[i])) {
-				return false;
+		else {
+			for (Type lBound : lUpperBounds) {
+				if (!isAssignable(lBound, rhsType)) {
+					return false;
+				}
+			}
+
+			for (Type lBound : lLowerBounds) {
+				if (!isAssignable(rhsType, lBound)) {
+					return false;
+				}
 			}
 		}
+
 		return true;
 	}
-
 }

org.springframework.core/src/test/java/org/springframework/util/TypeUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,26 +16,41 @@
 
 package org.springframework.util;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import org.junit.Test;
 
 /**
+ * Unit tests for {@link TypeUtils}.
+ *
  * @author Juergen Hoeller
+ * @author Chris Beams
  */
 public class TypeUtilsTests {
 
+	public static Object object;
+
+	public static String string;
+
+	public static Integer number;
+
 	public static List<Object> objects;
 
+	public static List<String> strings;
+
 	public static List<? extends Object> openObjects;
 
-	public static List<String> strings;
+	public static List<? extends Number> openNumbers;
+
+	public static List<? super Object> storableObjectList;
 
 	public static List<Number>[] array;
 
@@ -51,6 +66,7 @@ public class TypeUtilsTests {
 		assertTrue(TypeUtils.isAssignable(List.class, LinkedList.class));
 		assertFalse(TypeUtils.isAssignable(List.class, Collection.class));
 		assertFalse(TypeUtils.isAssignable(List.class, HashSet.class));
+		assertFalse(TypeUtils.isAssignable(null, Object.class));
 	}
 
 	@Test
@@ -76,6 +92,25 @@ public class TypeUtilsTests {
 		assertFalse(TypeUtils.isAssignable(objectsType, stringsType));
 	}
 
+	@Test
+	public void withWildcardTypes() throws Exception {
+		ParameterizedType openObjectsType = (ParameterizedType) getClass().getField("openObjects").getGenericType();
+		ParameterizedType openNumbersType = (ParameterizedType) getClass().getField("openNumbers").getGenericType();
+		Type storableObjectListType = getClass().getField("storableObjectList").getGenericType();
+
+		Type objectType = getClass().getField("object").getGenericType();
+		Type numberType = getClass().getField("number").getGenericType();
+		Type stringType = getClass().getField("string").getGenericType();
+
+		Type openWildcard = openObjectsType.getActualTypeArguments()[0]; // '?'
+		Type openNumbersWildcard = openNumbersType.getActualTypeArguments()[0]; // '? extends number'
+
+		assertTrue(TypeUtils.isAssignable(openWildcard, objectType));
+		assertTrue(TypeUtils.isAssignable(openNumbersWildcard, numberType));
+		assertFalse(TypeUtils.isAssignable(openNumbersWildcard, stringType));
+		assertFalse(TypeUtils.isAssignable(storableObjectListType, openObjectsType));
+	}
+
 	@Test
 	public void withGenericArrayTypes() throws Exception {
 		Type arrayType = getClass().getField("array").getGenericType();