[java] BrokenNullCheck FP with PMD 6.30.0

[testation21]

Affects PMD Version: pmd 6.30.0

Rule: BrokenNullCheck
https://pmd.github.io/pmd-6.30.0/pmd_rules_java_errorprone.html#brokennullcheck

Description:
Hello, I think I found some False Positives in BrokenNullCheck and I want to report them.

There are some alarms on cases which don't call method on object. It means that they cannot throw NullPointException.
I think that I found three cases which are different from each other.
Case 1. Using instanceof
Case 2. Using Boolean Class
Case 3. Comparing between Local Variable and Field which have the same identifier and accessed by this.

Details about codes are below.
Thank you.

Code Sample demonstrating the issue:

class Test {
    Object obj = new Object();
    Object objectWithDifferentName;

    public void testPMD(){
        Object obj = new Object();
        String str = "str";
        Boolean boo = null;

        //Case 1. : An Alarm is generated
        if(obj == null && !(obj instanceof String));

        //Case 2. : Alarms are generated for each line.
        if(boo == null && boo != true) ;
        if(boo == null && boo != false);

        //Case 3.
        if(obj == null && this.obj == null); //An Alarm is generated
        if(obj == null && this.objectWithDifferentName); //An Alarm isn't generated
        if(obj == null && objectWithDifferentName); //An Alarm isn't generated
    }
 }

Expected outcome:
Expected that there wouldn't be any alarms.

Running PMD through: [CLI]

[testation21]

Hi,

I found some other cases, please, check these.

1. null checking two other variables,
2. null checking two other variables which are returned value of a method,
3. null checking with a variable and method calling with another variable,
4. null checking with an array element and method calling with another array's element.

Example codes is

class TestBrokenNullCheck{
    public void testPMD(){
        //Case 1.
        if ((foo == null) != (another == null) || (foo != null && !foo.equals(another)));

        //Case 2.
        if(list.remove(null) != null || list.remove("") != null);
        if(foo(something) != null || foo(something.methodCall) != null);

        //Case 3.
        if (foo != null || (foo == null && another != null && another.methodCall() == 0));

        //Case 4.
        if (null != arr[idx] || (null == arr[idx] && !anotherArr[idx].foo()));
    }
 }

which generate 6 alarms.

Two for case 1, one for each other lines.
Please, check these cases.

Thank you.

[adangel]

@testation21

I believe, the following case is valid and not a false positive:

 //Case 2. : Alarms are generated for each line.
        if(boo == null && boo != true) ;
        if(boo == null && boo != false);

Since "boo" is of type "java.lang.Boolean", if it is null, then the auto-unboxing will throw a NPE:

java.lang.NullPointerException: Cannot invoke "java.lang.Boolean.booleanValue()" because "boo" is null

So, this is really a mixup between "&&" and "||" - exactly the case the rule should detect.

[adangel]

This has been fixed with PMD 7.0.0-rc1.