Could inspections be made to understand collections?

It's not my inspection, but that looks really hard. The big question you are asking is whether an inspection can use facts in it's dataflow algorithms from the List contract. That would be shockingly cool, but difficult to do in general. For instance, in your example the inference you wish to make would not be valid if the "items" collection is being modified in another thread. In that case result could indeed be null, if the items collection is cleaned out after it was asserted not to be empty. Wicked tough problems, and way valuable...

--Dave Griffith

Gordon Tyler wrote:

Hi,

I have some code that looks like this:

 public List someMethod(List items) {
>     assert items != null;
>     assert !items.isEmpty();
> 
>     List result = null;
>     for (Iterator it = stuff.iterator; it.hasNext();) {
>         Item item = (Item)it.next();
>         if (result == null) {
>             result = new ArrayList(item.getSomeListOfStuff());
>         } else {
>             result.retainAll(item.getSomeListOfStuff());
>         }
>     }
> 
>     result.add(SOME_CONSTANT);
> 
>     return result;
> }
> ]]>

The statement "result.add(SOME_CONSTANT)" is highlighted with the
warning "Method invocation '...' may produce 'NullPointerException'".

If you follow the logic of the method you can see that the "result"
variable will always be non-null. But the inspection can't see this
because it doesn't understand the link between the assertion that the
"items" list is not empty and that iterating over that "items" list will
assign a non-null value to the "result" variable.

Would it be possible to improve the inspection to understand this?

Ciao,
Gordon

Actually, the inspection is correct.

If "stuff" (which I assume is defined elsewhere) is an empty Collection,
result would be null. If "stuff" is replaced by "items" (as you
probably intended), then it cannot be null so long as assertions are
being enforced. However, the problem is not with understanding
collections, it is with trusting assertions. In non-debug code
assertions are turned off and then result could indeed be null.
I wouldn't change the inspection, I would change your logic.

Tim