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.

Ramsdell

At first place, you should think if returning null is good practice. :) – Lynda

Consider using an Optional style construct to indicate intent; there's one in Guava. – Noninterference

@Noninterference There is one in JDK 8 as well – Lorie

@Gimby even when I add javadocs, it doesn't force people to read it. besides, I may make mistakes too by forgetting to add JavaDocs, and I may even use SDKs that doesn't tell about the possibility of getting null values. I tried to make it clear that what I've written is an example, but think of all possible options of getting such a thing - it can happen, no matter how good you are as a programmer. – Ramsdell

@kocko please read my comment to Gimby. What I've written is an example. You can't be in charge of every possible code out there. not all SDKs are even open source. please try to think outside of the example I've written. – Ramsdell

@Noninterference you don't always have the chance of writing the code that you are using. sometimes you use SDKs or work in a huge team. – Ramsdell

@Lorie JDK 8 has it? cool. Sadly, Android development need to be used with JDK 6... Will using Eclipse when both JDK8 and JDK6 are installed help in some way? wait, now that I think about it, shouldn't it be related more about JDT ? – Ramsdell

FWIW Netbeans does the same - no warning with a method but warning when it is inlined. – Coffle

@Coffle I don't use Netbeans. but if it doesn't help, why mention it ? :) – Ramsdell

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.

Grappling

Can you please show an example? also, how can I handle it in case I work in a huge project, with a lot of people (where one of them might have forgotten to add it), or when I deal with SDKs that don't tell about the possibility of getting null? – Ramsdell

Please read more here help.eclipse.org/juno/… But as I said, it will only partially solve your problem. In case you mentioned it would be hard to use it widely. You can also enable more aggressive null checking in your favorite IDE to at least see some warnings. – Grappling

Are you sure it can be applied to returned values of functions ? I can't find it in the link you've provided. just like other links I've read, there are only annotations about the opposite, and (I think) only for parameters of functions. besides, even if there was an annotation, it doesn't solve the problem since you might be using code that you didn't personally written. – Ramsdell

Sure, just look carefully at paragraph 'Inter-prodedural null analysis' and the second example on method getString --- it declares, that it will never return null (on the other hand you can use @Nullable to tell it actually can return a null). And yes, it won't solve ALL problems you've mentioned, hence the word "partially". 8) – Grappling

Can you please edit your answer and show this? It's not quite the perfect answer to this problem, but it can help on some cases, and can be useful for some people... – Ramsdell

Your example doesn't show that it's possible to show a warning in case of null value being returned, as it can't even return null in the "getString" function... you should let it return null on some cases and prove that eclipse can show a warning about the returned value being used, just as i've shown. you didn't even use the "@Nullable" annotation you talked about... – Ramsdell

Check it now. Please read about @Nullable and @NonNull over the Internet, there's plenty related material, for instance under the link I gave you (different cases and many tips on how to use it properly). – Grappling

ummm... even though I've updated to JDK 1.8 (and JDT for it) and it compiles fine (and it's ok with the @Nullable annotation), Eclipse doesn't show any warning about any of what you've written. – Ramsdell

You must enable it first: Window/Preferences/Java/Compiler/"Errors/Warning"/Null analysis/Enable annotation-based null analysis. You can also set what levels (error, warning, ignore) assign to specific problems. BTW, it works with earlier JDK-s too. – Grappling

still it doesn't work. i've tried to play with the settings of it. maybe you could take a screenshot of your eclipse settings regarding this? – Ramsdell

Please also note, that you need to add org.eclipse.jdt.annotation.jar to your build path, but quick-fix on "missing import" does it for you. – Grappling

I don't understand. if it's a part of Java, and the code compiles just fine and knows this annotation, what's missing? why should I add anything more than this? and how? also, btw, the settings is the same as on your screenshot... – Ramsdell

What do you mean by "it's part of Java"? Annotation mechanism for null analysis used in Eclipse allows you to use almost ANY (even your own) annotation to mark nullable and not-nullable fields, arguments, variables or methods. Please read more about using those annotations under the provided link, it seems pointless to copy-paste description here. – Grappling

So it's a part of JDT? If it succeeds compiling, how come it doesn't show any warning like yours? what am I missing? – Ramsdell

Annotations themselves don't do anything. They are just attached to the source and that's it. What is important, it's what uses it. In this case Eclipse uses these annotations to instruct compiler to show warnings or errors in case it discovers some problem. So you must enable annotated null analysis and I believe you must also enable Build Automatically on your project. – Grappling

Sadly, all the things you've written are already enabled. I will try to play with the settings a little more, but I'm pretty sure I'm missing something else here... :( – Ramsdell

I'm sorry to hear that... Uhm, have you added org.eclipse.jdt.annotation-*.jar to your build path (I know, you said everything compiles, but...). – Grappling

again, I don't know how to do it. also, the import is on "import com.sun.istack.internal.Nullable;" . maybe that's why it doesn't work. what is this Nullable? I've now tried searching "org.eclipse.jdt.annotation*" on the path of Eclipse on its "plugins" subfolder, and couldn't find anything. I've even updated to the beta version of Eclipse-Luna. – Ramsdell

Oh, that's why it doesn't work! It should be import org.eclipse.jdt.annotation.NonNull; and import org.eclipse.jdt.annotation.Nullable; --- different classes! 8) Please do that: remove your imports (com.sun.istack.*) completely. Then type @NonNull or @Nullable at some meaningful place. If you see error message about missing imports, take a look at quick fix and look for something like "include eclipse annotation jar into build path" --- click it. It should work since then. – Grappling

it doesn't compile when I use this import "import org.eclipse.jdt.annotation.Nullable;" . it doesn't even exist in the repositories Eclipse knows of, and I couldn't find the jar file you were talking about... there is no fix that looks like what you've written. – Ramsdell

What Eclipse version do you use? – Grappling

I've installed Eclipse Luna, but before this, I had Eclipse Keplar . – Ramsdell

I use Kepler --- see updated answer for further explanation. Damn, I really hope this will help. – Grappling

ohh.... now I see it. cool ! thanks. could be useful. not quite the answer to the question, but still useful. you will get an upvote for this. – Ramsdell

I wonder though why Eclipse always automatically used the wrong import, instead of telling me that there are other possible imports. – Ramsdell

It's because org.eclispe.jdt.annotation is not on your build path. In fact it's opposite --- Eclipse suggests adding it and it does do that for you. 8) Further, you can use any annotations you like (even your own) to mark nullable and non-nullable expressions/fields --- it's a matter of configuration under annotation null-analysis. – Grappling

if it's not on the build path, yet it is available, eclipse should have offered me to use it too. in fact, it worked even without adding anything to the build path... – Ramsdell

Well, it does offer you to add it to build path, only not from import suggestion dialog --- why should it? If one wouldn't like to use it or have their own annotations then why to show it? And if you check your build path settings and see Libraries tab, you will see an extra JAR there. Anyway it's good it finally worked. 8) – Grappling

no, it didn't offer me anything. it also didn't need anything being done on the build path. it just allowed me to do the importing in the weird popup you've shown. that's why it's weird - it knows about what can be imported, but didn't offer me that using the normal importing way. usually, when there are 2 classes that have the same name, it asks which file i want to use (for example when you press CTRL+SHIFT+O). – Ramsdell

This weird popup is so called Quick Fix. It does not import Java packages into your source, it just adds specified JAR to your build path, so it's something completely different from Java import suggestions. And Java imports can suggest only these entries, that are in your build path. By accident there is a annotation from com.sun.* that have the same name (Nullable I guess, or was it NonNull or NotNull?) and as it is on your build path, as this is part of JRE, then it simply shows in import suggestions. It's nothing weird --- actually it works exactly as expected. – Grappling

wait, so eclipse knows about suggestions of jars too? where does it take the jars from? – Ramsdell

It apparently knows only of these involved in very particular cases --- like this whole non-null analysis case. And, as you have probably already noticed, it "knows" only of jars being visible to Eclipse (those in plugin folder). If you need some tool to scan your whole drive for a given class in all JAR-s it can find, then probably you need something else --- or write your own tool to do that. With Java support of ZipInputStream it is a very easy task. – Grappling

Haven't made any eclipse plugin yet. is it that easy? – Ramsdell

Depends on the plug-in complexity. 8) But there are many tutorials on the web and as this plug-in would probably display some directory picker and get input from user as to the name of file in the JAR you need to find, then probably you will be able to do that in one day, and I mean including reading tutorials. – Grappling

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?

Uniparous

While this may be a good thing to do on some cases, it's not practical to do it on all functions of any project (and of SDKs you may use)... – Ramsdell

If you do it for all public accessors, you should achieve a very high level of coverage. Your problem is why people use unit tests - static code analysis may trap simple errors, but won't ever detect if your application doesn't behave as expected. Unit testing does. – Uniparous

again, unit tests are great, but it's not practical to make one for each function you, your colleagues and third party SDK developers make. it's just too much. You might as well just create a tool that warns you of such cases by yourself... – Ramsdell

I can understand why you may not want to put the effort in, but there's a strong argument for unit testing. SCA isn't a "magic bullet" - it'll never cover everything by itself. – Uniparous

Unit tests are good for many cases, but nobody will make one for each function. this is an overkill for development time. plus there might be reasonable cases that null can be returned, but whoever uses it wasn't just aware of it. in this case unit tests cannot help since it doesn't check who used those functions, it will just tell you that a function returns null or not. not only that, but most unit tests cannot tell you that a function can return null without digging into the code - you need to give the function all possible inputs, and that can take infinite time to run... – Ramsdell

running code of unit test in order to check if a function can return null isn't possible, not only because you need to check all possible inputs, but also because you don't know how long it will take. it's just like the loopy problem - you can't detect whether a code will stay forever or will return a result eventually. – Ramsdell

Then you just abstract to the method that calls foo(), to see if that provides the expected behaviour (or the method that calls that etc.). A few good unit tests can cover a lot of code indirectly. – Uniparous

No, Unit tests cannot help you at all, since you can't know which inputs to put, and how long they will take. read this for more information: en.wikipedia.org/wiki/Halting_problem (yes the "loopy" problem that i've written is called "halting problem", my mistake). – Ramsdell

Again , unit tests have their benefits, but they can't help you at all in this case. the reason is that it's not practical: you need to set it to work on all functions of all projects, you need to give them all possible inputs, and most importantly, you won't know how long to wait till the tests end (could even be infinite time). – Ramsdell

You don't generally test "all possible inputs"... A test that covers common scenarios (including max positive and negatives, plus 0) would have found your issue. – Uniparous

The "null" problem is more fundamental and cannot be reliably avoided with unit tests. That being said, it is very often the only possibility, so one can at least gain some confidence. – Dryfoos

"Unit tests cannot help you at all" Hmmm, my simple test found the issue in your example. Unit tests can help, fact. They aren't perfect, but no solution is. I can see this isn't the answer you want, but that doesn't necessarily make it wrong either... – Uniparous

@Mikaveli I agree with you regarding the importance of unit testing. Guess I'll assume you called foo() in a non-static way here because you reflexively avoid static methods in your code, which is also a good practice. – Highmuckamuck

Again, this was just an example. you can't know what are the common scenarios as you are not in charge of writing all functions ever written. you can only do those checks on code you've written. not only that, but it won't even help you with the problem i'm presenting - getting warned when your code tried to access something that can have a null values from a function. – Ramsdell

@user2317875 Indeed. :) I missed that foo() was a static method in his example and yes, I do tend to avoid static methods reflexively. :D – Uniparous

@Mikaveli it doesn't matter. I could set it as non static and will still not get any warning about it. please try to think outside of this example. it's just an example. also, as i've written, unit tests cannot help you about what i've presented. you can't possibly read all code of all functions and know what to write in unit tests, especially if you handle closed source SDKs . Unit tests are good on other cases, not here. – Ramsdell

@androiddeveloper I am - I don't believe you are. This is just one example, you've demonstrated that SCA can't catch every error (even if you solve this null issue). Even with unit testing, you still couldn't guarantee all is well with 100% certainty so you'd still have system and acceptance testing etc. – Uniparous

@Mikaveli i don't understand what you are referring to by "I am - I don't believe you are" . the SCA can't catch all errors, but it usually does get common mistakes, and without any effort on the developer side. Unit tests are not an acceptable solution here, since it's not generalized, not solving the problem, and not practical. Unit tests are used on code you know, to help you validate that your code works on common cases even after you change the code. As far as I know, there isn't an auto generating unit-testing tool that can understand everyone's code and know what to check... – Ramsdell

@androiddeveloper If you don't want to take my advice, you don't have to. My answer was a suggestion to try to help you - if you have a better solution, feel free to use (and share) it. – Uniparous

@Mikaveli if there was a solution , it would be some tool that shows you a warning, as this is what i've asked about. if you have 1000 functions (from all types of sources), you wouldn't make 1000 unit tests. it's not practical, and it can't even help in this case, as I try to find a way to handle it as soon as you use a function that can return null. what you offer is detecting in which cases (and that's not even true, just like the "halting problem" shows) a specific function returns null. it's very different from what i asked for. sorry, but unit tests cannot be the solution here. – Ramsdell

@androiddeveloper See my previous response [sigh] – Uniparous

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.

Spectroheliograph

you've missed my point. if you call a function on the returned value, and it could be null on some cases, you should get a warning . it doesn't matter how you got this value from. Eclipse could check the code even in a basic way , and warn you in such cases. – Ramsdell