False negative java:S2168: not reported when the outer null check is written `!(helper != null)`

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

Product/version: SonarQube Community Build 26.7.0.124771, sonar-scanner 8.1.0.6389

Language: Java

Rule: java:S2168

Issue:

S2168 reports the double-checked locking on the non-volatile field helper in seed/Dcl.java. In variant/Dcl.java the outer null check is written !(helper != null) instead of helper == null, everything else is the same, and nothing is reported. !(!(helper == null)) isn’t reported either.

DoubleCheckedLockingCheck.isIfFieldEqNull only treats an if as a field null check when the condition is EQUAL_TO with a null literal on one side (DoubleCheckedLockingCheck.java:88-103 at 8.34.0.44906, unchanged at 88.0.0.3367). It never unwraps !, so the outer if isn’t pushed on the field stack and the inner check finds no matching outer one.

PMD’s DoubleCheckedLocking had the same gap and now accepts negated null checks (pmd#6900, fixed in 7.27.0). Error Prone accepted the same report (error-prone#5963).

Reproducer:

seed/Dcl.java:

public class Dcl {
    private Helper helper;

    Helper getHelper() {
        if (helper == null) {                     // S2168 reported on the synchronized below
            synchronized (this) {
                if (helper == null) {
                    helper = new Helper();
                }
            }
        }
        return helper;
    }

    static class Helper {
        int x;
    }
}

variant/Dcl.java:

public class Dcl {
    private Helper helper;

    Helper getHelper() {
        if (!(helper != null)) {                  // the same outer check; nothing reported
            synchronized (this) {
                if (helper == null) {
                    helper = new Helper();
                }
            }
        }
        return helper;
    }

    static class Helper {
        int x;
    }
}

Expected behavior:

S2168 (“Remove this dangerous instance of double-checked locking.”) on the synchronized keyword in variant/Dcl.java, as in seed/Dcl.java.

How to reproduce:

# one project per file
sonar-scanner -Dsonar.projectKey=<key> -Dsonar.sources=. \
    -Dsonar.java.binaries=. -Dsonar.scm.disabled=true

Output of the commands above:

### SonarQube Community Build 26.9.0.129388 (sonar-java 8.41.0.47177), server reports 26.9.0.129388, default Sonar way profile  rule: S2168

--- SEED -- expected: reported (seed/)
    java:S1220             line -    Move this file to a named package.
    java:S2168             line 6    Remove this dangerous instance of double-checked locking.  <== S2168
    [S2168: 1]

--- VARIANT -- expected: reported; actual: not reported (variant/)
    java:S1220             line -    Move this file to a named package.
    java:S1940             line 5    Use the opposite operator ("==") instead.
    [S2168: 0]
3

Hi @MarkLee131!

Thank you for reporting this issue and providing a clear reproducer.

I have created a ticket to track it here: SONARJAVA-7282

Thanks again for your contribution!

David