Why does code inspection on SharedPreferences.getString() with default value report "may produce NPE"?

Question

When running "Analyze/Inspect Code" in Android Studio 1.2.2 with the default settings on below code:

public class MainActivity extends Activity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        SharedPreferences sharedPref = PreferenceManager.getDefaultSharedPreferences(this);
        String value = sharedPref.getString("somekey", "default");
        if (value.equals("default")) {
            Log.d("MainActivity", "value matches default");
        }
    }
}

it yields the following warning under "Probable bugs":

Method invocation 'value.equals("default")' at line 16 may produce 'java.lang.NullPointerException'

However, if the preference doesn't exist, the default value will be returned. Even if the value of the preference is set explicitly to null with:

sharedPref.edit().putString("somekey", null).commit();

the default value will be returned (presumably under the hood this is actually removing the preference, but that's not important here).

The only way the above code can actually produce an NPE is by passing a null value as the default when the preference doesn't exist:

String value = sharedPref.getString("somekey", null);

However, if somebody did that, it would be highly unlikely he was missing the null check afterwards. I do realize that null may not be a literal here and may come from some other variable, but again, a rather unlikely use-case for getting preferences.

I do realize that "it's only a warning" and I "can ignore it" and it's my code, I can do "whatever I want" and "lint isn't a babysitter" and all the rest of it, but it bugs me (pardon the pun), that this obviously trivial non-bug shows up as a "Probable bug" during code inspection.

Now the questions:

1. Is there some lurking danger in my presumed bug-free code that I haven't seen yet?
2. Is the code inspection just not smart enough?
3. What actually triggers this warning during code inspection?
4. What can be done to remove the warning?

In an attempt to answer 4. I have come up with the following:

1. nothing, just ignore the output. Looks ugly, and looks like you didn't consider the warning at all.
2. disable the entire class of warning. May hide other, genuine bugs.
3. the classic, change the equals around to "default".equals(value). Doesn't work for switch(value){...}
4. just add a separate null check if (value != null) {...}. Redundant check, smells of surrender.

In an attempt to answer 3. I have experimented around a bit:

public class MainActivity extends Activity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        String value = null;
        if (value.equals("default")) {
            Log.d("MainActivity", "value matches default");
        }
    }
}

Results in the expected warning and blows up as expected. Fair enough.

Next...

public class MainActivity extends Activity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        String value = "test";
        if (value.equals("default")) {
            Log.d("MainActivity", "value matches default");
        }
    }
}

No warning, no crash. Obviously.

Next...

public class MainActivity extends Activity {
    private String getString() {
        String value = null;
        if (new Random().nextBoolean())
            value = "test";
        return value;
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        String value = getString();
        if (value.equals("default")) {
            Log.d("MainActivity", "value matches default");
        }
    }
}

Obviously crashes randomly (literally), but no warning! Ouch.

So, let's stick a @Nullable annotation on getString(). Now we get the warning, good. However, there appears to be no annotation on SharedPreferences getString().

Well, let's try the opposite. Stick a @NonNull on getString(). New warning:

Expression 'value' might evaluate to null but is returned by the method declared as @NotNull (at line 28)

Cool, so the code inspection knows that the result of getString() may by null, but why doesn't it give us the original warning when trying to use the return value?

Update

There is another implicit question here that I might not have emphasised enough the first time around.

The description of this particular code inspection reads:

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, e.g. report possible NullPointerException errors.

Which seems to imply that those annotations are what drive the warning.

And indeed my own method generates the warning only when annotated with @Nullable. However, SharedPreferences getString() generates this warning although it is not annotated. Why is this?

Answer 1

Is there some lurking danger in my presumed bug-free code that I haven't seen yet?

Probably not. It still makes sense to pay attention to analyzer results and strive to get rid of any issues by fixing or selectively suppressing them.

Is the code inspection just not smart enough? What actually triggers this warning during code inspection?

Yes it's not smart enough.

The analyzer can see a method can return null and the return value is used unchecked but cannot reason deeply enough that it won't be the case in your code.

What can be done to remove the warning?

Prefer

"default".equals(value)

style when possible as you've already discovered. This gives you NPE-free comparisons in all cases, and also prevents the analyzer from nagging.

When not possible, suppress the warning locally, for example:

//noinspection ConstantConditions    
switch (value) {
    ...