improved javadoc, error handling and testing of matches

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java

@@ -21,13 +21,13 @@ import java.util.regex.PatternSyntaxException;
 
 import org.antlr.runtime.Token;
 import org.springframework.expression.EvaluationException;
+import org.springframework.expression.spel.ExpressionState;
 import org.springframework.expression.spel.SpelException;
 import org.springframework.expression.spel.SpelMessages;
-import org.springframework.expression.spel.ExpressionState;
 
-// TODO what should be the difference between like and matches?
 /**
- * Implements the matches operator.
+ * Implements the matches operator. Matches takes two operands. The first is a string and the second is a java regex. It
+ * will return true when getValue() is called if the first operand matches the regex.
  * 
  * @author Andy Clement
  */
@@ -38,21 +38,38 @@ public class OperatorMatches extends Operator {
 	}
 
 	@Override
-	public Object getValue(ExpressionState state) throws EvaluationException {
-		Object left = getLeftOperand().getValue(state);
+	public String getOperatorName() {
+		return "matches";
+	}
+
+	/**
+	 * Check the first operand matches the regex specified as the second operand.
+	 * 
+	 * @param state the expression state
+	 * @return true if the first operand matches the regex specified as the second operand, otherwise false
+	 * @throws EvaluationException if there is a problem evaluating the expression (e.g. the regex is invalid)
+	 */
+	@Override
+	public Boolean getValue(ExpressionState state) throws EvaluationException {
+		SpelNode leftOp = getLeftOperand();
+		SpelNode rightOp = getRightOperand();
+		Object left = leftOp.getValue(state, String.class);
 		Object right = getRightOperand().getValue(state);
 		try {
+			if (!(left instanceof String)) {
+				throw new SpelException(leftOp.getCharPositionInLine(),
+						SpelMessages.INVALID_FIRST_OPERAND_FOR_LIKE_OPERATOR, left);
+			}
+			if (!(right instanceof String)) {
+				throw new SpelException(rightOp.getCharPositionInLine(),
+						SpelMessages.INVALID_SECOND_OPERAND_FOR_LIKE_OPERATOR, right);
+			}
 			Pattern pattern = Pattern.compile((String) right);
 			Matcher matcher = pattern.matcher((String) left);
 			return matcher.matches();
 		} catch (PatternSyntaxException pse) {
-			throw new SpelException(pse, SpelMessages.INVALID_PATTERN, right);
+			throw new SpelException(rightOp.getCharPositionInLine(), pse, SpelMessages.INVALID_PATTERN, right);
 		}
 	}
 
-	@Override
-	public String getOperatorName() {
-		return "matches";
-	}
-
 }

org.springframework.expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java

@@ -101,6 +101,18 @@ public class EvaluationTests extends ExpressionTestCase {
 		evaluate("{1, 2, 3, 4, 5} is T(List)", "true", Boolean.class);
 	}
 
+	public void testRelOperatorsIs04() {
+		evaluate("null is T(String)", "false", Boolean.class);
+	}
+
+	public void testRelOperatorsIs05() {
+		evaluate("null is T(Integer)", "false", Boolean.class);
+	}
+
+	public void testRelOperatorsIs06() {
+		evaluateAndCheckError("'A' is null", SpelMessages.IS_OPERATOR_NEEDS_CLASS_OPERAND, 7, "null");
+	}
+
 	public void testRelOperatorsMatches01() {
 		evaluate("'5.0067' matches '^-?\\d+(\\.\\d{2})?$'", "false", Boolean.class);
 	}
@@ -109,6 +121,18 @@ public class EvaluationTests extends ExpressionTestCase {
 		evaluate("'5.00' matches '^-?\\d+(\\.\\d{2})?$'", "true", Boolean.class);
 	}
 
+	public void testRelOperatorsMatches03() {
+		evaluateAndCheckError("null matches '^.*$'", SpelMessages.INVALID_FIRST_OPERAND_FOR_LIKE_OPERATOR, 0, null);
+	}
+
+	public void testRelOperatorsMatches04() {
+		evaluateAndCheckError("'abc' matches null", SpelMessages.INVALID_SECOND_OPERAND_FOR_LIKE_OPERATOR, 14, null);
+	}
+
+	public void testRelOperatorsMatches05() {
+		evaluate("27 matches '^.*2.*$'", true, Boolean.class); // conversion int>string
+	}
+
 	// mathematical operators
 	public void testMathOperatorAdd01() {
 		evaluate("2 + 4", "6", Integer.class);
@@ -336,7 +360,7 @@ public class EvaluationTests extends ExpressionTestCase {
 				SpelMessages.RESULT_OF_SELECTION_CRITERIA_IS_NOT_BOOLEAN);
 	}
 
-	// TODO 3 Q Is $index within projection/selection useful or just cute?
+	// TODO Is $index within projection/selection useful or just cute?
 	public void testSelectionUsingIndex() {
 		evaluate("{1,2,3,4,5,6,7,8,9,10}.?{$index > 5 }", "[7, 8, 9, 10]", ArrayList.class);
 	}

org.springframework.expression/src/test/java/org/springframework/expression/spel/ExpressionTestCase.java

@@ -164,7 +164,7 @@ public abstract class ExpressionTestCase extends TestCase {
 			assertEquals("Type of the result was not as expected.  Expected '" + expectedClassOfResult
 					+ "' but result was of type '" + resultType + "'", expectedClassOfResult
 					.equals/* isAssignableFrom */(resultType), true);
-			// TODO 4 isAssignableFrom would allow some room for compatibility
+			// TODO isAssignableFrom would allow some room for compatibility
 			// in the above expression...
 
 			boolean isWritable = e.isWritable(eContext);
@@ -243,7 +243,19 @@ public abstract class ExpressionTestCase extends TestCase {
 								+ " properties of the exception, it only has " + inserts.length + " inserts");
 					}
 					for (int i = 1; i < otherProperties.length; i++) {
-						if (!inserts[i - 1].equals(otherProperties[i])) {
+						if (otherProperties[i] == null) {
+							if (inserts[i - 1] != null) {
+								ex.printStackTrace();
+								fail("Insert does not match, expected 'null' but insert value was '" + inserts[i - 1]
+										+ "'");
+							}
+						} else if (inserts[i - 1] == null) {
+							if (otherProperties[i] != null) {
+								ex.printStackTrace();
+								fail("Insert does not match, expected '" + otherProperties[i]
+										+ "' but insert value was 'null'");
+							}
+						} else if (!inserts[i - 1].equals(otherProperties[i])) {
 							ex.printStackTrace();
 							fail("Insert does not match, expected '" + otherProperties[i] + "' but insert value was '"
 									+ inserts[i - 1] + "'");

org.springframework.expression/src/test/java/org/springframework/expression/spel/LiteralTests.java

@@ -97,7 +97,7 @@ public class LiteralTests extends ExpressionTestCase {
 		evaluate("null", null, null);
 	}
 
-	// TODO 3 'default' format for date varies too much, we need to standardize on a format for EL
+	// TODO 'default' format for date varies too much, we need to standardize on a format for EL
 	// public void testLiteralDate01() {
 	// eval("date('3-Feb-2008 4:50:20 PM').getTime()>0", "true", Boolean.class);
 	// }