How to get warned about NPE caused by functions returning null?

Question

Background

Sometimes, functions return null on some cases, and yet whoever uses them wasn't aware of it, so NPE is inevitable .

For example (and it's just an example, to show what I'm talking about) :

public class Test
  {
  public static void main(final String[] args)
    {
    final int t=0;
    System.out.println("result:"+foo(t).toString()); // no warning here...
    }

  public static String foo(final int t)
    {
    if(t>0)
      return "positive";
    else return null;
    }
  }

The problem

Eclipse doesn't warn about such a thing, and it doesn't even have it on the settings.

It can detect such problems only when there are no functions involved (see my edit below).

Not only that, but I can't find an annotation that says that the returned value can be null. I think it has the opposite though.

What I've tried

So I thought that since Eclipse doesn't have some checks (and it's fine), I could use alternative static code analysis tools, like FindBugs and CodePro Analytix . Both failed to find such bugs.

I've tried to contact both projects, but didn't get any answer yet. for CodePro I'm not even sure if I did it right... In fact, I think this project has been discontinued (last update on 2010...).

The question

Is there any tool that can help avoiding such bugs?

---

EDIT: since many have thought that this is solvable by just changing the code, consider working in a huge team where some people might have forgotten about it (everyone can make mistakes). Or maybe you are even working on a project that uses an SDK (which could even be closed source) that can return null on some cases, but it's not documented because the creator of the SDK forgot about it.

Such a thing can happen, no matter how good you are as a programmer.

What I ask for is a way to overcome this by letting eclipse help. it should be able to warn me, at least on basic examples as i've written here.

Eclipse should be able to do such checks, just like it can warn me about this example:

int t=0;
--t;
String s="s";
if(t<0)
  s=null;
System.out.println(s.toString()); // here you will get a warning

This can be detected when you enable it on the settings of Eclipse :

Java-> Compiler-> Errors/Warning-> Potential null pointer access

I like Eclipse as it has plenty of cool warnings and errors that can help avoid bugs, but what i've written above isn't detectable by it and not even by the tools i've talked about.

Many bugs are hard to find, and nobody is a perfect programmer who doesn't make mistakes. Even if you do know a lot, you aren't protected from other people's bugs. People can even fail to find what's wrong with basic code (see here , here or just here to see how well you can find bugs yourself). That's why there are tools that can help you.

Answer 1

Consider using @Nullable and @NonNull annotations in your code. Partially it can solve your problem.

UPDATE: An example of use copied from http://help.eclipse.org/juno/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Ftasks%2Ftask-using_null_annotations.htm

@NonNull String getString(String maybeString) {
    if (maybeString != null)
        return maybeString;                         // the above null check is required
    else
        return "<n/a>";
}
void caller(String s) {
    System.out.println(getString(s).toUpperCase()); // no null check required
}

UPDATE: case with @Nullable

public @Nullable String getMessage() {
    return System.currentTimeMillis() % 2 != 0
        ? "That's odd..."
        : null;
}

public void showCase() {
    String msg = getMessage();
    if (msg.isEmpty()) { //- Here it yields about possible null access
        //...
    }
}

UPDATE: Settings for Null Analysis in my environment:

Actual error message from Eclipse:

UPDATE: You need to include JAR with Eclipse annotation in build path. Move your mouse pointer over @NonNull or @Nullable annotation that can not be compiled (as shown on the image) and choose Copy library with default null annotations to build path —

New JAR should appear in Package Explorer and compiler should be able to see it. Then move your cursor to the annotation and press Ctrl+Space — Eclipse should find correct import (note, that you might see two different packages on the list — choose the one from JDT) — and select it. It should work.

Answer 2

Write unit tests for your code:

@Test
public void testFooWithZero() {
    try{
        Test tester = new Test();
        assertEquals("Expect string returned", "positive", tester.foo(0));
    catch(NullPointerException npe) {
        fail("Threw NPE");
    }
}

Not only do they describe your expected behaviour (usually better than a lazily written Javadoc), it guarantees your code does what you expect.

Write tests for the positive and negative scenarios and you're golden.

Static code analysis certainly has its benefits, but it has its limitations - a common case would be how you'd protect against a get(Object) returning null from a HashMap or similar collection. SCA has no way of knowing what data is in the collection at runtime. You're not going to be able to annotate code from core Java packages or third-parties either.

Unit tests should be used in a complementary way and will improve the quality of your code. See this question for several benefits and reasons why you should be considering it:

Is Unit Testing worth the effort?

Answer 3

Methods have two ways of signalling this kind of "failure". Returning null or throwing an exception, which is usually indicated in the javadoc. Returning a null is not an error per se, since for example HashMap.get(key) will return null, if there isn't a value associated with a key. Having HashMap throw an exception would be a bad choice. On the other hand, some JPA methods will throw an exception if an entity isn't found (for example, requesting a unique entity, which is expected to exist).

Imagine the amount of warnings you would have if every time you used a HashMap, eclipse would say "be careful, it might return null".

So to sum it up: it wouldn't be very useful to warn that in real life, since you would start to ignore the warnings due to clutter.