Polish

Closes gh-1581

spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java

@@ -24,7 +24,6 @@ import org.springframework.asm.MethodVisitor;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.spel.CodeFlow;
 import org.springframework.lang.Nullable;
-import org.springframework.util.ClassUtils;
 import org.springframework.util.NumberUtils;
 import org.springframework.util.ObjectUtils;
 

spring-expression/src/main/java/org/springframework/expression/spel/support/StandardTypeComparator.java

@@ -46,7 +46,7 @@ public class StandardTypeComparator implements TypeComparator {
 			return true;
 		}
 		if (left instanceof Comparable && right instanceof Comparable) {
-            Class<?> ancestor = ClassUtils.determineCommonAncestor(left.getClass(), right.getClass());
+			Class<?> ancestor = ClassUtils.determineCommonAncestor(left.getClass(), right.getClass());
 			return ancestor != null && Comparable.class.isAssignableFrom(ancestor);
 		}
 		return false;

spring-expression/src/test/java/org/springframework/expression/spel/AlternativeComparatorTests.java

@@ -1,62 +0,0 @@
-/*
- * Copyright 2002-2014 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.expression.spel;
-
-import org.junit.Test;
-import org.springframework.lang.Nullable;
-import org.springframework.expression.Expression;
-import org.springframework.expression.TypeComparator;
-import org.springframework.expression.EvaluationException;
-import org.springframework.expression.ExpressionParser;
-import org.springframework.expression.spel.standard.SpelExpressionParser;
-import org.springframework.expression.spel.support.StandardEvaluationContext;
-
-import static org.junit.Assert.*;
-
-/**
- * Test construction of arrays.
- *
- * @author Andy Clement
- */
-public class AlternativeComparatorTests {
-	private ExpressionParser parser = new SpelExpressionParser();
-
-	// A silly comparator declaring everything to be equal
-	private TypeComparator customComparator = new TypeComparator() {
-		@Override
-		public boolean canCompare(@Nullable Object firstObject, @Nullable Object secondObject) {
-			return true;
-		}
-
-		@Override
-		public int compare(@Nullable Object firstObject, @Nullable Object secondObject) throws EvaluationException {
-			return 0;
-		}
-
-	};
-
-	@Test
-	public void customComparatorWorksWithEquality() {
-		final StandardEvaluationContext ctx = new StandardEvaluationContext();
-		ctx.setTypeComparator(customComparator);
-
-		Expression expr = parser.parseExpression("'1' == 1");
-
-		assertEquals(true, expr.getValue(ctx, Boolean.class));
-
-	}
-}

spring-expression/src/test/java/org/springframework/expression/spel/ComparatorTests.java

@@ -21,8 +21,13 @@ import java.math.BigDecimal;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.expression.EvaluationException;
+import org.springframework.expression.Expression;
+import org.springframework.expression.ExpressionParser;
 import org.springframework.expression.TypeComparator;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.expression.spel.support.StandardTypeComparator;
+import org.springframework.lang.Nullable;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -32,7 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * @author Andy Clement
  * @author Giovanni Dall'Oglio Risso
  */
-class DefaultComparatorUnitTests {
+public class ComparatorTests {
 
 	@Test
 	void testPrimitives() throws EvaluationException {
@@ -120,4 +125,30 @@ class DefaultComparatorUnitTests {
 		assertThat(comparator.canCompare(String.class,3)).isFalse();
 	}
 
+	@Test
+	public void customComparatorWorksWithEquality() {
+		final StandardEvaluationContext ctx = new StandardEvaluationContext();
+		ctx.setTypeComparator(customComparator);
+
+		ExpressionParser parser = new SpelExpressionParser();
+		Expression expr = parser.parseExpression("'1' == 1");
+
+		assertThat(expr.getValue(ctx, Boolean.class)).isTrue();
+
+	}
+
+	// A silly comparator declaring everything to be equal
+	private TypeComparator customComparator = new TypeComparator() {
+		@Override
+		public boolean canCompare(@Nullable Object firstObject, @Nullable Object secondObject) {
+			return true;
+		}
+
+		@Override
+		public int compare(@Nullable Object firstObject, @Nullable Object secondObject) throws EvaluationException {
+			return 0;
+		}
+
+	};
+
 }

spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java

@@ -58,7 +58,9 @@ class OperatorTests extends AbstractExpressionTests {
 		evaluate("'abc' == new java.lang.StringBuilder('abc')", true, Boolean.class);
 		evaluate("'abc' == 'def'", false, Boolean.class);
 		evaluate("'abc' == null", false, Boolean.class);
-		evaluate("new org.springframework.expression.spel.OperatorTests$SubComparable() == new org.springframework.expression.spel.OperatorTests$OtherSubComparable()", true, Boolean.class);
+		evaluate("new org.springframework.expression.spel.OperatorTests$SubComparable(0) == new org.springframework.expression.spel.OperatorTests$OtherSubComparable(0)", true, Boolean.class);
+		evaluate("new org.springframework.expression.spel.OperatorTests$SubComparable(1) < new org.springframework.expression.spel.OperatorTests$OtherSubComparable(2)", true, Boolean.class);
+		evaluate("new org.springframework.expression.spel.OperatorTests$SubComparable(2) > new org.springframework.expression.spel.OperatorTests$OtherSubComparable(1)", true, Boolean.class);
 
 		evaluate("3 eq 5", false, Boolean.class);
 		evaluate("5 eQ 3", false, Boolean.class);
@@ -621,18 +623,40 @@ class OperatorTests extends AbstractExpressionTests {
 
 	public static class BaseComparable implements Comparable<BaseComparable> {
 
+		private int id;
+
+		public BaseComparable() {
+			this.id = 0;
+		}
+
+		public BaseComparable(int id) {
+			this.id = id;
+		}
+
 		@Override
 		public int compareTo(BaseComparable other) {
-			return 0;
+			return this.id - other.id;
 		}
 	}
 
 
 	public static class SubComparable extends BaseComparable {
+		public SubComparable() {
+		}
+
+		public SubComparable(int id) {
+			super(id);
+		}
 	}
 
 
 	public static class OtherSubComparable extends BaseComparable {
+		public OtherSubComparable() {
+		}
+
+		public OtherSubComparable(int id) {
+			super(id);
+		}
 	}
 
 }

spring-expression/src/test/java/org/springframework/expression/spel/StandardTypeComparatorTests.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright 2002-2021 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.expression.spel;
+
+import java.math.BigDecimal;
+
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.expression.EvaluationException;
+import org.springframework.expression.TypeComparator;
+import org.springframework.expression.spel.support.StandardTypeComparator;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for type comparison
+ *
+ * @author Andy Clement
+ * @author Giovanni Dall'Oglio Risso
+ */
+public class StandardTypeComparatorTests {
+
+	@Test
+	void testPrimitives() throws EvaluationException {
+		TypeComparator comparator = new StandardTypeComparator();
+		// primitive int
+		assertThat(comparator.compare(1, 2)).isNegative();
+		assertThat(comparator.compare(1, 1)).isZero();
+		assertThat(comparator.compare(2, 1)).isPositive();
+
+		assertThat(comparator.compare(1.0d, 2)).isNegative();
+		assertThat(comparator.compare(1.0d, 1)).isZero();
+		assertThat(comparator.compare(2.0d, 1)).isPositive();
+
+		assertThat(comparator.compare(1.0f, 2)).isNegative();
+		assertThat(comparator.compare(1.0f, 1)).isZero();
+		assertThat(comparator.compare(2.0f, 1)).isPositive();
+
+		assertThat(comparator.compare(1L, 2)).isNegative();
+		assertThat(comparator.compare(1L, 1)).isZero();
+		assertThat(comparator.compare(2L, 1)).isPositive();
+
+		assertThat(comparator.compare(1, 2L)).isNegative();
+		assertThat(comparator.compare(1, 1L)).isZero();
+		assertThat(comparator.compare(2, 1L)).isPositive();
+
+		assertThat(comparator.compare(1L, 2L)).isNegative();
+		assertThat(comparator.compare(1L, 1L)).isZero();
+		assertThat(comparator.compare(2L, 1L)).isPositive();
+	}
+
+	@Test
+	void testNonPrimitiveNumbers() throws EvaluationException {
+		TypeComparator comparator = new StandardTypeComparator();
+
+		BigDecimal bdOne = new BigDecimal("1");
+		BigDecimal bdTwo = new BigDecimal("2");
+
+		assertThat(comparator.compare(bdOne, bdTwo)).isNegative();
+		assertThat(comparator.compare(bdOne, new BigDecimal("1"))).isZero();
+		assertThat(comparator.compare(bdTwo, bdOne)).isPositive();
+
+		assertThat(comparator.compare(1, bdTwo)).isNegative();
+		assertThat(comparator.compare(1, bdOne)).isZero();
+		assertThat(comparator.compare(2, bdOne)).isPositive();
+
+		assertThat(comparator.compare(1.0d, bdTwo)).isNegative();
+		assertThat(comparator.compare(1.0d, bdOne)).isZero();
+		assertThat(comparator.compare(2.0d, bdOne)).isPositive();
+
+		assertThat(comparator.compare(1.0f, bdTwo)).isNegative();
+		assertThat(comparator.compare(1.0f, bdOne)).isZero();
+		assertThat(comparator.compare(2.0f, bdOne)).isPositive();
+
+		assertThat(comparator.compare(1L, bdTwo)).isNegative();
+		assertThat(comparator.compare(1L, bdOne)).isZero();
+		assertThat(comparator.compare(2L, bdOne)).isPositive();
+
+	}
+
+	@Test
+	void testNulls() throws EvaluationException {
+		TypeComparator comparator = new StandardTypeComparator();
+		assertThat(comparator.compare(null, "abc")).isNegative();
+		assertThat(comparator.compare(null, null)).isZero();
+		assertThat(comparator.compare("abc", null)).isPositive();
+	}
+
+	@Test
+	void testObjects() throws EvaluationException {
+		TypeComparator comparator = new StandardTypeComparator();
+		assertThat(comparator.compare("a", "a")).isZero();
+		assertThat(comparator.compare("a", "b")).isNegative();
+		assertThat(comparator.compare("b", "a")).isPositive();
+	}
+
+	@Test
+	void testCanCompare() throws EvaluationException {
+		TypeComparator comparator = new StandardTypeComparator();
+		assertThat(comparator.canCompare(null, 1)).isTrue();
+		assertThat(comparator.canCompare(1, null)).isTrue();
+
+		assertThat(comparator.canCompare(2, 1)).isTrue();
+		assertThat(comparator.canCompare("abc", "def")).isTrue();
+		assertThat(comparator.canCompare("abc", 3)).isFalse();
+		assertThat(comparator.canCompare(String.class, 3)).isFalse();
+	}
+
+	@Test
+	public void shouldUseCustomComparator() {
+		TypeComparator comparator = new StandardTypeComparator();
+		ComparableType t1 = new ComparableType(1);
+		ComparableType t2 = new ComparableType(2);
+
+		assertThat(comparator.canCompare(t1, 2)).isFalse();
+		assertThat(comparator.canCompare(t1, t2)).isTrue();
+		assertThat(comparator.compare(t1, t1)).isZero();
+		assertThat(comparator.compare(t1, t2)).isNegative();
+		assertThat(comparator.compare(t2, t1)).isPositive();
+	}
+
+	static class ComparableType implements Comparable<ComparableType> {
+
+		private final int id;
+
+		public ComparableType(int id) {
+			this.id = id;
+		}
+
+		@Override
+		public int compareTo(@NotNull ComparableType other) {
+			return this.id - other.id;
+		}
+
+	}
+
+}