Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[java] BrokenNullCheck FP with PMD 6.30.0 #3071

Closed
testation21 opened this issue Jan 18, 2021 · 3 comments · Fixed by #3191
Closed

[java] BrokenNullCheck FP with PMD 6.30.0 #3071

testation21 opened this issue Jan 18, 2021 · 3 comments · Fixed by #3191
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone
7.0.0

Comments

@testation21
Copy link
Contributor

testation21 commented Jan 18, 2021 •
edited by adangel
Loading

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 testation21 added the a:bug PMD crashes or fails to analyse a file. label Jan 18, 2021
@oowekyala oowekyala added a:false-positive PMD flags a piece of code that is not problematic and removed a:bug PMD crashes or fails to analyse a file. labels Jan 18, 2021
@testation21
Copy link
Contributor Author

testation21 commented Jan 29, 2021 •
edited by adangel
Loading

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.

oowekyala added a commit to oowekyala/pmd that referenced this issue Apr 3, 2021
@oowekyala
Test cases for pmd#3071
250d405
@oowekyala oowekyala linked a pull request Apr 3, 2021 that will close this issue
[java] Update rule BrokenNullCheck #3191
Merged
4 tasks
@oowekyala oowekyala added this to the 7.0.0 milestone Apr 3, 2021
@oowekyala oowekyala mentioned this issue Apr 3, 2021
Merged
4 tasks
@adangel
Copy link
Member

adangel commented Apr 10, 2021

@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 adangel mentioned this issue Jan 23, 2023
Closed
55 tasks
@adangel
Copy link
Member

adangel commented Apr 22, 2023

This has been fixed with PMD 7.0.0-rc1.

@adangel adangel closed this as completed Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[java] Update rule BrokenNullCheck oowekyala/pmd
3 participants
@adangel @oowekyala @testation21