eisop / checker-framework

Pluggable type-checking for Java
https://eisop.github.io/
Other
15 stars 16 forks source link

[Nullness checker] Use null value in stream #759

Open Ao-senXiong opened 2 months ago

Ao-senXiong commented 2 months ago

Take this example,

import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;

public class Example {
    public void doSomething() {
        List<@Nullable Integer> list = Arrays.asList(null, 1, 2);
        var comparator =
                Comparator.<Integer>nullsLast(Comparator.naturalOrder());
        System.out.println(list.stream().max(comparator));
    }

    public static void main(String[] args) {
        Example example = new Example();
        example.doSomething();
    }
}

Command to run it, nullness checker is fine with it, but end up in NPE.

java -jar checker/dist/checker.jar -processor nullness Example.java
Exception in thread "main" java.lang.NullPointerException
        at java.base/java.util.Objects.requireNonNull(Objects.java:222)
        at java.base/java.util.Optional.<init>(Optional.java:107)
        at java.base/java.util.Optional.of(Optional.java:120)
        at java.base/java.util.stream.ReduceOps$2ReducingSink.get(ReduceOps.java:129)
        at java.base/java.util.stream.ReduceOps$2ReducingSink.get(ReduceOps.java:107)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:558)
        at java.base/java.util.stream.ReferencePipeline.max(ReferencePipeline.java:594)
        at Example.doSomething(Example.java:11)
        at Example.main(Example.java:16)
wmdietl commented 2 months ago

Thanks for the report!

Even though you provide an explicit type argument to nullsLast in Comparator.<Integer>nullsLast the type for comparator should still accept @Nullable: https://github.com/eisop/jdk/blob/d1b063fab9b8d5141bc240ec10f6b9ebcafb9d6b/src/java.base/share/classes/java/util/Comparator.java#L413 Can you confirm that's correct? Why did you provide the explicit type argument?

The documentation for Stream#min/max say that they NPE if the min/max is null. It seems okay to pass a comparator that accepts null, but there is no way to say that null should never be the maximum.

https://github.com/eisop/jdk/blob/d1b063fab9b8d5141bc240ec10f6b9ebcafb9d6b/src/java.base/share/classes/java/util/stream/Stream.java#L1244 We could add a receiver type restriction, only allowing min/max on streams that contain no null values at all. That might be overly conservative, but would get rid of this NPE.

It would be nice to forbid nullsLast with max and nullsFirst with min, only on nullable streams. But we would need some separate way to track that and that seems like overkill.

Do you see alternatives?