add detector FII_USE_COPYCONSTRUCTOR

mebigfatguy · Nov 23, 2024 · 174f64a · 174f64a
1 parent 6748661
commit 174f64a
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 5 deletions.
diff --git a/etc/bugrank.txt b/etc/bugrank.txt
@@ -61,6 +61,7 @@
 +0 BugPattern FII_AVOID_SIZE_ON_COLLECTED_STREAM
 +0 BugPattern FII_COMBINE_FILTERS
 +1 BugPattern FII_USE_ANY_MATCH
++0 BugPattern FII_USE_COPYCONSTRUCTOR
 +0 BugPattern FII_USE_FIND_FIRST
 +0 BugPattern FII_USE_FUNCTION_IDENTITY
 +0 BugPattern FII_USE_METHOD_REFERENCE

diff --git a/etc/findbugs.xml b/etc/findbugs.xml
@@ -326,7 +326,7 @@
 
     <Detector class="com.mebigfatguy.fbcontrib.detect.ListUsageIssues" speed="fast" reports="LUI_USE_SINGLETON_LIST,LUI_USE_COLLECTION_ADD,LUI_USE_GET0"/>
 
-    <Detector class="com.mebigfatguy.fbcontrib.detect.FunctionalInterfaceIssues" speed="fast" reports="FII_USE_METHOD_REFERENCE,FII_AVOID_CONTAINS_ON_COLLECTED_STREAM,FII_USE_ANY_MATCH,FII_USE_FIND_FIRST,FII_COMBINE_FILTERS,FII_USE_FUNCTION_IDENTITY,FII_AVOID_SIZE_ON_COLLECTED_STREAM" />
+    <Detector class="com.mebigfatguy.fbcontrib.detect.FunctionalInterfaceIssues" speed="fast" reports="FII_USE_METHOD_REFERENCE,FII_AVOID_CONTAINS_ON_COLLECTED_STREAM,FII_USE_ANY_MATCH,FII_USE_FIND_FIRST,FII_COMBINE_FILTERS,FII_USE_FUNCTION_IDENTITY,FII_AVOID_SIZE_ON_COLLECTED_STREAM,FII_USE_COPYCONSTRUCTOR" />
 
     <Detector class="com.mebigfatguy.fbcontrib.detect.SetUsageIssues" speed="fast" reports="SUI_CONTAINS_BEFORE_ADD,SUI_CONTAINS_BEFORE_REMOVE"/>
 
@@ -650,6 +650,7 @@
 	<BugPattern abbrev="FII" type="FII_COMBINE_FILTERS" category="CORRECTNESS"/>
 	<BugPattern abbrev="FII" type="FII_USE_FUNCTION_IDENTITY" category="CORRECTNESS"/>
 	<BugPattern abbrev="FII" type="FII_AVOID_SIZE_ON_COLLECTED_STREAM" category="CORRECTNESS"/>
+	<BugPattern abbrev="FII" type="FII_USE_COPYCONSTRUCTOR" category="CORRECTNESS"/>
 	<BugPattern abbrev="SUI" type="SUI_CONTAINS_BEFORE_ADD" category="CORRECTNESS" experimental="true" />
 	<BugPattern abbrev="SUI" type="SUI_CONTAINS_BEFORE_REMOVE" category="CORRECTNESS" experimental="true" />
     <BugPattern abbrev="SAT" type="SAT_SUSPICIOUS_ARGUMENT_TYPES" category="CORRECTNESS" experimental="true" />

diff --git a/etc/messages.xml b/etc/messages.xml
@@ -6496,6 +6496,25 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { }
         </Details>
     </BugPattern>
 
+    <BugPattern type="FII_USE_COPYCONSTRUCTOR">
+        <ShortDescription>Method calls collect() directly on a stream</ShortDescription>
+        <LongDescription>Method {1} calls collect() directly on a stream</LongDescription>
+        <Details>
+        <![CDATA[
+        	Code perform a "copy constructor" operation by using streams, when it is more simply done
+        	by passing the source collection into a new Collection constructors argument. Use, 
+        	
+        	<code><pre>
+        		Set<String> s = new HashSet<myArrayList);
+        	</pre></code>
+        	
+        	<code><pre>
+        		Set<String> s = myArrayList.stream().collect(Collectors.toSet);
+        	</pre></code>
+        ]]>
+        </Details>
+    </BugPattern>
+
     <BugPattern type="SUI_CONTAINS_BEFORE_ADD">
         <ShortDescription>Method checks for an item in a set with contains, before using add()</ShortDescription>
         <LongDescription>Method {1} checks for an item in a set with contains, before using add()</LongDescription>

diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/FunctionalInterfaceIssues.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/FunctionalInterfaceIssues.java
@@ -68,6 +68,7 @@ public class FunctionalInterfaceIssues extends BytecodeScanningDetector {
 
     private static final QMethod CONTAINS = new QMethod("contains", SignatureBuilder.SIG_OBJECT_TO_BOOLEAN);
     private static final QMethod SIZE = new QMethod("size", SignatureBuilder.SIG_VOID_TO_INT);
+    private static final QMethod STREAM = new QMethod("stream", "()Ljava/util/stream/Stream;");
 
     private static final FQMethod COLLECT = new FQMethod("java/util/stream/Stream", "collect",
             "(Ljava/util/stream/Collector;)Ljava/lang/Object;");
@@ -77,6 +78,9 @@ public class FunctionalInterfaceIssues extends BytecodeScanningDetector {
             "()Ljava/util/Optional;");
     private static final FQMethod ISPRESENT = new FQMethod("java/util/Optional", "isPresent",
             SignatureBuilder.SIG_VOID_TO_BOOLEAN);
+    private static final FQMethod TOLIST = new FQMethod("java/util/stream/Collectors", "toList", "()Ljava/util/stream/Collector;");
+    private static final FQMethod TOSET = new FQMethod("java/util/stream/Collectors", "toSet", "()Ljava/util/stream/Collector;");
+
     private static final FQMethod GET = new FQMethod("java/util/List", "get", SignatureBuilder.SIG_INT_TO_OBJECT);
 
     enum ParseState {
@@ -88,7 +92,7 @@ enum AnonState {
     }
 
     enum FIIUserValue {
-        COLLECT_ITEM, FILTER_ITEM, FINDFIRST_ITEM;
+    	STREAM_ITEM, COLLECT_ITEM, FILTER_ITEM, FINDFIRST_ITEM;
     }
 
     private BugReporter bugReporter;
@@ -290,7 +294,7 @@ public void sawOpcode(int seen) {
                 }
             } else {
                 switch (seen) {
-                case Const.INVOKEDYNAMIC:
+                case Const.INVOKEDYNAMIC: {
                     ConstantInvokeDynamic cid = (ConstantInvokeDynamic) getConstantRefOperand();
 
                     ConstantMethodHandle cmh = getMethodHandle(cid.getBootstrapMethodAttrIndex());
@@ -310,8 +314,9 @@ public void sawOpcode(int seen) {
                         fiis.add(fii);
                     }
                     break;
+                }
 
-                case Const.INVOKEINTERFACE:
+                case Const.INVOKEINTERFACE: {
                     QMethod m = new QMethod(getNameConstantOperand(), getSigConstantOperand());
 
                     if (CONTAINS.equals(m)) {
@@ -332,6 +337,10 @@ public void sawOpcode(int seen) {
                                                 NORMAL_PRIORITY).addClass(this).addMethod(this).addSourceLine(this));
                             }
                         }
+                    } else if (STREAM.equals(m)) {
+                        if (stack.getStackDepth() >= 1) {
+                            userValue = FIIUserValue.STREAM_ITEM;
+                        }
                     } else {
                         FQMethod fqm = new FQMethod(getClassConstantOperand(), getNameConstantOperand(),
                                 getSigConstantOperand());
@@ -369,8 +378,9 @@ public void sawOpcode(int seen) {
                         }
                     }
                     break;
+                }
 
-                case Const.INVOKEVIRTUAL:
+                case Const.INVOKEVIRTUAL: {
                     FQMethod fqm = new FQMethod(getClassConstantOperand(), getNameConstantOperand(),
                             getSigConstantOperand());
                     if (ISPRESENT.equals(fqm)) {
@@ -385,6 +395,24 @@ public void sawOpcode(int seen) {
                     }
                     break;
                 }
+
+                case Const.INVOKESTATIC: {
+                    if (stack.getStackDepth() > 0) {
+                        OpcodeStack.Item itm = stack.getStackItem(0);
+                        FIIUserValue uv = (FIIUserValue) itm.getUserValue();
+                        if (uv == FIIUserValue.STREAM_ITEM) {
+		                    FQMethod fqm = new FQMethod(getClassConstantOperand(), getNameConstantOperand(),
+		                            getSigConstantOperand());
+		                    if (TOLIST.equals(fqm) || TOSET.equals(fqm)) {
+                                bugReporter
+                                .reportBug(new BugInstance(this, BugType.FII_USE_COPYCONSTRUCTOR.name(), NORMAL_PRIORITY)
+                                        .addClass(this).addMethod(this).addSourceLine(this));
+		                    }
+                        }
+                    }
+                	break;
+                }
+                }
             }
         } finally {
             stack.sawOpcode(this, seen);

diff --git a/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java b/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java
@@ -90,6 +90,7 @@ public enum BugType {
     FII_AVOID_SIZE_ON_COLLECTED_STREAM,
     FII_COMBINE_FILTERS,
     FII_USE_ANY_MATCH,
+    FII_USE_COPYCONSTRUCTOR,
     FII_USE_FIND_FIRST,
     FII_USE_FUNCTION_IDENTITY,
     FII_USE_METHOD_REFERENCE,

diff --git a/src/samples/java/ex/FII_Sample.java b/src/samples/java/ex/FII_Sample.java
@@ -69,6 +69,10 @@ public Map<String, Bauble> mapIdentity(List<Bauble> baubles) {
     public int sizeOnACollect(List<Bauble> baubles, String name) {
         return baubles.stream().filter(b -> b.getName().equals(name)).collect(Collectors.toSet()).size();
     }
+
+    public List<String> streamingRatherThanCC(Set<String> s) {
+    	return s.stream().collect(Collectors.toList());    
+    }
 
     public void fpUnrelatedLambdaValue282(Map<String, Bauble> map, BaubleFactory factory) {
         map.computeIfAbsent("pixie dust", _unused -> factory.getBauble());