Collection.contains() and Object.equals()
Mahmood Ali
> A related question: for legacy reasons, Collection<T>.contains() takes
> an Object, not T, as an argument. Is there a way to make the checker
> permit contains(null) calls on Collection<@Nullable String>, but
> reject it on Collection<String> (eg what would I need to write in the
> stub file for the Collection class)?
This is another example on Collections API breaking the substitution
principle! Concrete classes differ on whither contains(null) throws
an Exception or not, so they cannot be substitutable!
I agree with you. It'll be great to write another checker (or a
Checker plugin) to handle Collections.contains() and Object.equals().
I have been bitten myself multiple times by calling
String.equals(CharSequence) or Collections<Name>.contains(String).
A Checker might be able to handle 80% of the cases by treating the
declarations of those two methods as (invalid in Java)
interface Collection<E> { boolean contains(<? super E> object); }
class Object { boolean equals(<? super SelfType> other) { ... } }
This wouldn't handle cases like Collection.equals (only the elements
matter, not the actual Collection type), or Date.equals (and its
interaction with sql TimeStamp).
We would appreciate any contributions towards having a reliable
checker for this problem.
- Mahmood
Artemus Harper
What would be needed is a separate checker that would warn when an operation doesn't modify the receiver or the parameters, and always has the same result for the known static information of the receiver and parameters.
E.g.
public boolean contains(@Result(exception=NullPointerException.class,valueIsNull=true) Object o);
which could have an 'alias' annotation of:
public boolean contains(@NullCheck Object o);
which would in normal compiler setting issue a warning if null was passed as a parameter to contains on a class known to throw NullPointerException on null values. Tweaking of this to an error, or for values that could possibly be null, should be a setting when the annotation processor is run (the API should tell you what its code does, not how to write your own).
An example of add for unmodifiable collections would be:
public boolean add(E val) @Result(exception=UnsupportedOperationException.class);
Michael Ernst
> operation, and the valid responses are 'true','false' or
> NullPointerException. Implementors are expected to handle null
> parameters, even if all they do is throw an exception.
This is a valid point of view, and the one that is used by people working
in the field of formal specifications.
Unfortunately, this is not a useful perspective from the point of view of
preventing null pointer errors -- which is why someone uses the nullness
checker of the Checker Framework.
Section 2.4.1 discusses this issue; see
As always, feel free to let me know if there are ways that section should
be improved.
> public boolean contains(@Result(exception=NullPointerException.class,valueIsNull=true) Object o);
>
> which could have an 'alias' annotation of:
> public boolean contains(@NullCheck Object o);
>
> which would in normal compiler setting issue a warning if null was passed
> as a parameter to contains on a class known to throw NullPointerException
> on null values. Tweaking of this to an error, or for values that could
> possibly be null, should be a setting when the annotation processor is
> run (the API should tell you what its code does, not how to write your
> own).
This has the feel of FindBugs.
FindBugs takes the approach of annotating a declaration, and thus
suppressing checking at all client uses, even the places that you want to
check. It is better to suppress warnings at only the specific client uses
where the value is known to be non-null; the Checker Framework supports
this, if you write @SuppressWarnings at the client uses. The Checker
Framework also supports suppressing checking at all client uses, by writing
a @SuppressWarnings annotation at the declaration site. So, you can choose
which approach you want.
-Mike