From 0dfe07c29a2b6e4ba867fa59eaab33fb505fc23b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lu=C3=ADs=20Mendes?= <luis.mendes@cleverthis.com>
Date: Tue, 12 Nov 2024 21:47:57 +0000
Subject: [PATCH] Fix SortedList.addAll(...) to be JLS/API compliant and
 provide unit tests (Fixes #6034)

SortedList.addAll(...) should return a boolean value indicating if the Collection was modified is any way as a result of the call
---
 .../java/spoon/support/util/SortedList.java   |   4 +-
 .../spoon/support/util/SortedListTest.java    | 154 ++++++++++++++++++
 2 files changed, 156 insertions(+), 2 deletions(-)
 create mode 100644 src/test/java/spoon/support/util/SortedListTest.java

diff --git a/src/main/java/spoon/support/util/SortedList.java b/src/main/java/spoon/support/util/SortedList.java
index 34033b4b51d..c14075855ea 100644
--- a/src/main/java/spoon/support/util/SortedList.java
+++ b/src/main/java/spoon/support/util/SortedList.java
@@ -42,9 +42,9 @@ public void add(int index, E element) {
 
 	@Override
 	public boolean addAll(Collection<? extends E> c) {
-		boolean ret = true;
+		boolean ret = false;
 		for (E e : c) {
-			ret &= add(e);
+			ret |= add(e);
 		}
 		return ret;
 	}
diff --git a/src/test/java/spoon/support/util/SortedListTest.java b/src/test/java/spoon/support/util/SortedListTest.java
new file mode 100644
index 00000000000..18bbfec2d3c
--- /dev/null
+++ b/src/test/java/spoon/support/util/SortedListTest.java
@@ -0,0 +1,154 @@
+package spoon.support.util;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+
+import spoon.reflect.declaration.CtClass;
+import spoon.reflect.declaration.CtElement;
+
+public class SortedListTest {
+    public class BasicComparator implements Comparator<CtElement> {
+
+        private final List<CtElement> myOrdering;
+        
+        public BasicComparator(List<CtElement> orderList) {
+            this.myOrdering = orderList;
+        }
+        
+        @Override
+        public int compare(CtElement o1, CtElement o2) {
+            int idxFirst = - 1;
+            int idxSecond = -1;
+            int idx = 1;
+            for(CtElement clazz : myOrdering) {
+                if (o1 == o2 && o1 == clazz) {
+                    return 0;
+                } else if (clazz == o1) {
+                    if (idxFirst == -1) {
+                        idxFirst = idx;
+                        idx++;    
+                    } else {
+                        throw new IllegalStateException();
+                    }
+                    
+                } else if (clazz == o2) {
+                    if (idxSecond == -1) {
+                        idxSecond = idx;
+                        idx++;
+                    } else {
+                        throw new IllegalStateException();
+                    }
+                };
+            }
+            
+            return idxFirst - idxSecond;
+        }
+        
+    }
+    
+    @Test
+    public void sortedListUsesComparatorTestPass() {
+        Comparator<CtClass<?>> cmp1 = mock();
+        SortedList<CtClass<?>> sl1 = new SortedList<>(cmp1);
+        CtClass<?> lc1 = mock();
+        CtClass<?> lc2 = mock();
+        assertTrue(sl1.add(lc1), "Element should have been inserted");
+        assertTrue(sl1.add(lc2), "Element should have been inserted");
+        verify(cmp1).compare(lc2, lc1);
+    }
+    
+    @Test
+    public void verifyAddWithIndexThrowsException() {
+        Comparator<CtClass<?>> cmp1 = mock();
+        SortedList<CtClass<?>> sl1 = new SortedList<>(cmp1);
+        CtClass<?> lc1 = mock();
+        IllegalArgumentException ex = assertThrows( 
+                IllegalArgumentException.class, 
+                new Executable() {
+
+                    @Override
+                    public void execute() throws Throwable {
+                        sl1.add(1, lc1);
+                    }
+                    
+                },
+                "Add with index should trhow IllegalArgumentException");
+        assertEquals(
+                "cannot force a position with a sorted list that has its own ordering",
+                ex.getMessage(),
+                "Exception message does not match the expected");
+    }
+    
+    @Test
+    public void verifyThatGetComparatorReturnsTheComparatorTestPass() {
+        Comparator<CtClass<?>> cmp1 = mock();
+        SortedList<CtClass<?>> sl1 = new SortedList<>(cmp1);
+        assertTrue(cmp1 == sl1.getComparator(), "Comparator does not match the expected");
+    }
+    
+    @Test
+    public void verifyThatComparatorCanBeReplacedTestPass() {
+        Comparator<? super CtClass<?>> cmp1 = mock();
+        SortedList<CtClass<?>> sl1 = new SortedList<>(cmp1);
+        BasicComparator bc1 = new BasicComparator(null);
+        sl1.setComparator(bc1);
+        assertTrue(bc1 == sl1.getComparator(), "Comparator does not match the expected");
+    }
+    
+    @Test
+    public void addAllInsertsAllElementsAndTheyAreOrderedTestPass() {
+        List<CtElement> orderedList = new ArrayList<>(3);        
+        CtClass<?> lc1 = mock();
+        CtClass<?> lc2 = mock();
+        CtClass<?> lc3 = mock();
+        orderedList.add(lc1);
+        orderedList.add(lc2);
+        orderedList.add(lc3);
+        
+        BasicComparator bc1 = new BasicComparator(orderedList);
+        SortedList<CtElement> sl1 = new SortedList<>(bc1);
+        
+        List<CtElement> unorderedList = new ArrayList<>(3);
+        unorderedList.add(lc3);
+        unorderedList.add(lc1);
+        unorderedList.add(lc2);
+        assertTrue(sl1.addAll(unorderedList), "All elements should have been added");
+        
+        //Verify ordering
+        assertTrue(sl1.get(0) == lc1, 
+                "First element does not match the expected ordering");
+        assertTrue(sl1.get(1) == lc2, 
+                "Second element does not match the expected ordering");
+        assertTrue(sl1.get(2) == lc3,
+                "Third element does not match the expected ordering");
+    }
+ 
+    @Test
+    public void addAllWithEmptyListShouldLeaveTheListUnchangedTestPass() {
+        List<CtElement> orderedList = new ArrayList<>(3);        
+        CtClass<?> lc1 = mock();
+        CtClass<?> lc2 = mock();
+        CtClass<?> lc3 = mock();
+        orderedList.add(lc1);
+        orderedList.add(lc2);
+        orderedList.add(lc3);
+        
+        BasicComparator bc1 = new BasicComparator(orderedList);
+        SortedList<CtElement> sl1 = new SortedList<>(bc1);
+
+        assertTrue(sl1.add(lc1), "List should have changed");
+        assertFalse(sl1.addAll(new LinkedList<>()), "List should not have changed");
+    }
+}
\ No newline at end of file