add check for Optional.equals(Optional.empty())

mebigfatguy · Oct 22, 2021 · db220ba · db220ba
1 parent d0e02e9
commit db220ba
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 11 deletions.
diff --git a/etc/bugrank.txt b/etc/bugrank.txt
@@ -174,6 +174,7 @@
 +0 BugPattern OC_OVERZEALOUS_CASTING
 +0 BugPattern ODN_ORPHANED_DOM_NODE
 +0 BugPattern OI_OPTIONAL_ISSUES_CHECKING_REFERENCE
++0 BugPattern OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED
 +0 BugPattern OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED
 +0 BugPattern OI_OPTIONAL_ISSUES_USES_DELAYED_EXECUTION
 +0 BugPattern OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION

diff --git a/etc/findbugs.xml b/etc/findbugs.xml
@@ -310,7 +310,7 @@
 
     <Detector class="com.mebigfatguy.fbcontrib.detect.UnsynchronizedSingletonFieldWrites" speed="fast" reports="USFW_UNSYNCHRONIZED_SINGLETON_FIELD_WRITES"/>
 
-    <Detector class="com.mebigfatguy.fbcontrib.detect.OptionalIssues" speed="fast" reports="OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION,OI_OPTIONAL_ISSUES_USES_DELAYED_EXECUTION,OI_OPTIONAL_ISSUES_CHECKING_REFERENCE,OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED,OI_OPTIONAL_ISSUES_USES_ORELSEGET_WITH_NULL"/>
+    <Detector class="com.mebigfatguy.fbcontrib.detect.OptionalIssues" speed="fast" reports="OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION,OI_OPTIONAL_ISSUES_USES_DELAYED_EXECUTION,OI_OPTIONAL_ISSUES_CHECKING_REFERENCE,OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED,OI_OPTIONAL_ISSUES_USES_ORELSEGET_WITH_NULL,OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED"/>
 
     <Detector class="com.mebigfatguy.fbcontrib.detect.UnnecessaryApiConversion" speed="fast" reports="UAC_UNNECESSARY_API_CONVERSION_DATE_TO_INSTANT,UAC_UNNECESSARY_API_CONVERSION_FILE_TO_PATH"/>
 
@@ -629,6 +629,7 @@
 	<BugPattern abbrev="OI" type="OI_OPTIONAL_ISSUES_CHECKING_REFERENCE" category="CORRECTNESS"/>
 	<BugPattern abbrev="OI" type="OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED" category="CORRECTNESS"/>
 	<BugPattern abbrev="OI" type="OI_OPTIONAL_ISSUES_USES_ORELSEGET_WITH_NULL" category="CORRECTNESS" />
+	<BugPattern abbrev="OI" type="OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED" category="CORRECTNESS"/>
 	<BugPattern abbrev="UAC" type="UAC_UNNECESSARY_API_CONVERSION_DATE_TO_INSTANT" category="CORRECTNESS"/>
 	<BugPattern abbrev="UAC" type="UAC_UNNECESSARY_API_CONVERSION_FILE_TO_PATH" category="CORRECTNESS"/>
 	<BugPattern abbrev="RFI" type="RFI_SET_ACCESSIBLE" category="CORRECTNESS"/>

diff --git a/etc/messages.xml b/etc/messages.xml
@@ -6115,6 +6115,18 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { }
     		]]>
     	</Details>
     </BugPattern>
+
+    <BugPattern type="OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED">
+    	<ShortDescription>Method uses Optional.equals(Optional.empty()), when Optional.isPresent is more readable</ShortDescription>
+    	<LongDescription>Method {1} uses Optional.equals(Optional.empty()), when Optional.isPresent is more readable</LongDescription>
+    	<Details>
+    		<![CDATA[
+    		<p>This method uses Optional.equals(Optional.empty()). It is more readable and more clear just to use the Optional.isPresent()
+    		method to determine whether the reference exists or not.
+    		</p>
+    		]]>
+    	</Details>    
+    </BugPattern>
 
     <BugPattern type="UAC_UNNECESSARY_API_CONVERSION_DATE_TO_INSTANT">
     	<ShortDescription>Method constructs a Date object, merely to convert it to an Instant object</ShortDescription>

diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/OptionalIssues.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/OptionalIssues.java
@@ -206,16 +206,23 @@ public void sawOpcode(int seen) {
                 String methodName = getNameConstantOperand();
                 curCalledMethod = new FQMethod(clsName, methodName, getSigConstantOperand());
 
-                if ("java/util/Optional".equals(clsName) && "of".equals(methodName)) {
-                    if (stack.getStackDepth() > 0) {
-                        OpcodeStack.Item itm = stack.getStackItem(0);
-                        String itmSig = itm.getSignature();
-                        if (BOXED_OPTIONAL_TYPES.contains(itmSig)) {
-                            bugReporter.reportBug(
-                                    new BugInstance(this, BugType.OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED.name(),
-                                            LOW_PRIORITY).addClass(this).addMethod(this).addSourceLine(this));
-                        }
-                    }
+                if ("java/util/Optional".equals(clsName)) {
+                	 if ("of".equals(methodName)) {
+	                    if (stack.getStackDepth() > 0) {
+	                        OpcodeStack.Item itm = stack.getStackItem(0);
+	                        String itmSig = itm.getSignature();
+	                        if (BOXED_OPTIONAL_TYPES.contains(itmSig)) {
+	                            bugReporter.reportBug(
+	                                    new BugInstance(this, BugType.OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED.name(),
+	                                            LOW_PRIORITY).addClass(this).addMethod(this).addSourceLine(this));
+	                        }
+	                    }
+                	 } else if ("equals".equals(methodName)) {
+                         bugReporter.reportBug(
+                                 new BugInstance(this, BugType.OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED.name(),
+                                         LOW_PRIORITY).addClass(this).addMethod(this).addSourceLine(this));
+
+                	 }
                 }
                 break;
 

diff --git a/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java b/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java
@@ -211,6 +211,7 @@ public enum BugType {
     ODN_ORPHANED_DOM_NODE,
     OI_OPTIONAL_ISSUES_CHECKING_REFERENCE,
     OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED,
+    OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED,
     OI_OPTIONAL_ISSUES_USES_DELAYED_EXECUTION,
     OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION,
     OI_OPTIONAL_ISSUES_USES_ORELSEGET_WITH_NULL,

diff --git a/src/samples/java/ex/OI_Sample.java b/src/samples/java/ex/OI_Sample.java
@@ -69,4 +69,8 @@ private Optional<String> get(String name) {
     public String fpGet384(String parameterName, Supplier<String> defaultValueSupplier) {
         return this.<String>get(parameterName).orElseGet(defaultValueSupplier);
     }
+
+    public boolean equalsToEmpty(Optional<String> foo) {
+    	return foo.equals(Optional.empty());
+    }
 }