Merge pull request #25478 from wind57

* pr/25478:
  Polish "Use Comparable instead of dedicated implementations"
  Use Comparable instead of dedicated implementations

Closes gh-25478

spring-core/src/main/java/org/springframework/util/comparator/BooleanComparator.java

@@ -26,6 +26,7 @@
  * {@code true} or {@code false} first.
  *
  * @author Keith Donald
+ * @author Eugene Rabii
  * @since 1.2.2
  */
 @SuppressWarnings("serial")
@@ -63,8 +64,9 @@ public BooleanComparator(boolean trueLow) {
 
 
 	@Override
-	public int compare(Boolean v1, Boolean v2) {
-		return (v1 ^ v2) ? ((v1 ^ this.trueLow) ? 1 : -1) : 0;
+	public int compare(Boolean left, Boolean right) {
+		int multiplier = this.trueLow ? -1 : 1;
+		return multiplier * Boolean.compare(left, right);
 	}
 
 
@@ -75,7 +77,7 @@ public boolean equals(@Nullable Object other) {
 
 	@Override
 	public int hashCode() {
-		return getClass().hashCode() * (this.trueLow ? -1 : 1);
+		return Boolean.hashCode(this.trueLow);
 	}
 
 	@Override

spring-core/src/main/java/org/springframework/util/comparator/ComparableComparator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -27,7 +27,9 @@
  * @since 1.2.2
  * @param <T> the type of comparable objects that may be compared by this comparator
  * @see Comparable
+ * @deprecated as of 6.1 in favor of {@link Comparator#naturalOrder()}
  */
+@Deprecated(since = "6.1")
 public class ComparableComparator<T extends Comparable<T>> implements Comparator<T> {
 
 	/**

spring-core/src/main/java/org/springframework/util/comparator/Comparators.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -29,49 +29,47 @@ public abstract class Comparators {
 
 	/**
 	 * Return a {@link Comparable} adapter.
-	 * @see ComparableComparator#INSTANCE
+	 * @see Comparator#naturalOrder()
 	 */
 	@SuppressWarnings("unchecked")
 	public static <T> Comparator<T> comparable() {
-		return ComparableComparator.INSTANCE;
+		return (Comparator<T>) Comparator.naturalOrder();
 	}
 
 	/**
 	 * Return a {@link Comparable} adapter which accepts
 	 * null values and sorts them lower than non-null values.
-	 * @see NullSafeComparator#NULLS_LOW
+	 * @see Comparator#nullsLast(Comparator)
 	 */
-	@SuppressWarnings("unchecked")
 	public static <T> Comparator<T> nullsLow() {
-		return NullSafeComparator.NULLS_LOW;
+		return nullsLow(comparable());
 	}
 
 	/**
 	 * Return a decorator for the given comparator which accepts
 	 * null values and sorts them lower than non-null values.
-	 * @see NullSafeComparator#NullSafeComparator(boolean)
+	 * @see Comparator#nullsLast(Comparator)
 	 */
 	public static <T> Comparator<T> nullsLow(Comparator<T> comparator) {
-		return new NullSafeComparator<>(comparator, true);
+		return Comparator.nullsLast(comparator);
 	}
 
 	/**
 	 * Return a {@link Comparable} adapter which accepts
 	 * null values and sorts them higher than non-null values.
-	 * @see NullSafeComparator#NULLS_HIGH
+	 * @see Comparator#nullsFirst(Comparator)
 	 */
-	@SuppressWarnings("unchecked")
 	public static <T> Comparator<T> nullsHigh() {
-		return NullSafeComparator.NULLS_HIGH;
+		return nullsHigh(comparable());
 	}
 
 	/**
 	 * Return a decorator for the given comparator which accepts
 	 * null values and sorts them higher than non-null values.
-	 * @see NullSafeComparator#NullSafeComparator(boolean)
+	 * @see Comparator#nullsFirst(Comparator)
 	 */
 	public static <T> Comparator<T> nullsHigh(Comparator<T> comparator) {
-		return new NullSafeComparator<>(comparator, false);
+		return Comparator.nullsFirst(comparator);
 	}
 
 }

spring-core/src/main/java/org/springframework/util/comparator/NullSafeComparator.java

@@ -30,7 +30,10 @@
  * @since 1.2.2
  * @param <T> the type of objects that may be compared by this comparator
  * @see Comparable
+ * @see Comparators
+ * @deprecated as of 6.1 in favor of {@link Comparator#nullsLast} and {@link Comparator#nullsFirst}
  */
+@Deprecated(since = "6.1")
 public class NullSafeComparator<T> implements Comparator<T> {
 
 	/**
@@ -69,9 +72,8 @@ public class NullSafeComparator<T> implements Comparator<T> {
 	 * @see #NULLS_LOW
 	 * @see #NULLS_HIGH
 	 */
-	@SuppressWarnings("unchecked")
 	private NullSafeComparator(boolean nullsLow) {
-		this.nonNullComparator = ComparableComparator.INSTANCE;
+		this.nonNullComparator = Comparators.comparable();
 		this.nullsLow = nullsLow;
 	}
 
@@ -92,17 +94,9 @@ public NullSafeComparator(Comparator<T> comparator, boolean nullsLow) {
 
 
 	@Override
-	public int compare(@Nullable T o1, @Nullable T o2) {
-		if (o1 == o2) {
-			return 0;
-		}
-		if (o1 == null) {
-			return (this.nullsLow ? -1 : 1);
-		}
-		if (o2 == null) {
-			return (this.nullsLow ? 1 : -1);
-		}
-		return this.nonNullComparator.compare(o1, o2);
+	public int compare(@Nullable T left, @Nullable T right) {
+		Comparator<T> comparator = this.nullsLow ? Comparator.nullsFirst(this.nonNullComparator) : Comparator.nullsLast(this.nonNullComparator);
+		return comparator.compare(left, right);
 	}
 
 
@@ -115,7 +109,7 @@ public boolean equals(@Nullable Object other) {
 
 	@Override
 	public int hashCode() {
-		return this.nonNullComparator.hashCode() * (this.nullsLow ? -1 : 1);
+		return Boolean.hashCode(this.nullsLow);
 	}
 
 	@Override

spring-core/src/test/java/org/springframework/core/convert/converter/ConvertingComparatorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -26,7 +26,7 @@
 
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.support.DefaultConversionService;
-import org.springframework.util.comparator.ComparableComparator;
+import org.springframework.util.comparator.Comparators;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -95,17 +95,17 @@ private void testConversion(ConvertingComparator<String, Integer> convertingComp
 	}
 
 	@Test
-	void shouldGetMapEntryKeys() throws Exception {
+	void shouldGetMapEntryKeys() {
 		ArrayList<Entry<String, Integer>> list = createReverseOrderMapEntryList();
-		Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryKeys(new ComparableComparator<String>());
+		Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryKeys(Comparators.comparable());
 		list.sort(comparator);
 		assertThat(list.get(0).getKey()).isEqualTo("a");
 	}
 
 	@Test
-	void shouldGetMapEntryValues() throws Exception {
+	void shouldGetMapEntryValues() {
 		ArrayList<Entry<String, Integer>> list = createReverseOrderMapEntryList();
-		Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryValues(new ComparableComparator<Integer>());
+		Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryValues(Comparators.comparable());
 		list.sort(comparator);
 		assertThat(list.get(0).getValue()).isEqualTo(1);
 	}
@@ -130,7 +130,7 @@ public Integer convert(String source) {
 	}
 
 
-	private static class TestComparator extends ComparableComparator<Integer> {
+	private static class TestComparator implements Comparator<Integer> {
 
 		private boolean called;
 
@@ -139,7 +139,7 @@ public int compare(Integer o1, Integer o2) {
 			assertThat(o1).isInstanceOf(Integer.class);
 			assertThat(o2).isInstanceOf(Integer.class);
 			this.called = true;
-			return super.compare(o1, o2);
+			return Comparators.comparable().compare(o1, o2);
 		}
 
 		public void assertCalled() {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -36,8 +36,7 @@
 import org.springframework.util.ConcurrentReferenceHashMap.Entry;
 import org.springframework.util.ConcurrentReferenceHashMap.Reference;
 import org.springframework.util.ConcurrentReferenceHashMap.Restructure;
-import org.springframework.util.comparator.ComparableComparator;
-import org.springframework.util.comparator.NullSafeComparator;
+import org.springframework.util.comparator.Comparators;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -51,8 +50,7 @@
  */
 class ConcurrentReferenceHashMapTests {
 
-	private static final Comparator<? super String> NULL_SAFE_STRING_SORT = new NullSafeComparator<>(
-			new ComparableComparator<String>(), true);
+	private static final Comparator<? super String> NULL_SAFE_STRING_SORT = Comparators.nullsLow();
 
 	private TestWeakConcurrentCache<Integer, String> map = new TestWeakConcurrentCache<>();
 

spring-core/src/test/java/org/springframework/util/comparator/BooleanComparatorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -29,34 +29,35 @@
  * @author Keith Donald
  * @author Chris Beams
  * @author Phillip Webb
+ * @author Eugene Rabii
  */
 class BooleanComparatorTests {
 
 	@Test
 	void shouldCompareWithTrueLow() {
 		Comparator<Boolean> c = new BooleanComparator(true);
-		assertThat(c.compare(true, false)).isEqualTo(-1);
+		assertThat(c.compare(true, false)).isLessThan(0);
 		assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
 	}
 
 	@Test
 	void shouldCompareWithTrueHigh() {
 		Comparator<Boolean> c = new BooleanComparator(false);
-		assertThat(c.compare(true, false)).isEqualTo(1);
+		assertThat(c.compare(true, false)).isGreaterThan(0);
 		assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
 	}
 
 	@Test
 	void shouldCompareFromTrueLow() {
 		Comparator<Boolean> c = BooleanComparator.TRUE_LOW;
-		assertThat(c.compare(true, false)).isEqualTo(-1);
+		assertThat(c.compare(true, false)).isLessThan(0);
 		assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
 	}
 
 	@Test
 	void shouldCompareFromTrueHigh() {
 		Comparator<Boolean> c = BooleanComparator.TRUE_HIGH;
-		assertThat(c.compare(true, false)).isEqualTo(1);
+		assertThat(c.compare(true, false)).isGreaterThan(0);
 		assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
 	}
 

spring-core/src/test/java/org/springframework/util/comparator/ComparableComparatorTests.java

@@ -30,6 +30,7 @@
  * @author Chris Beams
  * @author Phillip Webb
  */
+@Deprecated
 class ComparableComparatorTests {
 
 	@Test

spring-core/src/test/java/org/springframework/util/comparator/NullSafeComparatorTests.java

@@ -29,6 +29,7 @@
  * @author Chris Beams
  * @author Phillip Webb
  */
+@Deprecated
 class NullSafeComparatorTests {
 
 	@SuppressWarnings("unchecked")