Eclipse gives dead code warning for reachable code (variant)

Question

I have following code:

public String myMethod(String keyValue) {
    Map<String, Integer> keyValueToRowIndex = ...
    Integer rowIndex = (keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue);
    if (rowIndex == null)
      return null;
    ...
}

Eclipse gives a "dead code" warning on the return null;. Removing the test for keyValue == null also removes the warning but I don't see how that extra test makes the return statement dead code. Clearly if the map contains no entry for some non-null keyValue, then rowIndex can still be null. Or am I missing something here?

I've seen similar Eclipse issues (here for instance), but this one seems a different and more trivial one.

Frail

What is your Java version? there is a difference between Java 7 and 8. https://mcmap.net/q/545121/-java-autoboxing-and-ternary-operator-madness – Zinciferous

I had it on Java8, but the warning is there both with 7 and 8. – Frail

Also it throws NPE in both cases (see comment on accepted answer) – Frail

Answer 1

My guess is that line 3 is interpreted as

Integer rowIndex = Integer.valueOf((keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue).intValue());

(so both arguments to ?: are unified as int) - oddly enogh, Eclipse now shows no warning, even if it is now obvious that rowIndex is never null...

You might also replace the 0 with Integer.valueOf(0) to make the warning disappear.

Cormier

You're right, the issue is not in Eclipse but in my code. The ternary expression provokes an autobox to int, so if keyValueToRowIndex.get(keyValue) returns null, a NullPointerException is thrown on line 3. Thanks for pointing this out. Replacing the 0 with Integer.valueOf(0) solves the issue. – Frail

Answer 2

The (surprising) short answer: Eclipse is right! This is dead code!

Reason

The important part is the ternary expression in the following line of code:

    Integer rowIndex = (keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue);

The Java language specification (JLS) says about the "Conditional Operator ?", that, if the first expression is of type int, and the second expression is of type Integer, the type of the entire expression will be int.

In your case, the first expression is the constant literal value 0, which is an int. The second expression is the result of the get method, which returns an object of type Integer. So according to the JLS, the entire expression has primitive type int!

This means, if the second expression (the get-call) will be evaluated, the result will be unboxed from Integer to int. This int value will then be auto-boxed again into an Integer to be able to assign it to the left operand, rowIndex.

But what happens, if the map returns a null value? In this case, unboxing from Integer to int is not possible, and a NullPointerExpression will be thrown!

So eclipse is right, as your expression can never return null, rowIndex will never be null either and the then-block of your if-statement will never be executed and hence is dead code!

Solution

The solution is simple: Use an Integer object instead of an primitive int value for your first expression:

Integer rowIndex = (keyValue == null) ? Integer.valueOf(0) : keyValueToRowIndex.get(keyValue);

Univalence

thanks, yes this is the full answer. I had already deduced that from @Gyro Gearless' answer, and accepted that. +1 for the references. – Frail