[Fb-contrib-commit] SF.net SVN: fb-contrib: [960] trunk/fb-contrib
Brought to you by:
dbrosius
Menu
▾
▴
[Fb-contrib-commit] SF.net SVN: fb-contrib: [960] trunk/fb-contrib
|
From: <dbr...@us...> - 2007-11-08 06:31:34
|
Revision: 960
http://fb-contrib.svn.sourceforge.net/fb-contrib/?rev=960&view=rev
Author: dbrosius
Date: 2007-11-07 22:31:38 -0800 (Wed, 07 Nov 2007)
Log Message:
-----------
add SPP_SUSPECT_STRING_TEST bug pattern
Modified Paths:
--------------
trunk/fb-contrib/etc/findbugs.xml
trunk/fb-contrib/etc/messages.xml
trunk/fb-contrib/samples/SPP_Sample.java
trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SillynessPotPourri.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml 2007-11-07 07:16:36 UTC (rev 959)
+++ trunk/fb-contrib/etc/findbugs.xml 2007-11-08 06:31:38 UTC (rev 960)
@@ -256,7 +256,7 @@
<Detector class="com.mebigfatguy.fbcontrib.detect.SillynessPotPourri"
speed="fast"
- reports="SPP_NEGATIVE_BITSET_ITEM,SPP_INTERN_ON_CONSTANT,SPP_NO_CHAR_SB_CTOR,SPP_USE_MATH_CONSTANT,SPP_STUTTERED_ASSIGNMENT,SPP_USE_ISNAN,SPP_USE_BIGDECIMAL_STRING_CTOR,SPP_STRINGBUFFER_WITH_EMPTY_STRING,SPP_EQUALS_ON_ENUM,SPP_INVALID_BOOLEAN_NULL_CHECK,SPP_USE_CHARAT,SPP_USELESS_TRINARY" />
+ reports="SPP_NEGATIVE_BITSET_ITEM,SPP_INTERN_ON_CONSTANT,SPP_NO_CHAR_SB_CTOR,SPP_USE_MATH_CONSTANT,SPP_STUTTERED_ASSIGNMENT,SPP_USE_ISNAN,SPP_USE_BIGDECIMAL_STRING_CTOR,SPP_STRINGBUFFER_WITH_EMPTY_STRING,SPP_EQUALS_ON_ENUM,SPP_INVALID_BOOLEAN_NULL_CHECK,SPP_USE_CHARAT,SPP_USELESS_TRINARY,SPP_SUSPECT_STRING_TEST" />
<Detector class="com.mebigfatguy.fbcontrib.detect.BloatedAssignmentScope"
speed="fast"
@@ -387,6 +387,7 @@
<BugPattern abbrev="SPP" type="SPP_INVALID_BOOLEAN_NULL_CHECK" category="CORRECTNESS" />
<BugPattern abbrev="SPP" type="SPP_USE_CHARAT" category="PERFORMANCE" />
<BugPattern abbrev="SPP" type="SPP_USELESS_TRINARY" category="PERFORMANCE" />
+ <BugPattern abbrev="SPP" type="SPP_SUSPECT_STRING_TEST" category="CORRECTNESS" />
<BugPattern abbrev="BAS" type="BAS_BLOATED_ASSIGNMENT_SCOPE" category="PERFORMANCE" />
<BugPattern abbrev="SCII" type="SCII_SPOILED_CHILD_INTERFACE_IMPLEMENTATOR" category="STYLE" />
<BugPattern abbrev="DWI" type="DWI_DELETING_WHILE_ITERATING" category="CORRECTNESS" />
Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml 2007-11-07 07:16:36 UTC (rev 959)
+++ trunk/fb-contrib/etc/messages.xml 2007-11-08 06:31:38 UTC (rev 960)
@@ -1895,6 +1895,18 @@
</Details>
</BugPattern>
+ <BugPattern type="SPP_SUSPECT_STRING_TEST">
+ <ShortDescription>Method treats null and normal strings differently than an empty strings</ShortDescription>
+ <LongDescription>Method {1} treats null and normal strings differently than an empty strings</LongDescription>
+ <Details>
+ <![CDATA[
+ <p>This method tests a string, and groups null values with real strings, leaving empty strings as another
+ case. This might be perfectly valid, but normally, null strings and empty strings are logically handled the same,
+ and so this test may be flawed.</p>
+ ]]>
+ </Details>
+ </BugPattern>
+
<BugPattern type="BAS_BLOATED_ASSIGNMENT_SCOPE">
<ShortDescription>Method assigns a variable in a larger scope then is needed</ShortDescription>
<LongDescription>Method {1} assigns a variable in a larger scope then is needed</LongDescription>
Modified: trunk/fb-contrib/samples/SPP_Sample.java
===================================================================
--- trunk/fb-contrib/samples/SPP_Sample.java 2007-11-07 07:16:36 UTC (rev 959)
+++ trunk/fb-contrib/samples/SPP_Sample.java 2007-11-08 06:31:38 UTC (rev 960)
@@ -96,4 +96,14 @@
{
return (s.length() != 0);
}
+
+ public void testSuspiciousStringTests(String s)
+ {
+ if ((s == null) || (s.length() > 0))
+ System.out.println("Booya");
+ if ((s == null) || (s.length() != 0))
+ System.out.println("Booya");
+ if ((s != null) && (s.length() == 0))
+ System.out.println("Booya");
+ }
}
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SillynessPotPourri.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SillynessPotPourri.java 2007-11-07 07:16:36 UTC (rev 959)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SillynessPotPourri.java 2007-11-08 06:31:38 UTC (rev 960)
@@ -27,7 +27,12 @@
import org.apache.bcel.Repository;
import org.apache.bcel.classfile.Code;
import org.apache.bcel.classfile.ConstantDouble;
+import org.apache.bcel.classfile.ConstantMethodref;
+import org.apache.bcel.classfile.ConstantNameAndType;
+import org.apache.bcel.classfile.ConstantPool;
import org.apache.bcel.classfile.JavaClass;
+import org.apache.bcel.classfile.LocalVariable;
+import org.apache.bcel.classfile.LocalVariableTable;
import com.mebigfatguy.fbcontrib.utils.Integer14;
import com.mebigfatguy.fbcontrib.utils.RegisterUtils;
@@ -37,6 +42,7 @@
import edu.umd.cs.findbugs.BytecodeScanningDetector;
import edu.umd.cs.findbugs.OpcodeStack;
import edu.umd.cs.findbugs.ba.ClassContext;
+import edu.umd.cs.findbugs.visitclass.LVTHelper;
/**
* looks for silly bugs that are simple but do not fit into one large pattern.
@@ -49,6 +55,7 @@
private int lastOpcode;
private int lastReg;
private boolean lastIfEqWasBoolean;
+ private boolean lastLoadWasString;
/** branch targets, to a set of branch instructions */
private Map<Integer, Set<Integer>> branchTargets;
@@ -85,6 +92,7 @@
lastOpcode = -1;
lastReg = -1;
lastIfEqWasBoolean = false;
+ lastLoadWasString = false;
Arrays.fill(lastPCs, -1);
branchTargets.clear();
super.visitCode(obj);
@@ -113,6 +121,34 @@
branchInsSet.add(Integer14.valueOf(getPC()));
}
+ if ((seen == IFEQ) || (seen == IFLE) || (seen == IFNE)) {
+ if (lastLoadWasString && (lastPCs[0] != -1)) {
+ byte[] bytes = getCode().getCode();
+ int loadIns = get1Byte(bytes, lastPCs[2]);
+
+ if ((((loadIns >= ALOAD_0) && (loadIns <= ALOAD_3)) || (loadIns == ALOAD))
+ && (get1Byte(bytes, lastPCs[3]) == INVOKEVIRTUAL)
+ && (get1Byte(bytes, lastPCs[2]) == loadIns)
+ && (get1Byte(bytes, lastPCs[1]) == IFNULL)
+ && (get1Byte(bytes, lastPCs[0]) == loadIns)
+ && ((loadIns != ALOAD) || (get1Byte(bytes, lastPCs[2]+1) == get1Byte(bytes, lastPCs[0]+1)))
+ && ((seen == IFNE) ? get2Bytes(bytes, lastPCs[1]+1) > 10 : get2Bytes(bytes, lastPCs[1]+1) == 10)) {
+ ConstantPool pool = getConstantPool();
+ int mpoolIndex = get2Bytes(bytes, lastPCs[3]+1);
+ ConstantMethodref cmr = (ConstantMethodref)pool.getConstant(mpoolIndex);
+ int nandtIndex = cmr.getNameAndTypeIndex();
+ ConstantNameAndType cnt = (ConstantNameAndType)pool.getConstant(nandtIndex);
+ if ("length".equals(cnt.getName(pool))) {
+ bugReporter.reportBug(new BugInstance(this, "SPP_SUSPECT_STRING_TEST", NORMAL_PRIORITY)
+ .addClass(this)
+ .addMethod(this)
+ .addSourceLine(this));
+ }
+ }
+ }
+ }
+
+
if (seen == IFEQ) {
if (stack.getStackDepth() > 0) {
OpcodeStack.Item itm = stack.getStackItem(0);
@@ -185,7 +221,7 @@
.addSourceLine(this));
}
}
- } else if (((seen >= ASTORE_0) && (seen < ASTORE_3)) || (seen == ASTORE)) {
+ } else if (((seen >= ASTORE_0) && (seen <= ASTORE_3)) || (seen == ASTORE)) {
reg = RegisterUtils.getAStoreReg(this, seen);
if (seen == lastOpcode) {
if (reg == lastReg) {
@@ -195,6 +231,15 @@
.addSourceLine(this));
}
}
+ } else if (((seen >= ALOAD_0) && (seen <= ASTORE_3)) || (seen == ALOAD)) {
+ lastLoadWasString = false;
+ LocalVariableTable lvt = getMethod().getLocalVariableTable();
+ if (lvt != null) {
+ LocalVariable lv = LVTHelper.getLocalVariableAtPC(lvt, RegisterUtils.getALoadReg(this, seen), getPC());
+ if (lv != null) {
+ lastLoadWasString = "Ljava/lang/String;".equals(lv.getSignature());
+ }
+ }
} else if ((seen >= ICONST_0) && (seen <= ICONST_3)) {
if (stack.getStackDepth() > 0) {
OpcodeStack.Item item = stack.getStackItem(0);
@@ -359,4 +404,14 @@
lastPCs[3] = getPC();
}
}
+
+ private int get1Byte(byte[] bytes, int offset)
+ {
+ return (0x00FF & bytes[offset]);
+ }
+
+ private int get2Bytes(byte[] bytes, int offset)
+ {
+ return (0x0000FFFF & (bytes[offset] << 8)) | (0x00FF & bytes[offset+1]);
+ }
}
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|
