A recent article on the top five causes of poor software quality and top 5 non-technical mistakes inspired me to also provide a top five on software quality bugs.  Here is my top 5 list of bugs (with some simple examples) that I see time and time again looking at customer code:

1.    Null Pointer dereference

This is far and beyond the most common issue that I see time and time again.

void npd_gen_must() {
int *p = 0; // NULL is assigned.
*p = 1;  // pointer is dereferenced
}

Now this example is pretty basic and if you ever did something this obvious, maybe it was time to re-evaluate your development skills.  The idea is simple, you assign NULL somewhere then dereference it at some point later.  This is usually missed under a complicated control flow (many conditionals).  Or even more common is the fact that I see memory is allocated, but is never checked against NULL.  Now, some organizations don’t care about this but I would hope anyone doing embedded development is all over it.

2.    Null pointer dereference from function

This is really the same thing but with one very important difference.  This deals with issues from functions.

void xstrcpy(char *dst, char *src) {
if (!src) return;
 dst[0] = src[0];
}

char global;

char *xmalloc() {
  if (global) return &global;
  return 0;
}

void npd_func_might(int flag, char *arg) {
  char *p = &arg;
  if (flag) p = xmalloc(); // xmalloc() may return NULL
  if (arg) { p = arg; } // p may get a new value here
  xstrcpy(p, "Hello"); // p will be dereferenced in xstrcpy()
}

It is this inter-procedural (spanning multiple files/functions) context that is often overlooked.

3.    Memory leaks

I have yet to find a programmer in the C/C++ world who doesn’t know this intimately.  Sadly they happen, a lot.

void foobar(int i) {
  char* p = new char[10];
  if(i) {
    p = 0;
  }
  delete[] p;
}

Here we have dynamic memory stored in ‘p’ and allocated through the function ‘new[]‘ at line 3 and is ultimately lost at line 5.

4.    Array index out of bounds

Again, most people know what these are but there are so many variations of this that they are always inevitable.

int main() {
  char fixed_buf[10];
  sprintf(fixed_buf,"Very long format string\n");
  return 0;
}

The string is 24 characters so at line 4 the array index of ‘fixed_buf’ may be out of bounds.

5.    Uninitialized variables

int foo(int t) {
  int x;
  if (t > 16) {
    x = 1;
  } else if (t > 8) {
    x = 2;
  }
  return x + 1;
}

The value of variable ‘x’ can be used at line 8, when it might be uninitialized.  I always found these surprising that these come up as they are pretty basic.  But I tend to only see these in complex control flow paths.  So the developer might check for these under normal conditions but forgot on some path.  Especially for legacy code this might not bite you until you change something later on.

So that’s it.  These examples are pretty simple and certainly not reflective of the real world (or at least I hope not).  Later I will post the same idea for Java code.

Here’s a short blog review of Klocwork Solo by Jeanne Boyarsky. Klocwork Solo is a downloadable Eclipse plug-in for Java. Aside from a few installation hiccups, it’ s a good review with kudos for the range of checkers we provide in the default configuration.

Try it out yourself and let us know what you think.

As a developer of static analysis tool for mainstream statically-typed languages, like C++ and Java, I was wondering for quite a while about how well static analysis applies to dynamically-typed languages, like Ruby and Python. And recently, I have come across this interesting project on GitHub: Reek – Code smell detector for Ruby. Well, I suppose that’s just a fancy way to name a static analysis tool.

What can Reek detect? It does not do heavyweight data/control flow analysis, so the list is not very exciting:

• Code Duplication – AFAIU, it’s not very accurate, ’cause Reek only tracks duplicated method calls (sic!)
• Some design anti-pattens, like Feature Envy, Utility Function, Nested Iterators and Control Couple
• Some metrics-based stuff, like Large Class, Long Method and Long Parameter List
• (Arguably) bad coding practices, like Uncommunicative Name

Interestingly, despite the poor set of features, compared to modern C++/Java static analysis solutions, Reek gets positive feedback from Ruby community. Some take it to the extreme – integrate Reek into the build and handle code smells as failing tests, and that of course breaks the build every time a new code smell is detected. So, is Ruby community starving for real static-analysis tool?..

This question forced me to run a quick research on what’s the current state of static analysis tools for dynamic languages. I was able to find the following tools for Ruby and Python:

Python tools appear to be pretty conservative about program analysis and try to “compensate” the dynamic nature of the language.

• PyChecker and pylint – These two tools aim to provide the same kind of warnings/errors, a compiler for statically-typed language would report automatically. For example: too few/many arguments in a method call, unused variables, using return value of a method that does not actually return any value, etc. One of the checkers for pylint, in fact tries to “simulate” static type system by using type inference, and detect missing members and functions, and this, in turn, might involve some elements of data flow analysis. Beside that, there is a bunch of metrics-based rules that can be checked automatically – you can put a limit on number of methods in a class, number of variables in a function, size/complexity of a function etc.

Tools for Ruby introduce the same kind of checks, but from slightly different angle. Rather than adopting the lint metaphor (a complementary tool, finding bugs/errors that compiler does not detect), the tools are positioned to find design flaws.

• Reek – See above.
• Flay – Checks code bases for duplicates. Aims to enforce the DRY design principle.
• Flog – A tool to spot the most complex functions in your code. Metric-based.
• Roodi – Supports a bunch of simple syntax-based and metric-bases checks. For example: assignment in condition, case missing else, max method/class/module line count, method/class/module name check.

Having looked through these projects, I’ve come to find two things:

First – There are some tools that do static analysis for Ruby/Python, but they are quite simple and don’t do (or don’t try to do?) any advanced heavyweight analysis.

Second – What Ruby/Python developers want to be automatically found in their code, is quite different from what their C/C++ fellows expect from a static analysis tool. And that is because the languages and programming cultures are quite different. For example, in Ruby there is no such thing as:

• null pointer dereference – nil is a first class object
• array bounds violation – array would return nil when index is out of bounds
• uninitialized variables – all variables are automatically initialized with nil
• memory leaks – garbage collection takes care of that

But they do have errors in their programs, don’t they? Yes, but Ruby/Python developers rely on tests pretty heavily. In fact, some claim, that tests are the only right way to deal with bugs in your program. This way a tool for automatic error detection might even be considered harmful – just because it can be used as an excuse for not writing tests.

So, what kind of job is left for a static analysis tool? Well, detect design flaws, a.k.a. “code smells”. In other words – automatically find subjects for refactoring (a change that does not affect program functionality). This way static analysis tool fits naturally into Red/Green/Refactor cycle.

Another possible area where static analysis based error detection can prove useful is security vulnerabilities – it is pretty hard to spot all corner cases up-front (or through exploratory testing), just because security is a complex domain and requires good amount of knowledge and expertise.

When I first started at Klocwork, I didn’t really know a lot about source code analysis. I understood the basic concept of how it finds bugs in software, but that is was essentially it. Sure I knew about Memory leaks, but I truly believed that they were only found a day or two before the GA date…at least, that was when our testing team always found them.

In one of my teams prior to joining Klocwork, we used Scrum. We were hard core, with daily 15 minute scrums, retrospective meetings, sprint planning sessions, defining “done”, secret handshakes, the whole 9 yards. We also broke our features down into small tasks, and those tasks were written on cue cards and then stuck to a big wall for all to see. What a great way to see the progress of a sprint. We had green cards for development tasks, blue cards for testing, yellow cards for documentation, and red cards for bugs. I remember how after 2 or 3 days into a sprint, the red cards would start showing up, and developers would then start addressing them. Since one of our team ‘rules’ was each person could only have a single task checked out at one time, our developers had to check-in the green card they were working on in order to tackle a red card. By the end of a sprint there were always a number of red cards left, which by definition, needed to be addressed first in the next sprint. I’m sure you can imagine the enthusiasm of heading into the next sprint knowing there was a wall of red cards to address first.

Anyways, my first few weeks at Klocwork consisted of talking with a lot of people; customers, prospects, etc. These people knew source code analysis, but they only knew the traditional way of source code analysis (SCA), and not the new generation of SCA where developers check their code before they check-in their code. I remember thinking I must be missing something…why is this so hard for these people to understand?  Source code analysis turns a lot of those red cards into green cards.  For more info on how SCA and agile can work together, check out this webinar I recently did…

Don’t get me wrong, I’m a big fan of open source, but why does everything have to be black and white? If it’s closed it must be evil and by association probably not written well, whereas if it’s open, it’s awesome and godly in its unnatural power to cure world hunger?

I’m referring, in this particular instance, to the righteous indignation that surfaced as a result of the castigation served up for the manufacturers of that ever-popular device, the breathalyzer. And yes, I’ve been stood at the side of the road looking stupidly at the officer whilst trying to remember just why I thought that 15^th shot of Jaeger was such a good idea, but I digress…

The manufacturer of this particular device, the Draeger Alcotest 7110 MKIII-C, had claimed vociferously that their device worked correctly, that their code was a part of their device, therefore proprietary, and not available to opposing council for analysis. Unfortunately for them, the courts disagreed and ordered the code handed over for analysis by Base One Technologies (who appear to be nothing more or less than your typical minority owned GSA hand-out specialists – your taxes at work, people…).

And what did they find? That far from being the highly skilled work of a bunch of Ph.D.’s that might warrant being labeled proprietary and top secret, it was instead a bunch of off-the-shelf engineering that had obviously been through many different iterations of development, through several different iterations of design, and wasn’t, bottom line, particularly smart. Nor was it particularly accurate, of course, which was the real hummer.

But come on, how many of us work on code (proprietary or open) that we can claim hand-on-heart hasn’t strayed from initial design goals?

Lest I now be pilloried for standing up for sub-par, closed (evil! evil!) source code, let me quickly segue onto the meme that is most aggravating me in relation to this story. And let me also quickly say for the record that anybody producing a breathalyzer that isn’t accurate needs stoning and feeding to the wolves, that much goes without saying. Back to the topic at hand…

So, Base One found a whole host of noxious practices and poorly executed designs in this particular code base. Not least, of course, being the afore-mentioned inaccuracies. But it did so in a very dry, engineering-centric sort of way that obviously wasn’t intended to pander to the pitchfork waving bigots, and so the ever helpful popular press took it upon themselves to take the one big number (ooo, shiny!) from the report, take it out of context (but of course), and then to label all closed source as bad by association.

• 19,400 potential errors in the code!!!

That’s obviously easier to get your editor interested in than a bunch of boring technical detail, like what was actually wrong with the device. 19. Thousand. Errors. Come on people, that’s a big number, amirite? Three out of every five lines of code contains a potential error. Sky. Falling. Must. Grab. Pitchfork!

But let’s read the small print here (or actually, not small at all, in fact it was right in the original report, but again wasn’t exciting enough to repeat): that number comes from an analysis performed using lint. You know, the tool that emits 400 errors for every 200 characters of input? You know you miss the days of 2,400 baud terminals that actually couldn’t keep up with the rate of error emission from this thing and just turned the whole screen into a weird whoosh of green CRT rays, don’t you?

Oh, but if only it was open source, goes the meme, the world (or at least that part of it which finds itself staring stupidly at the officer by the side of the road) would be a much better place. At least, that’s what we’re encouraged to believe.

Anybody tried lint on an open source project of any renown recently? I have (I won’t name them, not because it’d be embarrassing, but because it’s kind of irrelevant). Frankly it’s almost impossible to find a project that doesn’t emit thousands of lint warnings. Let’s face it, if you can write code that doesn’t emit lint warnings, you’re spending time in that happy place I like to call Hello World.

Come on people, wake up. There are very good reasons to hate bad software, whether it’s closed or open. Don’t be a schmuck and jump on the religious bandwagon just because it’s there. Think for yourself. There’s very good reasons why this device was castigated as a piece of junk, and they had nothing whatsoever to do with that big shiny number. If you’re going to report on something technical, do your readers the favor of at least trying to understand what you’re talking about before you go balls out into meme land.

In response to Jason’s post about false positives.  First of all there is a general misconception of false positives.  Modern static source code analysis tools have changed the game.  It is not the Lint tool of the past, a focus with deep inter-procedural technology has placed the requirement that static tools today produce more real issues than false reports.

With that said, Jason is right, large code bases never running static analysis will produce a large number of issues no matter how accurate it is.  Even though static analysis tools do provide a number of ways to manage this (and Jason talks about one) it does make sense to put this in your code reviews. You are looking at legacy code but if you are doing code reviews then you must have changed something with that legacy code.  Therefore having those bugs visible to you during the code review could suddenly now apply.

Jason certainly hits the nail on the head.  Automation, specifically using static analysis, is key and it should be tightly integrated with your code review. Although we need to be careful where we label source code analysis.  Static source code analysis certainly can find those low level issues such as labeling your local variables correctly, but it goes beyond simple code style issues.

Where static source code analysis can really help is with the deep inter-procedural context that it can provide.  For example, during a code review you go through some code with a number of function calls.  Hopefully you know what each and every function is doing…but do you really?  This is where the deep analysis of static source code tools can help.  It can help you identify that there may be an issue in the code review and that issue happens to show that a function is returning NULL.  Uh oh, potential null pointer dereference on our hands.

Now add code reviews with other static source code technology, such as full source cross reference information, flowcharting, impact analysis for any function/methods and architectural representation to show you the full context of the system.  Now you’re talking powerful.

We’ve posted previously on some of the differences between Findbugs’ open source Java analysis and commercial Java static analysis tools, specifically on the JSR-305 specification and source code versus byte code analysis topics. Due to these differences, many Java shops will use a commercial Java static analysis tool in conjunction with Findbugs to make sure they’re getting as complete issue detection as possible.

One area that’s been discussed previously is the ability to identify situations of possible null pointer dereference. This peaked my interest and led me to do some benchmarking against a few open source projects to assess the Findbugs analysis on intra- and inter-procedural possible null pointer dereference issues.

The normal assumption is that Findbugs is strong with intra-procedural analysis but unable to provide inter-procedural Java analysis. Here is an inter-procedural example from an open source project called hsqldb:

    protected void directRefreshTree() {

         int[]                  rowCounts;

        ...

            try {

                rowCounts = getRowCounts(tables, schemas);

            } catch (Exception e) {

                 //  Added: (weconsultants)@users

               CommonSwing.errorMessage(e);

            }

            ResultSet col;

            // For each table, build a tree node with interesting info

            for (int i = 0; i < tables.size(); i++) {

                col = null;

                String name;

                try {

                    name   = (String) tables.elementAt(i);

                    if (isOracle && name.startsWith("BIN$")) {

                        continue;

                        // Oracle Recyle Bin tables.

                        // Contains metacharacters which screw up metadata

                        // queries below.

                    }

                    schema = (String) schemas.elementAt(i);

                    String schemaname = "";

                    if (schema != null && showSchemas) {

                        schemaname = schema + '.';

                    }

                    String rowcount = displayRowCounts

                                      ? (" " + DECFMT.format(rowCounts[i]))

                                      : "";

    ...

    }

In the first try block getRowCounts() can have a null value.  How you ask?

    protected int[] getRowCounts(Vector inTable,

                                 Vector inSchema) throws Exception {

        if (!displayRowCounts) {

            return (null);

        }

      ...

    }

Okay, so automatically finding these issues is not going to happen with Findbugs.  I understand that, and that is why there are commercial tools to help with that.

But what really surprised me are the intra-procedural examples that are missed.  Again using examples from hsqldb and a specific class called TransferDb, Findbugs finds one issue. But there are clearly more intra-procedural issues that you would think Findbugs would have found.  Let’s take a look:

    TransferResultSet getData(String statement)

    throws DataAccessPointException {

        ResultSet rsData = null;

        try {

            if (srcStatement != null) {

                srcStatement.close();

            }

            srcStatement = conn.createStatement();

            rsData       = srcStatement.executeQuery(statement);

        } catch (SQLException e) {

            try {

                srcStatement.close();

            } catch (Exception e1) {}

            srcStatement = null;

            rsData       = null;

            throw new DataAccessPointException(e.getMessage());

        }

        return new TransferResultSet(rsData);

    }

This is a conditional situation where “srcStatement” could have a null value (the conditional is false), then you throw an exception with “createStatement()” and dereference later with “srcStatement.close()”.  Why does Findbugs miss this issue?  There are several issues of this type.

Or another situation where you throw an exception with “createStatement()” again.  This example spans multiple lines so you will find it in an attachment.  Here “select_rs” has a value of null and is clearly dereferenced much later on.

…

       ResultSet         col            = null;

        int               colnum         = 1;

  Statement         stmt           = null;

Source: null here ->  ResultSet         select_rs      = null;

        ResultSetMetaData select_rsmdata = null;

         try {

Exception throw here -> stmt           = conn.createStatement();

            select_rs      = stmt.executeQuery(TTable.Stmts.sSourceSelect);

            select_rsmdata = select_rs.getMetaData();

            col = meta.getColumns(TTable.Stmts.sDatabaseToConvert,

                                  TTable.Stmts.sSchema,

                                  TTable.Stmts.sSourceTable, null);

        } catch (SQLException eSchema) {

            // fredt - second try with null schema

            if (TTable.Stmts.sSchema.equals("")) {

                try {

                    col = meta.getColumns(TTable.Stmts.sDatabaseToConvert,

                                          null, TTable.Stmts.sSourceTable,

                                          null);

                } catch (SQLException eSchema1) {}

            }

        }

        try {

            while (col.next()) {

                String name = Dest.helper.formatIdentifier(col.getString(4));

                int    type        = col.getShort(5);

                String source      = col.getString(6);

                int    column_size = col.getInt(7);

                String DefaultVal  = col.getString(13);

                boolean rsmdata_NoNulls =

                    (select_rsmdata.isNullable(colnum)

                     == java.sql.DatabaseMetaData.columnNoNulls);

                boolean rsmdata_isAutoIncrement = false;

                try {

                    rsmdata_isAutoIncrement =

                        select_rsmdata.isAutoIncrement(colnum);

                } catch (SQLException e) {

                    rsmdata_isAutoIncrement = false;

                }

                int rsmdata_precision = select_rsmdata.getPrecision(colnum);

                int rsmdata_scale     = select_rsmdata.getScale(colnum);

                type = helper.convertFromType(type);

                type = Dest.helper.convertToType(type);

                Integer inttype  = new Integer(type);

                String  datatype = (String) TTable.hTypes.get(inttype);

                if (datatype == null) {

                    datatype = source;

                    tracer.trace("No mapping for type: " + name + " type: "

                                 + type + " source: " + source);

                }

                if (type == Types.NUMERIC) {

                    datatype += "(" + Integer.toString(rsmdata_precision);

                    if (rsmdata_scale > 0) {

                        datatype += "," + Integer.toString(rsmdata_scale);

                    }

                    datatype += ")";

                } else if (type == Types.CHAR) {

                    datatype += "(" + Integer.toString(column_size) + ")";

                } else if (rsmdata_isAutoIncrement) {

                    datatype = "SERIAL";

                }

                if (DefaultVal != null) {

                    if (type == Types.CHAR || type == Types.VARCHAR

                            || type == Types.LONGVARCHAR

                            || type == Types.BINARY || type == Types.DATE

                            || type == Types.TIME

                            || type == Types.TIMESTAMP) {

                        DefaultVal = "\'" + DefaultVal + "\'";

                    }

                    datatype += " DEFAULT " + DefaultVal;

                }

                if (rsmdata_NoNulls) {

                    datatype += " NOT NULL ";

                }

                v.addElement(inttype);

                datatype = helper.fixupColumnDefRead(TTable, select_rsmdata,

                                                     datatype, col, colnum);

                datatype = Dest.helper.fixupColumnDefWrite(TTable,

                        select_rsmdata, datatype, col, colnum);

                create += name + " " + datatype + ",";

                insert += "?,";

                colnum++;

            }

Sink: dereference here-> select_rs.close();

            stmt.close();

            col.close();

        } catch (SQLException e) {

            throw new DataAccessPointException(e.getMessage());

        }

So I’m a little confused as to why these issues are not highlighted.  It seems that this is the exact type of analysis Findbugs does.  Maybe it has to do with not recognizing the possible exceptions?  Does anyone have any ideas?

There’s been some recent discussion around using source code analysis (SCA) technology for build clean-up and optimization. I thought it might be useful to try and separate the spin from reality and outline where and how static source code analysis can be used for build optimization.

First, every SCA tool worth its salt does build analysis. Automated discovery of a customer’s build system is a required capability for deep static code analysis. Most users of SCA attempt to discover bugs, security vulnerabilities, and other maintainability problems. Some customers will also leverage the build analysis itself to conduct targeted clean-up of the build. Three common ways that this can be done are:

1. Trace file analysis – provides visibility of the entire build process to help find issues and inefficiencies in the build that can impact build times, and ultimately developer productivity.
2. Header file analysis – goal here is to identify inefficient and overly complex include structures that can lead to long build times and bloated system size.
3. Interface analysis – find low level issues that can cause build failures due to improper API usage.

First, trace file analysis is the process of analyzing, understanding and mapping every process executed during your build process, including all compiler and linker invocations. This kind of analysis is mandatory for a good SCA tool since it is necessary to understand all the full details of how you compile and link code so that detailed models of your system can be generated. The benefit from a build optimization standpoint is that this maps the entire build process and not just your compile and linking process, gives development leads and build managers visibility into the build, any inefficiencies, and where it may just be broken.

The other two types of analysis, header file and interface analysis, is focused on the source code directly but also important to build improvement. Specifically the focus here is on header files themselves and optimizations you can perform. A simple example of this is an include file that is simply never used. Why include it? This adds to the build time, size of the system and not to mention the complexity and maintainability of the system. Finding extra includes is not ground breaking technology but there are various types of issues to look for with header files. Other examples of more complex issues that involve deep analysis are with extra transitive issues or context dependent issues. For example, a missing include with a transitive dependency is a relationship between three or more files. In the following example, the first file, File1.c, includes the second file, header1.h, which, in turn, includes the third file, header2.h. File File1.c uses some symbols from file header2.h, but does not include it directly.

Good practice would have you include header2.h directly, of course developers include header1.h as a means to simply get the build to work. By eliminating instances where, in this example File1.c doesn’t even use anything in header1.h, real reductions in build time, and potentially system size, can be realized. Interface analysis is another type of issue focused on header files again looking for cyclical header files, duplicate header files and a whole slew of other in code issues that can be a nightmare such as multiple definitions or declarations.

Frankly there are limits to what can be done in this area with SCA. There are vendors in the build space such as Electric Cloud or IBM Buildforge who do this for a living and specialize in not only setting up production build environments that scale, but also have tools that complement the kind of optimization described above.

JSR-305 is a Java Specification Request intended to improve the effectiveness of static analysis tools operating in Java 5+ environments. The idea here is that one can use special purpose annotations in order to provide static analysis tools with hints regarding the behaviour and side effects of methods.

An example of such annotations can be found in the presentation ‘Annotations for Software Defect Detection’ by William Pugh, who is masterminding the whole spec. Here we go:

 1: void test() {
 2:    if (spec != null) fFragments.add(spec);
 3:    if (isComplete(spec)) fPreferences.add(spec);
 4: }
 6:
 5: boolean isComplete(AnnotationPreferences spec) {
 6:    return spec.getColorPreferenceKey() != null
 7:        && spec.getColorPreferenceValue() != null
 8:        && spec.getTextPreferenceKey() != null
 9:        && spec.getOverviewRulerPreferenceKey() != null;
10: }

What’s wrong with the snippet above? Well, the check for null on line 2 shows that the developer expects that the value of ‘spec’ can potentially be null, but it is still passed to method ‘isComplete’. Later, ‘isComplete’ attempts to dereference the value, which causes a NullPointerException.

According to Dr. Pugh, the best way to detect this issue statically is to force a developer to add the annotation @Nonnull to the method signature like this:

5: boolean isComplete(@Nonnull AnnotationPreferences spec) {

In this way, a basic static analysis tool that can minimally track ‘spec’ as a potential suspect for ‘null’ can issue a warning when the @Nonnull annotation is contradicted (that is, when ‘spec’ is passed to ‘isComplete’ as a parameter).

There are two problems with this approach:

• it forces the developer to do work that rightly should be performed by a static analysis engine
• it takes time to write the annotation for static analysis, but it takes even more effort to maintain the annotations and the actual code base in a consistent state.

In reality, the proposals behind JSR-305 exist to enable a tool intended for single function analysis (so-called intra-procedural analysis) to act as if it were performing whole program analysis by requiring the developer to state expected behaviour up front (whether or not that behaviour is actually expressed correctly in the developer’s code).

In contrast, this same scenario is supported by a whole program static analysis tool (so-called inter-procedural analysis) without developer intervention:

1. First, a complete call-graph of the system is built, and then all the methods are ordered so that called methods are processed prior to callers — such an ordering allows the tool to generate all the necessary information about, in this instance, the method ‘isComplete’ by the time the analysis of the method ‘test’ begins.
2. During the analysis of ‘isComplete’, the tool records the fact that the incoming argument ‘spec’ is dereferenced.
3. Next, the method ‘test’ is analyzed. In this method, the variable ‘spec’ is checked for null, so it is tracked as a potential suspect for an exception. Using the information generated about ‘isComplete’ the tool can reliably issue a warning on line 3, since it already knows that ‘isComplete’ dereferences the incoming argument.

So that example applies to a simple unconditional dereference scenario. In more complicated cases, Dr. Pugh proposes to use the annotation parameter ‘when’, with one of the following values:

• ALWAYS

• NEVER

• MAYBE

• UNKNOWN

For example: ‘@Nonnull(when=When.NEVER)’ means that a value is always null in the given context.

This specification seems to be a compromise between the amount of information provided by a developer to a static analysis tool and the amount of effort a developer has to put into it, a compromise that does not seem to be a particularly good solution here. First of all, the amount of information provided in such a manner is insufficient to provide accurate analysis, and secondly this seems to be too much work for the developer, especially when this work can be avoided.

Let’s examine how conditional value dereferencing is supported by whole program static analysis tools:

 1: void test() {
 2:     entity.qualifiedName = null;
 3:     saveName(entity, false);
 4: }
 5:
 6: boolean    saveName(Entity entity, boolean qualified) {
 7:    String name;
 8:    if (qualified)
 9:        name = entity.qualifiedName.trim();
10:    else
11:        name = entity.name.trim()
12:
13:     save(name);
14: }

In this example, an inter-procedural static analysis tool would first analyze the method ‘saveName’. A good analysis engine should be able to record the fact that this method only dereferences ‘entity.qualifiedName’ if the second parameter, ‘qualified’, is set to ‘true’. This, it would appear, is a deal more detailed than one can practically achieve by adding @Nonnull(when=When.XXX) annotations, even with all the work the annotation implies for the developer.

Next, the method ‘test’ would be analyzed. A good static analysis tool will naturally keep track of ‘entity.qualifiedName’ because of the assignment to ‘null’ on line 2 and its therefore potential use in an exception causing context. However, given that the actual call to ‘saveName’ on line 3 uses ‘false’ as its second argument, such a tool will not issue a warning that would in reality be a false positive, since the knowledge gained from analyzing ‘saveName’ disqualifies any potential warning due to the conditional relationship between arguments.

In summary, JSR-305 proposes a whole roster of interesting ideas for using annotations to enhance static analysis of Java code, and NPE detection seems to be only one aspect of this specification request. In upcoming blog posts, we shall continue the discussion of proposed annotations as well as offering our own ideas about how and when annotations should be used in static analysis.