#1151 NP_NONNULL_RETURN_VIOLATION incorrect

2.0.3
closed-rejected
William Pugh
5
2013-02-20
2012-12-29
Anonymous
No

The following method complains that it may return a null when it really cannot:

@Nonnull public static BigDecimal numberToBigDecimal(@Nonnull Number number) {
BigDecimal bigDecimal = null;

if (number instanceof BigDecimal)
bigDecimal = (BigDecimal)number;
else {
if (!(number instanceof BigInteger) && !(number instanceof Byte) && !(number instanceof Short) && !(number instanceof Integer) && !(number instanceof Long))
throw new ClassCastException(number.getClass().getName());

if (number.intValue() == 0)
bigDecimal = BigDecimal.ZERO;
else if (number.intValue() == 1)
bigDecimal = BigDecimal.ONE;
else if (number.intValue() == 10)
bigDecimal = BigDecimal.TEN;
else if (number instanceof BigInteger)
bigDecimal = new BigDecimal((BigInteger)number);
else if (number instanceof Byte)
bigDecimal = new BigDecimal(number.byteValue());
else if (number instanceof Short)
bigDecimal = new BigDecimal(number.shortValue());
else if (number instanceof Integer)
bigDecimal = new BigDecimal(number.intValue());
else if (number instanceof Long)
bigDecimal = new BigDecimal(number.longValue());
}

return bigDecimal;
}

Discussion


  • Anonymous
    2012-12-29

    • labels: 914653 --> false positive
     
  • William Pugh
    William Pugh
    2013-01-26

    • status: open --> pending-rejected
     
  • William Pugh
    William Pugh
    2013-01-26

    Sorry, but FindBugs isn't going to be able to understand the logic of the if statement for the throw new ClassCastException.

    If you want to ensure you don't return a null value, don't initialize bigDecimal to null and add a final else clause. In fact, I'd recommend getting rid of the bigDecimal variable all together. As soon as you know what value you want to return, return it.

    In other words, something like the following:

    public static @Nonnull BigDecimal numberToBigDecimal(@Nonnull Number number) {

    if (number instanceof BigDecimal)
    return (BigDecimal) number;
    else if (!(number instanceof BigInteger) && !(number instanceof Byte) && !(number instanceof Short)
    && !(number instanceof Integer) && !(number instanceof Long))
    throw new ClassCastException(number.getClass().getName());
    else if (number.intValue() == 0)
    return BigDecimal.ZERO;
    else if (number.intValue() == 1)
    return BigDecimal.ONE;
    else if (number.intValue() == 10)
    return BigDecimal.TEN;
    else if (number instanceof BigInteger)
    return new BigDecimal((BigInteger) number);
    else if (number instanceof Byte)
    return new BigDecimal(number.byteValue());
    else if (number instanceof Short)
    return new BigDecimal(number.shortValue());
    else if (number instanceof Integer)
    return new BigDecimal(number.intValue());
    else if (number instanceof Long)
    return new BigDecimal(number.longValue());
    else
    throw new AssertionError(number.getClass().getName());
    }

     
  • William Pugh
    William Pugh
    2013-02-20

    • status: pending-rejected --> closed-rejected
    • milestone: --> v1.0_(example)