false positive: condition always evaluates to true

[mfis...@doubleslash.de]

Sonar Java Version 3.13.1

Sonar reports on line 17:

Change this condition so that it does not always evaluate to "true"

Seems like a bug to me?!

Full example source for reproducing the bug: https://github.com/dsmf/sonarbug

import java.io.Serializable;
import java.util.Comparator;


public class PartComparator implements Comparator<Part>, Serializable {


   private static final long serialVersionUID = 1L;


   @Override
   public int compare(final Part p1, final Part p2) {


      // null checks
      if (p1 == null && p2 == null) {
         return 0;
      } else if (p1 != null && p2 == null) {
         return 1;
      } else if (p1 == null && p2 != null) { // --> Change this condition so that it does not always evaluate to "true"
         return -1;
      } else {


         // null checks
         if (p1.getRecord() == null && p2.getRecord() == null) {
            return 0;
         } else if (p1.getRecord() != null && p2.getRecord() == null) {
            return 1;
         } else if (p1.getRecord() == null && p2.getRecord() != null) {
            return -1;
         } else if (p1.getRecord() != null && p2.getRecord() != null) {


            // type checks
            if (isRecordFromSap(p1) && isRecordFromSupplier(p2)) {
               return 1;
            } else if (isRecordFromSupplier(p1) && isRecordFromSap(p2)) {
               return -1;
            } else if (areRecordsOfSameType(p1, p2)) {
               // comparing same type
               final MyRecord r1 = (MyRecord) p1.getRecord();
               final MyRecord r2 = (MyRecord) p2.getRecord();
               return r1.getSourcePos().compareTo(r2.getSourcePos());
            }
         }
      }


      return 0;
   }


   private boolean areRecordsOfSameType(final Part et1, final Part et2) {
      return (isRecordFromSap(et1) && isRecordFromSap(et2)) || (isRecordFromSupplier(et1) && isRecordFromSupplier(et2));
   }


   private boolean isRecordFromSupplier(final Part et2) {
      return et2.getRecord() instanceof RecordType1;
   }


   private boolean isRecordFromSap(final Part et1) {
      return et1.getRecord() instanceof RecordType2;
   }


}

[Nicolas Peru]

Hi, 

This is not a false positive,

where the issue is raised:  p2 != null cannot be evaluated to false : because if it was, that means that the previous if would have been taken. 

The condition evaluated to false here is the right operand of the && expression. 

Cheers, 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/d81ae434-06c3-41ef-993f-d002d6144bc8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

[mfis...@doubleslash.de]

Thanks for your answer, Nicolas.

So you mean that the second part of the third condition should be removed. 

I tried this locally and the warning disappeared. However I wonder why Sonar does not report the same issue on the null checks in the else branch. This seems inconsistent (could however be explained by the indirect access via getter method). 

Kind regards,

Matthias

[Nicolas Peru]

Hi, 

indeed, it is a getter so you know two distinct calls will return the same results but (for now, but cross procedural analysis is on the corner) the analyzer have no way to be sure of this and this is why you don't have an issue on the second construction. 

Cheers,

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/12312277-305a-4a1a-9641-98c143f96841%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.