From 5f250ead2df52d7fe26a3ed2bdd7a38355f764b1 Mon Sep 17 00:00:00 2001 From: CoreRasurae <38363426+CoreRasurae@users.noreply.github.com> Date: Fri, 15 Nov 2024 07:21:12 +0000 Subject: [PATCH] fix: Make SortedList#add return value adhere to List contract (#6049) --- .../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 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 { + + private final List myOrdering; + + public BasicComparator(List 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> cmp1 = mock(); + SortedList> 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> cmp1 = mock(); + SortedList> 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> cmp1 = mock(); + SortedList> sl1 = new SortedList<>(cmp1); + assertTrue(cmp1 == sl1.getComparator(), "Comparator does not match the expected"); + } + + @Test + public void verifyThatComparatorCanBeReplacedTestPass() { + Comparator> cmp1 = mock(); + SortedList> 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 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 sl1 = new SortedList<>(bc1); + + List 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 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 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