Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

FP in S2259 when initial value comes from another non-null constant

Description

Reproducer:

public final class Test { private static final BigDecimal IDENTITY = BigDecimal.ZERO; // No issue when = new BigDecimal("123"); private static <T> T valueOrDefault(T value, T defaultValue) { return value == null ? defaultValue : value; } public static BigDecimal bigDecimalSum(Collection<BigDecimal> values) { BigDecimal sum = IDENTITY; for (BigDecimal v : values) { sum = valueOrDefault(sum, IDENTITY).add(v); // S2259 FP } return sum; } }

In this code, the issue raised is that if IDENTITY is null, this code will throw a NPE. The engine believes it is possible since sum is checked for null during the first iteration of the loop where sum is equal to IDENTITY, meaning that IDENTITY can also be null. It misses the fact that the first check value == null will always be false and that it is only useful from the second iteration (if `add` returns null).

In the end, we miss detecting that the initial value can never be null. Note that if IDENTITY is initialized with new BigDecimal("123");, the FP disappears.

Another example (in the same file):

public class NpeEx { static final String SONAR_TEST = "sonar_test"; } class S2259Demo { public void someMethod(boolean cond) { String testString = null; if (cond) { testString = NpeEx.SONAR_TEST; } if (testString == null) { // Do something } if (NpeEx.SONAR_TEST.equals(testString)) { // FP, constant can not be nullable // Do something } } }

In this second example, if we move the constant to the same class, the issue disappears.
Thrid similar example in this community thread.

Another example from discuss, slightly different from this ticket’s main example

Activity

Pinned fields
Click on the next to a field label to start pinning.

Details

Assignee

Reporter

Labels

Components

Priority

Sentry

Created February 12, 2021 at 2:15 PM
Updated June 20, 2024 at 10:02 AM