S2259 possible false positive, function may not return null

Rules and Languages Report False-positive / False-negative...
,
1

Hello, I have noticed a sonar bug report, which I believe is a false-positive. Consider the following code:

public final class Test {

	private static final BigDecimal IDENTITY = BigDecimal.ZERO;

	private Test() {
	}
	 
	private static <T> T valueOrDefault(T value, T defaultValue) {
		return value == null ? defaultValue : value;
	}
	
	public static BigDecimal bigDecimalSum(Collection<BigDecimal> values) {
		BigDecimal sum = IDENTITY; // when null is assigned here, there is no failure
		for (BigDecimal v : values) {
			if (v != null) {
				sum = valueOrDefault(sum, IDENTITY).add(v); // S2259 occurs here
			}
		}
		return sum;
	}

}

Sonar reports a possible NullPointerException being thrown (I have marked the line with a //S2259 comment), however, I cannot see how this is possible. Whenever the second argument to valueOrDefault is not null, the function may never return null, and that’s the case here (IDENTITY static final field is assigned to a non-null value).

Interestingly, when I assign the sum variable on the first line of bigDecimalSum function to null, and not to IDENTITY, the sonar warning disappears.

I can reproduce this using Eclipse 20.09 and SonarLint 5.6.0.25634.

2

Hello Tomas,

Thanks for joining the community and for raising this issue.

It looks like a false positive indeed, and I reproduce on my side. Let me assign a collegue to have a closer look.

Thanks again,
Damien

4

Hello @Tomas_Rosenberg,

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, you code seems fine to me, this is a false positive. Ticket created: SONARJAVA-3691.

Interestingly, when I assign the sum variable on the first line of bigDecimalSum function to null , and not to IDENTITY , the sonar warning disappears

It is correct and expected to not report an issue. The engine will not have the constraint “sum == IDENTITY”, he will never assume that IDENTITY can actually be null.

Best,
Quentin