Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8265905

ImmutableCollections: Querying for 'null' should return 'false' instead of NPE



    • Type: Enhancement
    • Status: Closed
    • Priority: P4
    • Resolution: Won't Fix
    • Affects Version/s: 16
    • Fix Version/s: None
    • Component/s: core-libs


      # Overview
      Currently the collections in java.util.ImmutableCollections do not support null values. When querying these collections for null, the query crashes with a NullPointerException.

      # Proposed Change
      Change the querying methods (contains, indexOf, lastIndexOf) in the different ImmutableCollections to return false when querying for null instead of NullPointerException.

      List<Integer> l = List.of(1, 2);
      l.contains(null); // Should not crash

      # Reasoning
      Code accepting the List interface can break easily because other List implementations like ArrayList can be queried for null. If now someone passes in an ImmutableCollections collection, it will crash. I think the change would make ImmutableCollections more consistent with the user's expectations as well as other implementations of List.

      Without this change, libraries would probably have to change many occurrences of contains/indexOf to make sure null is never passed in (or always put all values into an ArrayList before querying).

      Right now this is probably not happening as often because there are not many ways to create these collections. However this only makes it much more likely to generate bugs since programmers might not be aware of this behavior.

      Also ImmutableCollections.ListN already has the option to allow nulls. So even when looking at ListN it is unclear if it is allowed to call contains(null) or not.

      # Would this conflict with the Spec?
      According to the API Docs, I would say it doesn't:
      List#contains says the following about NPE:
      "NullPointerException - if the specified element is null and this list does not permit null elements (optional)"
      - https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/List.html#contains(java.lang.Object)

      However this behavior is optional. Quoting the API Docs:
      "Attempting to query the presence of an ineligible element may throw an exception, or it may simply return false; some implementations will exhibit the former behavior and some will exhibit the latter. More generally, attempting an operation on an ineligible element whose completion would not result in the insertion of an ineligible element into the collection may throw an exception or it may succeed, at the option of the implementation. Such exceptions are marked as "optional" in the specification for this interface."
      - https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/Collection.html#optional-restrictions

      As a sidenote: It would be cleanest to completely remove the option of throwing an NPE from the API. However there are probably some corner cases where this is necessary and the change would break all existing implementations that throw NPEs.




            forax RĂ©mi Forax
            webbuggrp Webbug Group
            0 Vote for this issue
            4 Start watching this issue