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

Undefined behavior in Javadoc for Immutable Collections copyOf method when passing null argumentlist1 ll #7617

Closed
2 tasks done
chevaris opened this issue Jan 14, 2025 · 1 comment
Labels
type=defect Bug, not working as expected

Comments

@chevaris
Copy link

chevaris commented Jan 14, 2025

Guava Version

33.4.0-jre

Description

When using copyOf(Collection<? extends E> elements) method of Immutable collection Javadoc is unclear about the behavior when passing null argument. It specifies that method throws NullPointerException when passed Collection has at least at element that is null (For instance an ArrayList with one of the items with null value), BUT it is NOT specified (or at least it is NOT 100% clear) the behavior if the collection object itself is null.

  • @throws NullPointerException if any of {@code elements} is null
    */
    public static ImmutableSortedSet copyOf(Collection<? extends E> elements) {

There are2 possible behaviors: one to throw NullPointerException (current behavior), BUT also another behavior could be to return and empty Immutable collection. Javadoc should be specific about the implemented behavior

I would re-write Javadoc as

*** @throws NullPointerException if {@code elements} is null or any of the elements of the collection is null**

Example

import java.util.ArrayList;
import java.util.List;

import com.google.common.collect.ImmutableSortedSet;

public class Example {

  public static void main(String[] args) {

    List<Integer> list1 = new ArrayList<>();
    list1.add(1);
    list1.add(null);

    // Throws NPE as defined in Javadoc
    try {
      ImmutableSortedSet<Integer> immutableSet1 = ImmutableSortedSet.copyOf(list1);
    } catch (NullPointerException e) {
      System.out.println("Expected NPE");
    }

    // Throws NPE but is undefined in Javadoc
    list1 = null;
    try {
      ImmutableSortedSet<Integer> immutableSet2 = ImmutableSortedSet.copyOf(list1);
    } catch (NullPointerException e) {
      System.out.println("Thrown NPE, BUT Javadoc does NOT specify it");
    }
  }
}

Output

Expected NPE
Thrown NPE, BUT Javadoc does NOT specify it

Expected Behavior

Javadoc is fixed

  • @throws NullPointerException if {@code elements} is null or any of the elements of the collection is null
    */
    public static ImmutableSortedSet copyOf(Collection<? extends E> elements) {

Actual Behavior

Javadoc is unclear

Packages

No response

Platforms

No response

Checklist

  • I agree to follow the code of conduct.

  • I can reproduce the bug with the latest version of Guava available.

@chevaris chevaris added the type=defect Bug, not working as expected label Jan 14, 2025
@chevaris chevaris changed the title Undefined behavior in Javadoc for Immutable Collections copyOf method when passing null argument Undefined behavior in Javadoc for Immutable Collections copyOf method when passing null argumentlist1 ll Jan 14, 2025
@cpovirk
Copy link
Member

cpovirk commented Jan 14, 2025

Most of our types are non-null unless they're annotated with @Nullable, as specified by the @NullMarked annotation on our package. (I'd like to make that clearer by putting @NullMarked on each individual class instead, but that can cause some technical issues for Java 8 users who do unusual things.)

Still, this particular method is awkward. I'll leave another comment on #2470, and you can follow up there.

@cpovirk cpovirk closed this as completed Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type=defect Bug, not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants