This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 253210 - @Nullable final field: "Dereferencing possible null pointer" false positive
Summary: @Nullable final field: "Dereferencing possible null pointer" false positive
Status: NEW
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 8.0.2
Hardware: PC Linux
: P4 normal with 1 vote (vote)
Assignee: Svata Dedic
URL:
Keywords:
Depends on:
Blocks: 249320
  Show dependency tree
 
Reported: 2015-06-26 16:01 UTC by mintern
Modified: 2015-09-11 17:45 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mintern 2015-06-26 16:01:58 UTC
public static class Test {
    @Nullable final Integer i = null;
    public static void main(String[] args) {
        Test t = new Test();
        if (t.i != null) {
            t.i.toString(); // false positive here
        }
    }
}
Comment 1 Jiri Prox 2015-06-26 17:11:15 UTC
reproducible
Comment 2 Svata Dedic 2015-09-10 13:36:18 UTC
This may be tempting to fix, but it's rather a corner case; if the `t' reference ever escapes from the method and there's a method call in between t.i != null and t.i.toString(), it's not guaranteed that the t.i. value remains the same unless i is final (the invoked method may eventually execute code which has access to t and alters i)

I would say it's a rather fragile situation.
Comment 3 mintern 2015-09-10 15:33:32 UTC
> it's not guaranteed that the t.i. value remains the same unless i is final

That's why I titled the bug "@Nullable *final* field".
Comment 4 mintern 2015-09-10 18:42:22 UTC
It's probably a separate bug, but interestingly, internal references to `@Nullable` fields get no warnings at all:

public class AnotherTest {
    @Nullable Integer i = 42;
    String asString() {
        return i.toString(); // no warning here
    }
}
Comment 5 Svata Dedic 2015-09-11 15:16:15 UTC
(In reply to mintern from comment #3)
> That's why I titled the bug "@Nullable *final* field".

Apologies :)

Re. comment #4: check that you have enabled checking for fields for the null dereference hint.
Comment 6 mintern 2015-09-11 17:03:12 UTC
I'm sorry... I'm not seeing anything in Hints related specifically to null field checking. When I search "null", I see:

- Unnecessary Throwable.initCause
- .equals(null)
- Boxed value identity comparison
- Null Pointer Dereference

All are checked. When I search "field", there are many results, but none of them seem to be related to null.

I'm on NetBeans 8.0.2:

Product Version: NetBeans IDE 8.0.2 (Build 201408251540)
Updates: NetBeans IDE is updated to version NetBeans 8.0.2 Patch 2
Java: 1.8.0_60; Java HotSpot(TM) 64-Bit Server VM 25.60-b23
Runtime: Java(TM) SE Runtime Environment 1.8.0_60-b27
System: Linux version 3.13.0-63-generic running on amd64; UTF-8; en_US (nb)

Should I be on the 8.1 Beta?


Anyway, the reason I brought that up was in response to your point regarding `t.i != null` followed by `t.i.toString()` not being safe in the face of intermediate calls or concurrent operations. I was checking to see whether the following construct would warn:

public class AnotherTest {
    @Nullable private Integer i = 42;
    
    public String asString() {
        if (i != null) {
            clearI();
            return i.toString();
        }
        return null;
    }

    public void clearI() {
        i = null;
    }
}

But I didn't even see a warning for a trivial case, so it was hard to make any kind of point there. ^_^

I guess it comes down to a question of philosophy regarding Hints. Is it better for a Hint to match too often, making it more likely for the programmer to turn off the hint (or omit `@Nullable` field annotations, in this case), or is it better to miss a case that might be a programmer error?

My assumption has been that the latter is preferable. If I make a mistake in null-handling, I'm not blaming the IDE. But if my code is correct and I'm having to suppress a bunch of warnings, the hint isn't useful to me.
Comment 7 Svata Dedic 2015-09-11 17:34:12 UTC
If you select the Null Pointer dereference, a panel should display on the right, with a checkbox "also warn for fields". The option is by default off as field analysis is just partial yet.
Comment 8 mintern 2015-09-11 17:45:59 UTC
Thank you! After enabling that, I see the following:

public class AnotherTest {
    @Nullable Integer i = 42;

    public String asStringWarning() {
        return i.toString(); // warning here
    }

    public String asStringNoWarning() {
        if (i != null) {
            clearI();
            return i.toString(); // no warning here
        }
        return "null";
    }

    private void clearI() {
        i = null;
    }
}


This is the behavior I would expect. The Hint is still useful, and I don't really expect NetBeans to be sophisticated enough to catch degenerate cases like this example.

My preference would be for external references to a class's field to have the same behavior when giving null hints.