You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2025/05/21 07:30:16 UTC
[PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
gnodet opened a new pull request, #2363:
URL: https://github.com/apache/maven/pull/2363
- Replace custom List implementation with standard List.of() and List.copyOf()
- Remove unnecessary custom list implementations (List1, List2, ListN, etc.)
- Remove ROProperties implementation as it's no longer needed
- Add documentation note referencing JDK issue JDK-8323729
- Reduce code complexity and maintenance burden by leveraging JDK standard APIs
This change aligns with the goal of using standard Java APIs where possible
instead of maintaining custom implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "pzygielo (via GitHub)" <gi...@apache.org>.
pzygielo commented on PR #2363:
URL: https://github.com/apache/maven/pull/2363#issuecomment-2910346144
> one landmine to watch out for is that JDK's immutable collections can't contain null.
https://bugs.openjdk.org/browse/JDK-8265905
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #2363:
URL: https://github.com/apache/maven/pull/2363#issuecomment-2910336190
one landmine to watch out for is that JDK's immutable collections can't contain null.
```java
List.of("duke").contains(null); // even contains throws NPE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #2363:
URL: https://github.com/apache/maven/pull/2363#discussion_r2100460591
##########
src/mdo/java/ImmutableCollections.java:
##########
@@ -79,32 +65,7 @@ public int size() {
};
static <E> List<E> copy(Collection<E> collection) {
- if (collection == null) {
- return emptyList();
- } else if (collection instanceof AbstractImmutableList) {
- return (List<E>) collection;
- } else {
- switch (collection.size()) {
- case 0:
- return emptyList();
- case 1:
- return singletonList(collection.iterator().next());
- case 2:
- Iterator<E> it = collection.iterator();
- return new List2<>(it.next(), it.next());
- default:
- return new ListN<>(collection);
- }
- }
- }
-
- @SuppressWarnings("unchecked")
- static <E> List<E> emptyList() {
- return (List<E>) EMPTY_LIST;
- }
-
- static <E> List<E> singletonList(E element) {
- return new List1<>(element);
+ return collection == null ? List.of() : List.copyOf(collection);
Review Comment:
Why not `Collections.emptyList()`? Does the empty list needs to be mutable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #2363:
URL: https://github.com/apache/maven/pull/2363#discussion_r2100460591
##########
src/mdo/java/ImmutableCollections.java:
##########
@@ -79,32 +65,7 @@ public int size() {
};
static <E> List<E> copy(Collection<E> collection) {
- if (collection == null) {
- return emptyList();
- } else if (collection instanceof AbstractImmutableList) {
- return (List<E>) collection;
- } else {
- switch (collection.size()) {
- case 0:
- return emptyList();
- case 1:
- return singletonList(collection.iterator().next());
- case 2:
- Iterator<E> it = collection.iterator();
- return new List2<>(it.next(), it.next());
- default:
- return new ListN<>(collection);
- }
- }
- }
-
- @SuppressWarnings("unchecked")
- static <E> List<E> emptyList() {
- return (List<E>) EMPTY_LIST;
- }
-
- static <E> List<E> singletonList(E element) {
- return new List1<>(element);
+ return collection == null ? List.of() : List.copyOf(collection);
Review Comment:
Why not `Collections.emptyList()`? Does the empty list needs to be mutable? The classname (`ImmutableCollections.java`) says no.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #2363:
URL: https://github.com/apache/maven/pull/2363#discussion_r2105317810
##########
src/mdo/java/ImmutableCollections.java:
##########
@@ -79,32 +65,7 @@ public int size() {
};
static <E> List<E> copy(Collection<E> collection) {
Review Comment:
```suggestion
static <E1, E2 extends E1> List<E1> copy(Collection<E2> collection) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #2363:
URL: https://github.com/apache/maven/pull/2363#discussion_r2100473936
##########
src/mdo/java/ImmutableCollections.java:
##########
@@ -79,32 +65,7 @@ public int size() {
};
static <E> List<E> copy(Collection<E> collection) {
- if (collection == null) {
- return emptyList();
- } else if (collection instanceof AbstractImmutableList) {
- return (List<E>) collection;
- } else {
- switch (collection.size()) {
- case 0:
- return emptyList();
- case 1:
- return singletonList(collection.iterator().next());
- case 2:
- Iterator<E> it = collection.iterator();
- return new List2<>(it.next(), it.next());
- default:
- return new ListN<>(collection);
- }
- }
- }
-
- @SuppressWarnings("unchecked")
- static <E> List<E> emptyList() {
- return (List<E>) EMPTY_LIST;
- }
-
- static <E> List<E> singletonList(E element) {
- return new List1<>(element);
+ return collection == null ? List.of() : List.copyOf(collection);
Review Comment:
List.of() also returns an immutable list (although not the same objectas Collections.emptyList()).
It's a more concise form, and I think the best practice will be to use List.of() and Map.of() instead of the similar methods from Collections.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: Simplify immutable collections implementation by using standard Java APIs [maven]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #2363:
URL: https://github.com/apache/maven/pull/2363#discussion_r2107677304
##########
src/mdo/java/ImmutableCollections.java:
##########
@@ -79,32 +65,7 @@ public int size() {
};
static <E> List<E> copy(Collection<E> collection) {
- if (collection == null) {
- return emptyList();
- } else if (collection instanceof AbstractImmutableList) {
- return (List<E>) collection;
- } else {
- switch (collection.size()) {
- case 0:
- return emptyList();
- case 1:
- return singletonList(collection.iterator().next());
- case 2:
- Iterator<E> it = collection.iterator();
- return new List2<>(it.next(), it.next());
- default:
- return new ListN<>(collection);
- }
- }
- }
-
- @SuppressWarnings("unchecked")
- static <E> List<E> emptyList() {
- return (List<E>) EMPTY_LIST;
- }
-
- static <E> List<E> singletonList(E element) {
- return new List1<>(element);
+ return collection == null ? List.of() : List.copyOf(collection);
Review Comment:
another advantage of using `Foo.of()` is that any defensive `Foo.copyOf()` downstream becomes a no-op.
https://github.com/openjdk/jdk/blob/a300c356555019a42c19bf0c16184f6dee4ad96e/src/java.base/share/classes/java/util/ImmutableCollections.java#L186-L187
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org