NullPointerException waiting to happen

Wouter Zelle has written another fine new rule for PMD; this one is a NullPointerException finder.  Consider the following code snippet (from the JBoss source code tree, SecurityFlushSessionListener.java):

if(principals != null || !principals.isEmpty()) {
  // do stuff
}

Since it's a "conditional or" expression, each expression will be evaluated until one of them is true.  In the first expression, we check principals to see if it refers to an object.  If it doesn't - e.g., if principals is null - the expression evaluates to false.   Dandy, so the expression evaluates to false; we'll move on to the next expression.  But the next expression involves a deference (specifically, a method invocation) of the principals object reference that we just verified as null!  NPE, coming right up.

I think this shows how tricky even a rather small expression gets once we involve some boolean logic and negation.  It's all the more reason to use a static code analysis tool to hunt down this sort of thing for us.

And for more along these lines, get the PMD book!

Comments

[George]

Wonderful! I've seen FindBugs catch such clever expressions. It's great to see this in PMD.

[Pål]

That's just poorly written! Anyone with just a little understanding of boolean algebra and programming would see it immediately!

[tomcopeland]

Pål - Of course! But how many times do folks do a line by line review of their x00KLOC project to find things like this? That's a job for a utility to do...

[Pål]

Yes, it sure is hard to detect when reading code, but it shouldn't appear in the first place. Nice catch, though!