Set.contains is incorrectly requiring a non-nullable input arg, can it be updated to allow null in?

[spacether]

Thanks for submitting an issue.
As explained in the instructions for submitting an issue at https://checkerframework.org/manual/#reporting-bugs, please include four pieces of information:

• commands (that can be cut-and-pasted into a command shell),
• inputs,
• outputs, and
• expectation.

Set.contains is incorrectly requiring a non-nullable input arg, update it?
When I have this code:

Set<@Nullable Object> enumValues = new HashSet<>();
// ... some code where null may be added to the Set
if (enumValues.contains(arg)) {
    return null;
}

The NullnessChecker complains:

java: [argument] incompatible argument for parameter arg0 of contains.
  found   : @Initialized @Nullable Object
  required: @Initialized @NonNull Object

So I am correctly passing in Nullable.
Per the Set.contains interface:

    /**
     * Returns {@code true} if this set contains the specified element.
     * More formally, returns {@code true} if and only if this set
     * contains an element {@code e} such that
     * {@code Objects.equals(o, e)}.
     *
     * @param o element whose presence in this set is to be tested
     * @return {@code true} if this set contains the specified element
     * @throws ClassCastException if the type of the specified element
     *         is incompatible with this set
     * (<a href="Collection.html#optional-restrictions">optional</a>)
     * @throws NullPointerException if the specified element is null and this
     *         set does not permit null elements
     * (<a href="Collection.html#optional-restrictions">optional</a>)
     */

So null might be allowed as a value and it might be disallowed depending on the Map implementation.
So the default should be to allow it, and disallow it where denylisting applies.
Can this behavior be updated?

I expect this code to pass type checking because the map contains interface should allow in null as a possible value to look for. HashSet allows in nulls for example.

[spacether]

Hmm per the docs

3.4.2 Null arguments to collection classes

For collection methods with Object formal parameter type, such as contains, indexOf, and remove, the annotated JDK forbids null as an argument.

The reason is that some implementations (like ConcurrentHashMap) throw NullPointerException if null is passed. It would be unsound to permit null, because it could lead to a false negative: the Checker Framework issuing no warning even though a NullPointerException can occur at run time.

However, many other common implementations permit such calls, so some users may wish to sacrifice soundness for a reduced number of false positive warnings. To permit null as an argument to these methods, pass the command-line argument -Astubs=collection-object-parameters-may-be-null.astub.

Nulls are still allowed into these methods, so my preference would be to allow them in by default, and throw checker warnings when classes that deny nulls into these methods are invoked with nulls. You can still provide null safety for all use cases (classes that do/don't allow null into these methods), just track instantiated classes a little more closely to know if they allow null into these methods or not.

[mernst]

That is exactly the manual text I was going to paste in. Thanks for finding it on your own!

Your idea is a very good one: track, for every collection, whether or not it supports null. That will improve precision. We'll add this to our backlog of tasks. I'm not sure when we will get to it, because we have limited manpower. If you want to help, we would gratefully accept assistance in implementing the feature.

[spacether]

You're welcome. Thank you for writing thorough docs.

Thank you for adding the idea to the backlog and for your hard work on this useful tool.
It is working very well for me in this PR openapi-json-schema-tools/openapi-json-schema-generator#340 and I will suppress these checks in my project.
I don't have time to help. Thank you for your work on this tool :)

[mernst]

I'm closing this because the specific issue is resolved.
The idea of writing a "collection supports null" checker is a good one, but is a big project.