Revised isInstanceOf/isAssignable message concatenation

Issue: SPR-15196
(cherry picked from commit 22322fd)

Author: jhoeller

spring-core/src/main/java/org/springframework/util/Assert.java

@@ -55,6 +55,30 @@
  */
 public abstract class Assert {
 
+	/**
+	 * Assert a boolean expression, throwing an {@code IllegalStateException}
+	 * if the expression evaluates to {@code false}.
+	 * <p>Call {@link #isTrue} if you wish to throw an {@code IllegalArgumentException}
+	 * on an assertion failure.
+	 * <pre class="code">Assert.state(id == null, "The id property must not already be initialized");</pre>
+	 * @param expression a boolean expression
+	 * @param message the exception message to use if the assertion fails
+	 * @throws IllegalStateException if {@code expression} is {@code false}
+	 */
+	public static void state(boolean expression, String message) {
+		if (!expression) {
+			throw new IllegalStateException(message);
+		}
+	}
+
+	/**
+	 * @deprecated as of 4.3.7, in favor of {@link #state(boolean, String)}
+	 */
+	@Deprecated
+	public static void state(boolean expression) {
+		state(expression, "[Assertion failed] - this state invariant must be true");
+	}
+
 	/**
 	 * Assert a boolean expression, throwing an {@code IllegalArgumentException}
 	 * if the expression evaluates to {@code false}.
@@ -287,21 +311,20 @@ public static void notEmpty(Map<?, ?> map) {
 
 	/**
 	 * Assert that the provided object is an instance of the provided class.
-	 * <pre class="code">Assert.instanceOf(Foo.class, foo, "Processing Foo:");</pre>
+	 * <pre class="code">Assert.instanceOf(Foo.class, foo, "Foo expected");</pre>
 	 * @param type the type to check against
 	 * @param obj the object to check
-	 * @param message a message which will be prepended to the message generated
-	 * by this method in order to provide further context. It should normally end
-	 * in ":" or "." so that the generated message looks OK when appended to it.
+	 * @param message a message which will be prepended to provide further context.
+	 * If it is empty or ends in ":" or ";" or "," or ".", a full exception message
+	 * will be appended. If it ends in a space, the name of the offending object's
+	 * type will be appended. In any other case, a ":" with a space and the name
+	 * of the offending object's type will be appended.
 	 * @throws IllegalArgumentException if the object is not an instance of type
-	 * @see Class#isInstance
 	 */
 	public static void isInstanceOf(Class<?> type, Object obj, String message) {
 		notNull(type, "Type to check against must not be null");
 		if (!type.isInstance(obj)) {
-			String className = (obj != null ? obj.getClass().getName() : "null");
-			throw new IllegalArgumentException(StringUtils.hasLength(message) ? message + ": " + className :
-					"Object of class [" + className + "] must be an instance of " + type);
+			instanceCheckFailed(type, obj, message);
 		}
 	}
 
@@ -311,27 +334,27 @@ public static void isInstanceOf(Class<?> type, Object obj, String message) {
 	 * @param type the type to check against
 	 * @param obj the object to check
 	 * @throws IllegalArgumentException if the object is not an instance of type
-	 * @see Class#isInstance
 	 */
 	public static void isInstanceOf(Class<?> type, Object obj) {
 		isInstanceOf(type, obj, "");
 	}
 
 	/**
 	 * Assert that {@code superType.isAssignableFrom(subType)} is {@code true}.
-	 * <pre class="code">Assert.isAssignable(Number.class, myClass);</pre>
+	 * <pre class="code">Assert.isAssignable(Number.class, myClass, "Number expected");</pre>
 	 * @param superType the super type to check against
 	 * @param subType the sub type to check
-	 * @param message a message which will be prepended to the message generated
-	 * by this method in order to provide further context. It should normally end
-	 * in ":" or "." so that the generated message looks OK when appended to it.
+	 * @param message a message which will be prepended to provide further context.
+	 * If it is empty or ends in ":" or ";" or "," or ".", a full exception message
+	 * will be appended. If it ends in a space, the name of the offending sub type
+	 * will be appended. In any other case, a ":" with a space and the name of the
+	 * offending sub type will be appended.
 	 * @throws IllegalArgumentException if the classes are not assignable
 	 */
 	public static void isAssignable(Class<?> superType, Class<?> subType, String message) {
 		notNull(superType, "Super type to check against must not be null");
 		if (subType == null || !superType.isAssignableFrom(subType)) {
-			throw new IllegalArgumentException(StringUtils.hasLength(message) ? message + ": " + subType :
-					subType + " is not assignable to " + superType);
+			assignableCheckFailed(superType, subType, message);
 		}
 	}
 
@@ -346,28 +369,50 @@ public static void isAssignable(Class<?> superType, Class<?> subType) {
 		isAssignable(superType, subType, "");
 	}
 
-	/**
-	 * Assert a boolean expression, throwing an {@code IllegalStateException}
-	 * if the expression evaluates to {@code false}.
-	 * <p>Call {@link #isTrue} if you wish to throw an {@code IllegalArgumentException}
-	 * on an assertion failure.
-	 * <pre class="code">Assert.state(id == null, "The id property must not already be initialized");</pre>
-	 * @param expression a boolean expression
-	 * @param message the exception message to use if the assertion fails
-	 * @throws IllegalStateException if {@code expression} is {@code false}
-	 */
-	public static void state(boolean expression, String message) {
-		if (!expression) {
-			throw new IllegalStateException(message);
+
+	private static void instanceCheckFailed(Class<?> type, Object obj, String msg) {
+		String className = (obj != null ? obj.getClass().getName() : "null");
+		String result = "";
+		boolean defaultMessage = true;
+		if (StringUtils.hasLength(msg)) {
+			if (endsWithSeparator(msg)) {
+				result = msg + " ";
+			}
+			else {
+				result = messageWithTypeName(msg, className);
+				defaultMessage = false;
+			}
+		}
+		if (defaultMessage) {
+			result = result + ("Object of class [" + className + "] must be an instance of " + type);
 		}
+		throw new IllegalArgumentException(result);
 	}
 
-	/**
-	 * @deprecated as of 4.3.7, in favor of {@link #state(boolean, String)}
-	 */
-	@Deprecated
-	public static void state(boolean expression) {
-		state(expression, "[Assertion failed] - this state invariant must be true");
+	private static void assignableCheckFailed(Class<?> superType, Class<?> subType, String msg) {
+		String result = "";
+		boolean defaultMessage = true;
+		if (StringUtils.hasLength(msg)) {
+			if (endsWithSeparator(msg)) {
+				result = msg + " ";
+			}
+			else {
+				result = messageWithTypeName(msg, subType);
+				defaultMessage = false;
+			}
+		}
+		if (defaultMessage) {
+			result = result + (subType + " is not assignable to " + superType);
+		}
+		throw new IllegalArgumentException(result);
+	}
+
+	private static boolean endsWithSeparator(String msg) {
+		return (msg.endsWith(":") || msg.endsWith(";") || msg.endsWith(",") || msg.endsWith("."));
+	}
+
+	private static String messageWithTypeName(String msg, Object typeName) {
+		return msg + (msg.endsWith(" ") ? "" : ": ") + typeName;
 	}
 
 }

spring-core/src/test/java/org/springframework/util/AssertTests.java

@@ -41,6 +41,18 @@ public class AssertTests {
 	public final ExpectedException thrown = ExpectedException.none();
 
 
+	@Test
+	public void state() {
+		Assert.state(true, "enigma");
+	}
+
+	@Test
+	public void stateWithFalseExpression() {
+		thrown.expect(IllegalStateException.class);
+		thrown.expectMessage("enigma");
+		Assert.state(false, "enigma");
+	}
+
 	@Test
 	public void isTrueWithMessage() {
 		Assert.isTrue(true, "enigma");
@@ -233,6 +245,21 @@ public void isInstanceOfWithTypeMismatchAndCustomMessage() {
 		Assert.isInstanceOf(String.class, 42L, "Custom message");
 	}
 
+	@Test
+	public void isInstanceOfWithTypeMismatchAndCustomMessageWithSeparator() {
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage(
+				"Custom message: Object of class [java.lang.Long] must be an instance of class java.lang.String");
+		Assert.isInstanceOf(String.class, 42L, "Custom message:");
+	}
+
+	@Test
+	public void isInstanceOfWithTypeMismatchAndCustomMessageWithSpace() {
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage("Custom message for java.lang.Long");
+		Assert.isInstanceOf(String.class, 42L, "Custom message for ");
+	}
+
 	@Test
 	public void isAssignable() {
 		Assert.isAssignable(Number.class, Integer.class, "enigma");
@@ -262,20 +289,22 @@ public void isAssignableWithTypeMismatchAndNullMessage() {
 	@Test
 	public void isAssignableWithTypeMismatchAndCustomMessage() {
 		thrown.expect(IllegalArgumentException.class);
-		thrown.expectMessage("enigma: class java.lang.Integer");
-		Assert.isAssignable(String.class, Integer.class, "enigma");
+		thrown.expectMessage("Custom message: class java.lang.Integer");
+		Assert.isAssignable(String.class, Integer.class, "Custom message");
 	}
 
 	@Test
-	public void state() {
-		Assert.state(true, "enigma");
+	public void isAssignableWithTypeMismatchAndCustomMessageWithSeparator() {
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage("Custom message: class java.lang.Integer is not assignable to class java.lang.String");
+		Assert.isAssignable(String.class, Integer.class, "Custom message:");
 	}
 
 	@Test
-	public void stateWithFalseExpression() {
-		thrown.expect(IllegalStateException.class);
-		thrown.expectMessage("enigma");
-		Assert.state(false, "enigma");
+	public void isAssignableWithTypeMismatchAndCustomMessageWithSpace() {
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage("Custom message for class java.lang.Integer");
+		Assert.isAssignable(String.class, Integer.class, "Custom message for ");
 	}
 
 }