Inappropriate Collection Call
ID |
java.inappropriate_collection_calls |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Java |
Tags |
best-practice, reliability |
Description
Reports a call to Collection.remove(Object), Collection.contains(Object),
Collection.removeAll(Collection), List.indexOf(Object) or List.lastIndexOf(Object) whose
argument’s declared type is unrelated to the collection’s own declared element type.
Map.containsKey/containsValue/get/getOrDefault/remove are a known limitation and are
never reported: the API used to read a collection’s element type collapses a `Map’s key/value
pair to no element type at all, so the key and value types cannot be told apart.
Rationale
Every one of these methods takes a bare Object argument and compares it against the elements
already in the collection. If the argument’s type cannot possibly be one of those elements — because neither type is a subtype of the other — the comparison is not occasionally wrong, it
is always wrong: contains/indexOf/lastIndexOf always return false/-1, remove/
removeAll always leave the collection untouched. The branch guarded by the result never runs,
and the removal that looked intentional never happens.
void purge(List<String> tags, Integer legacyId) {
tags.remove(legacyId); // always returns false: no String in tags equals an Integer
}Remediation
Convert the argument to the collection’s element type before the call, or fix the collection’s declared type if the argument’s type was the one that was actually intended:
void purge(List<String> tags, Integer legacyId) {
tags.remove(String.valueOf(legacyId)); // compares like with like
}