Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
CoreRasurae opened this issue Oct 23, 2024 · 2 comments · Fixed by #6049
Closed
Labels

Comments

@CoreRasurae
Copy link
Contributor

CoreRasurae commented Oct 23, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

Sure, i can submit a pull request for that.

CoreRasurae added a commit to CoreRasurae/spoon that referenced this issue Oct 28, 2024
…ests (Fixes INRIA#6034)

SortedList.addAll(...) should return a boolean value indicating if the Collection was modified is any way as a result of the call
CoreRasurae added a commit to CoreRasurae/spoon that referenced this issue Oct 29, 2024
…ests (Fixes INRIA#6034)

SortedList.addAll(...) should return a boolean value indicating if the Collection was modified is any way as a result of the call
CoreRasurae pushed a commit to CoreRasurae/spoon that referenced this issue Nov 12, 2024
…ests (Fixes INRIA#6034)

SortedList.addAll(...) should return a boolean value indicating if the Collection was modified is any way as a result of the call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants
@algomaster99 @CoreRasurae and others