Dereference may produce 'java.lang.NullPointerException'

Question

Following snippet produces a lint warning in Android Studio.

    Bundle extras = getIntent().getExtras();
    if (extras != null && extras.getString(REDIRECT_KEY) != null) {
        switch (extras.getString(REDIRECT_KEY)) { ...

extras.getString(REDIRECT_KEY) produces the warning

Dereference of 'extras.getString(REDIRECT_KEY)' may produce 'java.lang.NullPointerException'

But I don't see any scenario where this could happen. Is it a bug in lint checks, that it simply does not recognize my null-checks in the if before? Or do I miss something?

EDIT: changing the code to following, did remove the warning

if(getIntent() != null && getIntent().getStringExtra(REDIRECT_KEY) != null){
        switch (getIntent().getStringExtra(REDIRECT_KEY)){
            ...
        }
    }

BUT this is only gone because of the way, this lint check works (at least I guess). If I show more infos to this check, it says at one point

Variables, method parameters and return values marked as @Nullable or @NotNull are treated as nullable (or not-null, respectively) and used during the analysis to check nullability contracts

Looking at the source code of Bundle and of Intent shows:

/**
 * Retrieve extended data from the intent.
 *
 * @param name The name of the desired item.
 *
 * @return the value of an item that previously added with putExtra()
 * or null if no String value was found.
 *
 * @see #putExtra(String, String)
 */
public String getStringExtra(String name) {
    return mExtras == null ? null : mExtras.getString(name);
}

And BaseBundle

/**
 * Returns the value associated with the given key, or null if
 * no mapping of the desired type exists for the given key or a null
 * value is explicitly associated with the key.
 *
 * @param key a String, or null
 * @return a String value, or null
 */
@Nullable
public String getString(@Nullable String key) {
    unparcel();
    final Object o = mMap.get(key);
    try {
        return (String) o;
    } catch (ClassCastException e) {
        typeWarning(key, o, "String", e);
        return null;
    }
}

As you can see, BaseBundle sets its return values to @Nullable, while Intent doesn't. So using getStringExtra only removes the symoptoms, not the cause. I still guess this is caused by an insufficient lint check, instead of wrong coding on my side. Or does anyone still see a scenario, where a Null pointer can be thrown?

I think instead of getting String everytime from extras, you should assign it to a variable. Because Your extras may become null on certain situations. — Pragnani, Feb 24 '16 at 11:17
third line in snippet, or line 80 in image. I am checking for extras == null and for the result of getString() == null. See the If in the second line. — AlbAtNf, Feb 24 '16 at 11:18
It seems you should be calling `getStringExtra(REDIRECT_KEY)` instead. — Neil, Feb 24 '16 at 11:19
The thing is, that this never produced a null pointer. There is only the warning that it COULD happen. If the values are really null during runtime, my code still works without errors. — AlbAtNf, Feb 24 '16 at 11:22
I think You should do something like: String redirectkey = extras.getString(REDIRECT_KEY); and then if(redirectKey!=null){} .... — Opiatefuchs, Feb 24 '16 at 11:31
@Neil thanks for not reading my edits and not understanding, why this is not a duplicate. Same for Jarrod Roberson... — AlbAtNf, Feb 25 '16 at 09:00

score 5 · Accepted Answer · answered Feb 24 '16 at 11:36

Take a look at this example taken from here

class Argument {

    public final static int TOMAYTO = 0;
    public final static int TOMAHTO = 1;

    static void argue() {

        int say = TOMAYTO;

        while (true) {

            switch (say) {

            case TOMAYTO:

                say = TOMAHTO;
                break;

            case TOMAHTO:

                say = TOMAYTO;
                break;
            }
        }
    }
}

The bytecodes generated by javac for the argue() method are shown below:

   0 iconst_0 // Push constant 0 (TOMAYTO)
   1 istore_0 // Pop into local var 0: int say = TOMAYTO;
   2 iload_0 // Push key for switch from local var 0
                          // Perform switch statement: switch (say) {...
                          // Low case value is 0, high case value is 1
                          // Default branch offset will goto 2
   3 tableswitch 0 to 1: default=2
            0: 24 // case 0 (TOMAYTO): goto 24
            1: 29 // case 1 (TOMAHTO): goto 29

                          // Note that the next instruction starts at address 24,
                          // which means that the tableswitch took up 21 bytes
  24 iconst_1 // Push constant 1 (TOMAHTO)
  25 istore_0 // Pop into local var 0: say = TOMAHTO
  26 goto 2 // Branch unconditionally to 2, top of while loop
  29 iconst_0 // Push constant 1 (TOMAYTO)
  30 istore_0 // Pop into local var 0: say = TOMAYTO
  31 goto 2 // Branch unconditionally to 2, top of while loop

As you can see for switch statement with String data type a tableswitch is done, and for each case the value passed to switch is compared with the case's value, so what this means is that in your case extras.getString can be called multiple times without your previous if checks being called, and as such since extras is a bundle there is a chance it might get dereferenced and cause a nullpointer exception.

It's always a good practice to create a local variable instead of doing multiple calls of the method, you can have a look at this presentation by Jake Wharton to understand why.

Thanks, as this is the only answer that really answers my original question "did I miss anything". — AlbAtNf, Feb 24 '16 at 11:39

Dereference may produce 'java.lang.NullPointerException'

1 Answers1