Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#1102 RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE false positive

open-accepted
William Pugh
5
2014-01-18
2012-08-02
J. Stolze
No

FindBugs assumes that it has all code available during execution. This assumption can be dangerous, As shown in the following example.

The class GoodBehavingClass will always return a non null String object in the method isNullReturned which according to FindBugs should not be checked against null in the checkIfNullIsReturned method. When this class is part of a library which is distributed to third parties it can lead to extension of the GoodBehavingClass by BadBehavingClass which always returns null, which would lead to NullPointerExceptions when passed to this checkIfNullIsReturned method when this redundant null check is removed.

I think you can only state this redundant null check only when the method or class is declared as final.

Discussion

  • J. Stolze
    J. Stolze
    2012-08-02

    Class which illustrates the problem

     
    Attachments
  • William Pugh
    William Pugh
    2012-08-10

    What version of FindBugs are you using? I'm unable to replicate this.

     
  • William Pugh
    William Pugh
    2012-08-10

    Nevermind. Had to run it without including BadBehavingClass. Was able to reproduct.

     
  • William Pugh
    William Pugh
    2012-08-10

    • status: open --> open-accepted
     

  • Anonymous
    2012-09-26

    I think this hypothesis is not that bad, since in most cases FindBugs will probably be right. However I think one should be able to make it disappear by annotating the method with @CheckForNull or similar but it does not seem to work (in Eclipse FindBugs 2.0.1 at least)

     
  • J. Stolze
    J. Stolze
    2012-10-01

    When you apply the criteria I suggest, final method or final class, FB would be right in all cases. This is clearly better than assuming you're right. As the example shows.

     
  • Fred Toussi
    Fred Toussi
    2014-01-17

    With version 2.03, related false positives occur in contexts like this and vice versa with respect to assignment of null or not null Object to globalError in large classes where several calls separate the assignments and nullability checks:

    Object globalError;
    
    void myMethod() {
      // some code
    
      if (someCondition)
      globalError = someVal;
    
      // some code
    }
    
    void myProg() {
      globalError = null;
      myMethod();
    
      // FindBugs considers this check redundant
      if (globalError != null) {
        // do something
    
      }
    }
    
     
    Last edit: Fred Toussi 2014-01-18
  • William Pugh
    William Pugh
    2014-01-17

    On that code snippet, FindBugs 2.0.3 and 3.0.0-Snapshot produce no warnings.