INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.76k stars 352 forks source link

[Bug]: spoon.support.util.SortedList.addAll(...) return value is not JLS compliant #6034

Closed CoreRasurae closed 1 week ago

CoreRasurae commented 1 month ago

Describe the bug

According to Java JLS/API docs we see that: https://docs.oracle.com/javase/8/docs/api/?java/util/Collections.html addAll(...) Returns: true if the collection changed as a result of the call

However in spoon.support.util.SortedList.addAll(...) calling addAll(...) on an empty collection returns true, even though no actual change was made in the collections and secondly, if some element fails to be added, despite some of them are added will return false, when at least one object was inserted in the collection, and thus the collection was modified.

It should also be noted that LinkedList which SortedList extends always inserts all elements and returns true. It only returns false if no element could be added and that if some problem occurs at the middle of an insertion of multiple objects, an exception will be thrown.

We propose the following patch:

--- a/src/main/java/spoon/support/util/SortedList.java  2024-10-23 02:02:48.000000000 +0100
+++ b/src/main/java/spoon/support/util/SortedList.java  2024-10-23 17:25:47.531343136 +0100
@@ -42,9 +42,9 @@

    @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;
    }

Source code you are trying to analyze/transform

No response

Source code for your Spoon processing

No response

Actual output

No response

Expected output

No response

Spoon Version

latest from master

JVM Version

Java 17, but not relevant for this problem

What operating system are you using?

NixOS Linux, but again not relevant for this issue

algomaster99 commented 1 month ago

Thanks for the patch! I agree with the inconsistency. Would you be able to submit a pull request with this patch and a test case that would cover this patch?

CoreRasurae commented 1 month ago

Sure, i can submit a pull request for that.