diff --git a/etc/bugrank.txt b/etc/bugrank.txt index 48f3dc2a..82dcab38 100644 --- a/etc/bugrank.txt +++ b/etc/bugrank.txt @@ -88,6 +88,7 @@ +0 BugPattern IMC_IMMATURE_CLASS_UPPER_PACKAGE +0 BugPattern IMC_IMMATURE_CLASS_VAR_NAME +0 BugPattern IMC_IMMATURE_CLASS_WRONG_FIELD_ORDER ++0 BugPattern IMC_IMMATURE_CLASS_COLLECTION_RETURN +0 BugPattern IOI_COPY_WITH_READER +0 BugPattern IOI_DOUBLE_BUFFER_COPY +0 BugPattern IOI_UNENDED_ZLIB_OBJECT diff --git a/etc/findbugs.xml b/etc/findbugs.xml index 4c293580..d69a13d1 100755 --- a/etc/findbugs.xml +++ b/etc/findbugs.xml @@ -286,7 +286,7 @@ - + @@ -601,6 +601,7 @@ + diff --git a/etc/messages.xml b/etc/messages.xml index 787d56a5..a71c826a 100755 --- a/etc/messages.xml +++ b/etc/messages.xml @@ -5737,7 +5737,20 @@ for (Person p : people) {

A field or variable is named 'var' which will conflict with the built in Java 10 feature using 'var' as a keyword.

]]> -
+
+ + + Method returns java.util.Collection + Method {1} returns java.util.Collection +
+ This method declares that it returns a Collection, when in fact it has created and returned + a more specific interface. Since the caller is free to cast any return value to Collection, it is + better to return the specific interface created so that it gives the caller the most + power. With methods, always expect the least with parameters, and provide the most with return types.

+ ]]> +
+
JAX-RS Method implements a GET request but consumes input diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/ImmatureClass.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/ImmatureClass.java index 9c3ec6fa..2c47db8a 100644 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/ImmatureClass.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/ImmatureClass.java @@ -8,6 +8,7 @@ import org.apache.bcel.Const; import org.apache.bcel.Repository; import org.apache.bcel.classfile.AnnotationEntry; +import org.apache.bcel.classfile.Code; import org.apache.bcel.classfile.Constant; import org.apache.bcel.classfile.ConstantLong; import org.apache.bcel.classfile.ConstantValue; @@ -27,7 +28,9 @@ import edu.umd.cs.findbugs.BugInstance; import edu.umd.cs.findbugs.BugReporter; import edu.umd.cs.findbugs.BytecodeScanningDetector; +import edu.umd.cs.findbugs.OpcodeStack; import edu.umd.cs.findbugs.ba.ClassContext; +import edu.umd.cs.findbugs.ba.SignatureParser; /** * looks for classes that aren't fully flushed out to be easily usable for @@ -62,8 +65,10 @@ enum FieldStatus { } private BugReporter bugReporter; + private OpcodeStack stack; private FieldStatus fieldStatus = FieldStatus.NONE; private boolean classIsJPAEntity; + private String actualReturnType; public ImmatureClass(BugReporter reporter) { bugReporter = reporter; @@ -177,7 +182,12 @@ public void visitClassContext(ClassContext classContext) { } } - super.visitClassContext(classContext); + try { + stack = new OpcodeStack(); + super.visitClassContext(classContext); + } finally { + stack = null; + } } @Override @@ -253,6 +263,22 @@ public void visitMethod(Method m) { } } } + + public void visitCode(Code obj) { + + stack.resetForMethodEntry(this); + + String declaredReturnType = new SignatureParser(getMethodSig()).getReturnTypeSignature(); + actualReturnType = null; + + super.visitCode(obj); + + if ("Ljava/util/Collection;".equals(declaredReturnType) && !"Ljava/util/Collection;".equals(actualReturnType)) { + bugReporter.reportBug( + new BugInstance(this, BugType.IMC_IMMATURE_CLASS_COLLECTION_RETURN.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this, 0)); + } + } /** * implements the visitor to check for calls to Throwable.printStackTrace() @@ -261,12 +287,25 @@ public void visitMethod(Method m) { */ @Override public void sawOpcode(int seen) { - if ((seen == Const.INVOKEVIRTUAL) && "printStackTrace".equals(getNameConstantOperand()) - && SignatureBuilder.SIG_VOID_TO_VOID.equals(getSigConstantOperand())) { - bugReporter - .reportBug(new BugInstance(this, BugType.IMC_IMMATURE_CLASS_PRINTSTACKTRACE.name(), NORMAL_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } + try { + if ((seen == Const.INVOKEVIRTUAL) && "printStackTrace".equals(getNameConstantOperand()) + && SignatureBuilder.SIG_VOID_TO_VOID.equals(getSigConstantOperand())) { + bugReporter + .reportBug(new BugInstance(this, BugType.IMC_IMMATURE_CLASS_PRINTSTACKTRACE.name(), NORMAL_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } else if (seen == Const.ARETURN) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item itm = stack.getStackItem(0); + if (actualReturnType == null) { + actualReturnType = itm.getSignature(); + } else if (!actualReturnType.equals(itm.getSignature())) { + actualReturnType = ""; + } + } + } + } 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 index eca9c2dd..5d5eabf4 100644 --- a/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java @@ -119,6 +119,7 @@ public enum BugType { IMC_IMMATURE_CLASS_LOWER_CLASS, IMC_IMMATURE_CLASS_BAD_SERIALVERSIONUID, IMC_IMMATURE_CLASS_VAR_NAME, + IMC_IMMATURE_CLASS_COLLECTION_RETURN, IOI_COPY_WITH_READER, IOI_DOUBLE_BUFFER_COPY, IOI_UNENDED_ZLIB_OBJECT, diff --git a/src/samples/java/ex/IMC_Sample.java b/src/samples/java/ex/IMC_Sample.java index 256e5b00..4b8e0f1b 100644 --- a/src/samples/java/ex/IMC_Sample.java +++ b/src/samples/java/ex/IMC_Sample.java @@ -9,6 +9,10 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.Collection; +import java.util.Collections; +import java.util.Collection; +import java.util.Collections; import javax.persistence.Entity; @@ -47,6 +51,10 @@ public void psf(File f) { public void hasVar() { int var = 0; } + + public Collection returnsCollection() { + return Collections.emptySet(); + } } class NeedsEquals {