fb-contrib-commit — Log for cvs commits of fb-contrib code

[Fb-contrib-commit] SF.net SVN: fb-contrib: [540] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java

[<dbr...@us...>]

Revision: 540
Author:   dbrosius
Date:     2006-05-16 21:03:41 -0700 (Tue, 16 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=540&view=rev

Log Message:
-----------
prioritize based on array size

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	2006-05-16 04:33:50 UTC (rev 539)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	2006-05-17 04:03:41 UTC (rev 540)
@@ -126,7 +126,7 @@
 			case SEEN_INDEX_STORE:
 				if ((seen == ASTORE)
 				||  ((seen >= ASTORE_0) && (seen <= ASTORE_3))) {
-					bugReporter.reportBug(new BugInstance(this, "SACM_STATIC_ARRAY_CREATED_IN_METHOD", NORMAL_PRIORITY)
+					bugReporter.reportBug(new BugInstance(this, "SACM_STATIC_ARRAY_CREATED_IN_METHOD", (arraySize < 3) ? LOW_PRIORITY : ((arraySize < 10) ? NORMAL_PRIORITY : HIGH_PRIORITY))
 							   .addClass(this)
 							   .addMethod(this)
 							   .addSourceLine(this, getPC()));


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [539] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java

[<dbr...@us...>]

Revision: 539
Author:   dbrosius
Date:     2006-05-15 21:33:50 -0700 (Mon, 15 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=539&view=rev

Log Message:
-----------
make sure the supposed array size is > 0

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	2006-05-16 04:17:35 UTC (rev 538)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	2006-05-16 04:33:50 UTC (rev 539)
@@ -67,10 +67,12 @@
 			case SEEN_NOTHING:
 				if (seen == BIPUSH) {
 					arraySize = getIntConstant();
-					state = SEEN_ARRAY_SIZE;
+					if (arraySize > 0)
+						state = SEEN_ARRAY_SIZE;
 				}else if ((seen >= ICONST_M1) && (seen <= ICONST_5)) {
 					arraySize = seen - ICONST_M1 - 1;
-					state = SEEN_ARRAY_SIZE;
+					if (arraySize > 0)
+						state = SEEN_ARRAY_SIZE;
 				}
 			break;
 			


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [538] trunk/fb-contrib/etc

[<dbr...@us...>]

Revision: 538
Author:   dbrosius
Date:     2006-05-15 21:17:35 -0700 (Mon, 15 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=538&view=rev

Log Message:
-----------
initial checkin - new SACM Detector

Modified Paths:
--------------
    trunk/fb-contrib/etc/findbugs.xml
    trunk/fb-contrib/etc/messages.xml

Added Paths:
-----------
    trunk/fb-contrib/samples/SACM_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml	2006-05-16 03:10:39 UTC (rev 537)
+++ trunk/fb-contrib/etc/findbugs.xml	2006-05-16 04:17:35 UTC (rev 538)
@@ -203,6 +203,10 @@
 			speed="moderate"
 			reports="ITC_INHERITANCE_TYPE_CHECKING" />
 			
+	<Detector class="com.mebigfatguy.fbcontrib.detect.StaticArrayCreatedInMethod"
+			speed="fast"
+			reports="SACM_STATIC_ARRAY_CREATED_IN_METHOD" />
+			
    	<!-- BugPattern -->
 	
 	<BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" />
@@ -256,4 +260,5 @@
 	<BugPattern abbrev="SIL" type="SIL_SQL_IN_LOOP" category="PERFORMANCE" experimental="true" />
 	<BugPattern abbrev="NMCS" type="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION" category="PERFORMANCE" experimental="true" />
 	<BugPattern abbrev="ITC" type="ITC_INHERITANCE_TYPE_CHECKING" category="STYLE" experimental="true" />
+	<BugPattern abbrev="SACM" type="SACM_STATIC_ARRAY_CREATED_IN_METHOD" category="PERFORMANCE" experimental="true" />
 </FindbugsPlugin>
\ No newline at end of file

Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml	2006-05-16 03:10:39 UTC (rev 537)
+++ trunk/fb-contrib/etc/messages.xml	2006-05-16 04:17:35 UTC (rev 538)
@@ -579,6 +579,17 @@
 			]]>
 		</Details>
 	</Detector>
+	
+	<Detector class="com.mebigfatguy.fbcontrib.detect.StaticArrayCreatedInMethod">
+		<Details>
+			<![CDATA[
+			<p>looks for creation of arrays in methods using constant values. These arrays
+			will need to be recreated each time the method is called. These arrays should probably
+			be defined as static fields, instead</p>
+			<p>It is a fast detector</p>
+			]]>
+		</Details>
+	</Detector> 
 
 	<!-- BugPattern -->
 	                
@@ -1246,6 +1257,18 @@
 		</Details>
 	</BugPattern>
 	
+	<BugPattern type="SACM_STATIC_ARRAY_CREATED_IN_METHOD">
+		<ShortDescription>Method creates array using constants</ShortDescription>
+		<LongDescription>Method {1} creates array using constants</LongDescription>
+		<Details>
+			<![CDATA[
+				<p>This method creates an initialized by constants. Each time this method is called
+				this array will be recreated. It would be more performant to define the array as a
+				static field of the class instead.</p>
+			]]>
+		</Details>
+	</BugPattern>
+	
 	<!-- BugCode -->
 	
 	<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -1295,4 +1318,5 @@
 	<BugCode abbrev="SIL">SQL In Loop</BugCode>
 	<BugCode abbrev="NMCS">Needless Member Collection Synchronization</BugCode>
 	<BugCode abbrev="ITC">Inheritance Type Checking</BugCode>
+	<BugCode abbrev="SACM">Static Array Created in Method</BugCode>
 </MessageCollection>
\ No newline at end of file

Added: trunk/fb-contrib/samples/SACM_Sample.java
===================================================================
--- trunk/fb-contrib/samples/SACM_Sample.java	                        (rev 0)
+++ trunk/fb-contrib/samples/SACM_Sample.java	2006-05-16 04:17:35 UTC (rev 538)
@@ -0,0 +1,12 @@
+
+public class SACM_Sample 
+{
+	public String test(int i)
+	{
+		String[] giantSounds = new String[] { "fee", "fi", "fo", "fum", "burp", "fart" };
+		if ((i < 0) || (i >= giantSounds.length))
+			return "";
+		
+		return giantSounds[i];
+	}
+}

Added: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	                        (rev 0)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/StaticArrayCreatedInMethod.java	2006-05-16 04:17:35 UTC (rev 538)
@@ -0,0 +1,136 @@
+package com.mebigfatguy.fbcontrib.detect;
+
+import org.apache.bcel.classfile.Code;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.BytecodeScanningDetector;
+import edu.umd.cs.findbugs.ba.ClassContext;
+
+/**
+ * looks for creation of arrays where the contents are constants, or static
+ * fields, and the array isn't further modified. These arrays should probably
+ * be defined as static fields so the method doesn't constantly recreate the array
+ * each time it is called.
+ */
+public class StaticArrayCreatedInMethod extends BytecodeScanningDetector 
+{
+	private static final int SEEN_NOTHING = 0;
+	private static final int SEEN_ARRAY_SIZE = 1;
+	private static final int SEEN_NEWARRAY = 2;
+	private static final int SEEN_DUP = 3;
+	private static final int SEEN_INDEX = 4;
+	private static final int SEEN_LDC = 5;
+	private static final int SEEN_INDEX_STORE = 6;
+	
+	private BugReporter bugReporter;
+	private int arraySize;
+	private int storeCount;
+	private int state;
+	
+	public StaticArrayCreatedInMethod(BugReporter bugReporter) {
+		this.bugReporter = bugReporter;
+	}
+	
+	@Override
+	public void visitClassContext(ClassContext classContext) {
+		try {
+			super.visitClassContext(classContext);
+		} finally {
+			
+		}
+	}
+	
+	/**
+	 * implements the visitor by forwarding calls for methods that are the static initializer
+	 * 
+	 * @obj the context object of the currently parsed code block
+	 */
+	@Override
+	public void visitCode(Code obj) {
+		if (!"<clinit>".equals(getMethodName())) {
+			state = SEEN_NOTHING;
+			super.visitCode(obj);
+		}
+	}
+	
+	/**
+	 * implements the visitor to look for creation of local arrays using constant values
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
+	@Override
+	public void sawOpcode(int seen) {
+		int index;
+		
+		switch (state) {
+			case SEEN_NOTHING:
+				if (seen == BIPUSH) {
+					arraySize = getIntConstant();
+					state = SEEN_ARRAY_SIZE;
+				}else if ((seen >= ICONST_M1) && (seen <= ICONST_5)) {
+					arraySize = seen - ICONST_M1 - 1;
+					state = SEEN_ARRAY_SIZE;
+				}
+			break;
+			
+			case SEEN_ARRAY_SIZE:
+				if ((seen == ANEWARRAY) || (seen == NEWARRAY)) {
+					state = SEEN_NEWARRAY;
+					storeCount = 0;
+				}
+				else
+					state = SEEN_NOTHING;
+			break;
+			
+			case SEEN_NEWARRAY:
+				if (seen == DUP)
+					state = SEEN_DUP;
+				else
+					state = SEEN_NOTHING;
+			break;
+				
+			case SEEN_DUP:
+				if (seen == BIPUSH)
+					index = getIntConstant();
+				else if ((seen >= ICONST_M1) && (seen <= ICONST_5))
+					index = seen - ICONST_M1 - 1;
+				else {
+					state = SEEN_NOTHING;
+					return;
+				}
+				if (index != storeCount)
+					state = SEEN_NOTHING;
+				else
+					state = SEEN_INDEX;
+			break;
+			
+			case SEEN_INDEX:
+				if (seen == LDC)
+					state = SEEN_LDC;
+				else
+					state = SEEN_NOTHING;
+			break;
+			
+			case SEEN_LDC:
+				if ((seen >= IASTORE) && (seen <= SASTORE)) {
+					if ((++storeCount) == arraySize)
+						state = SEEN_INDEX_STORE;
+					else
+						state = SEEN_NEWARRAY;
+				}
+			break;
+			
+			case SEEN_INDEX_STORE:
+				if ((seen == ASTORE)
+				||  ((seen >= ASTORE_0) && (seen <= ASTORE_3))) {
+					bugReporter.reportBug(new BugInstance(this, "SACM_STATIC_ARRAY_CREATED_IN_METHOD", NORMAL_PRIORITY)
+							   .addClass(this)
+							   .addMethod(this)
+							   .addSourceLine(this, getPC()));
+				}
+				state = SEEN_NOTHING;
+			break;
+		}
+	}
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [537] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 537
Author:   dbrosius
Date:     2006-05-15 20:10:39 -0700 (Mon, 15 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=537&view=rev

Log Message:
-----------
don't report NMCS on fields that are passed as parameters to methods in ordinary methods

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-15 03:31:05 UTC (rev 536)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-16 03:10:39 UTC (rev 537)
@@ -288,7 +288,12 @@
 							}
 						}
 					}
+                    removeCollectionParameters();
 				break;
+                
+                case INVOKESTATIC:
+                    removeCollectionParameters();
+                break;
 				
 				case ARETURN:
 					if (stack.getStackDepth() > 0) {
@@ -372,6 +377,24 @@
 			}
 		}
 	}
+    
+    /**
+     * removes collection fields that are passed to other methods as arguments
+     * 
+     * @param seen the opcode of the currently parsed instruction
+     */
+    private void removeCollectionParameters() {
+        int parmCount = Type.getArgumentTypes(getSigConstantOperand()).length;
+        if (stack.getStackDepth() >= parmCount) {
+            for (int i = 0; i < parmCount; i++) {
+                OpcodeStack.Item item = stack.getStackItem(i);
+                FieldAnnotation fa = item.getField();
+                if (fa != null) {
+                    collectionFields.remove(fa.getFieldName());
+                }
+            }
+        }
+    }
 	
 	/**
 	 * holds information about a field, namely the annotation and 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [536] trunk/fb-contrib/htdocs/index.html

[<dbr...@us...>]

Revision: 536
Author:   dbrosius
Date:     2006-05-14 20:31:05 -0700 (Sun, 14 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=536&view=rev

Log Message:
-----------
doc new detectors

Modified Paths:
--------------
    trunk/fb-contrib/htdocs/index.html
Modified: trunk/fb-contrib/htdocs/index.html
===================================================================
--- trunk/fb-contrib/htdocs/index.html	2006-05-15 03:26:53 UTC (rev 535)
+++ trunk/fb-contrib/htdocs/index.html	2006-05-15 03:31:05 UTC (rev 536)
@@ -46,22 +46,30 @@
 		<a href="javadoc/index.html">JavaDoc</a>
 	
 		<hr/>
-		<img id="svn_image" src="flip1.gif" onClick="toggleBlock('svn', 'svn_image');" align="top"/>
+		<img id="svn_image" src="flip2.gif" onClick="toggleBlock('svn', 'svn_image');" align="top"/>
 		Detectors added in svn
-		<div id="svn" style="display:none;">
+		<div id="svn" style="display:block;">
 			<ul>
 				<li><b>[BSB] Bloated Synchronized Block</b><br/>
 				Looks for methods that implement synchronized blocks that appear to contain some code at 
 				the beginning that does not need to be synchronized. Moving these lines out of the synchronized
 				block should help multithreaded performance.</li>
+				<li><b>[NMCS] Needless Member Collection Synchronization</b><br/>
+				Looks for private collection members, either static or instance, that are only initialized in
+				the clinit or init, but are synchronized. This is not necessary as the constructor or static 
+ 				initializer are guaranteed to be thread safe.</li>
+ 				<li><b>[ITC] Inheritance Type Checking</b><br/>
+ 				Looks for if/else blocks where a series of them use instanceof on the same 
+				variable to determine what do to. If these classes are related by inheritance,
+				this often is better handled through calling a single overridden method.</li>
 			</ul>
 		</div>
 
 		<hr/>
-		<img id="v2_6_0_image" src="flip2.gif" onClick="toggleBlock('v2_6_0', 'v2_6_0_image');" align="top"/>
+		<img id="v2_6_0_image" src="flip1.gif" onClick="toggleBlock('v2_6_0', 'v2_6_0_image');" align="top"/>
 		Detectors added in v2.6.0<br/>
 		Note: This version of fb-contrib requires FindBugs v0.9.7.<br/>
-		<div id="v2_6_0" style="display:block;">
+		<div id="v2_6_0" style="display:none;">
 			<ul>
 				<li><b>[FCBL] Field could be Local</b><br/>
 				Looks for classes that declare fields that are used in a locals-only fashion, specifically private fields 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [535] trunk/fb-contrib/etc

[<dbr...@us...>]

Revision: 535
Author:   dbrosius
Date:     2006-05-14 20:26:53 -0700 (Sun, 14 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=535&view=rev

Log Message:
-----------
initial checkin - new ITC Detector

Modified Paths:
--------------
    trunk/fb-contrib/etc/findbugs.xml
    trunk/fb-contrib/etc/messages.xml

Added Paths:
-----------
    trunk/fb-contrib/samples/ITC_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/InheritanceTypeChecking.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml	2006-05-15 02:06:52 UTC (rev 534)
+++ trunk/fb-contrib/etc/findbugs.xml	2006-05-15 03:26:53 UTC (rev 535)
@@ -199,6 +199,10 @@
 			speed="moderate"
 			reports="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION" />
 			
+	<Detector class="com.mebigfatguy.fbcontrib.detect.InheritanceTypeChecking"
+			speed="moderate"
+			reports="ITC_INHERITANCE_TYPE_CHECKING" />
+			
    	<!-- BugPattern -->
 	
 	<BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" />
@@ -251,4 +255,5 @@
 	<BugPattern abbrev="UEC" type="UEC_USE_ENUM_COLLECTIONS" category="PERFORMANCE" />
 	<BugPattern abbrev="SIL" type="SIL_SQL_IN_LOOP" category="PERFORMANCE" experimental="true" />
 	<BugPattern abbrev="NMCS" type="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION" category="PERFORMANCE" experimental="true" />
+	<BugPattern abbrev="ITC" type="ITC_INHERITANCE_TYPE_CHECKING" category="STYLE" experimental="true" />
 </FindbugsPlugin>
\ No newline at end of file

Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml	2006-05-15 02:06:52 UTC (rev 534)
+++ trunk/fb-contrib/etc/messages.xml	2006-05-15 03:26:53 UTC (rev 535)
@@ -568,6 +568,17 @@
 			]]>
 		</Details>
 	</Detector>
+	
+	<Detector class="com.mebigfatguy.fbcontrib.detect.InheritanceTypeChecking">
+		<Details>
+			<![CDATA[
+			<p>looks for if/else blocks where a series of them use instanceof on the same 
+			variable to determine what do to. If these classes are related by inheritance,
+			this often is better handled through calling a single overridden method.</p>
+			<p>It is a moderately fast detector</p>
+			]]>
+		</Details>
+	</Detector>
 
 	<!-- BugPattern -->
 	                
@@ -1222,6 +1233,19 @@
 		</Details>
 	</BugPattern>
 	
+	<BugPattern type="ITC_INHERITANCE_TYPE_CHECKING">
+		<ShortDescription>Method uses instanceof on multiple types to arbitrate logic</ShortDescription>
+		<LongDescription>Method {1} uses instanceof on multiple types to arbitrate logic</LongDescription>
+		<Details>
+			<![CDATA[
+				<p>This method uses the instanceof operator in a series of if/else statements to 
+				differentiate blocks of code based on type. If these types are related by inheritance,
+				it is cleaner to just define a method in the base class, and use overridden methods
+				in these classes.</p>
+			]]>
+		</Details>
+	</BugPattern>
+	
 	<!-- BugCode -->
 	
 	<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -1270,4 +1294,5 @@
 	<BugCode abbrev="UEC">Use Enum Collections</BugCode>
 	<BugCode abbrev="SIL">SQL In Loop</BugCode>
 	<BugCode abbrev="NMCS">Needless Member Collection Synchronization</BugCode>
+	<BugCode abbrev="ITC">Inheritance Type Checking</BugCode>
 </MessageCollection>
\ No newline at end of file

Added: trunk/fb-contrib/samples/ITC_Sample.java
===================================================================
--- trunk/fb-contrib/samples/ITC_Sample.java	                        (rev 0)
+++ trunk/fb-contrib/samples/ITC_Sample.java	2006-05-15 03:26:53 UTC (rev 535)
@@ -0,0 +1,19 @@
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Vector;
+
+public class ITC_Sample 
+{
+	public String test(List<String> l)
+	{
+		if (l instanceof ArrayList)
+			return (String)((ArrayList)l).remove(0);
+		else if (l instanceof LinkedList)
+			return (String)((LinkedList) l).removeFirst();
+		else if (l instanceof Vector)
+			return (String)((Vector) l).remove(0);
+		else
+			return null;
+	}
+}

Added: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/InheritanceTypeChecking.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/InheritanceTypeChecking.java	                        (rev 0)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/InheritanceTypeChecking.java	2006-05-15 03:26:53 UTC (rev 535)
@@ -0,0 +1,172 @@
+/*
+ * fb-contrib - Auxilliary detectors for Java programs
+ * Copyright (C) 2005-2006 Dave Brosius
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+package com.mebigfatguy.fbcontrib.detect;
+
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+import org.apache.bcel.classfile.Code;
+
+import com.mebigfatguy.fbcontrib.utils.RegisterUtils;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.BytecodeScanningDetector;
+import edu.umd.cs.findbugs.ba.ClassContext;
+
+/**
+ * looks for if/else blocks where a series of them use instanceof on the same 
+ * variable to determine what do to. If these classes are related by inheritance,
+ * this often is better handled through calling a single overridden method.
+ */
+public class InheritanceTypeChecking extends BytecodeScanningDetector 
+{
+	private BugReporter bugReporter;
+	private Set<IfStatement> ifStatements;
+
+	/**
+     * constructs a ITC detector given the reporter to report bugs on
+     * @param bugReporter the sync of bug reports
+	 */
+	public InheritanceTypeChecking(BugReporter bugReporter) {
+		this.bugReporter = bugReporter;
+	}
+	
+	/**
+	 * implements the visitor to allocate and clear the ifStatements set
+	 * 
+	 * @param classContext the context object of the currently parsed class
+	 */
+	@Override
+	public void visitClassContext(ClassContext classContext) {
+		try {
+			ifStatements = new HashSet<IfStatement>();
+			super.visitClassContext(classContext);
+		} finally {
+			ifStatements = null;
+		}
+	}
+	
+	/**
+	 * implements the visitor to clear the ifStatements set
+	 * 
+	 * @param obj the context object of the currently parsed code block
+	 */
+	@Override
+	public void visitCode(Code obj) {
+		ifStatements.clear();
+		super.visitCode(obj);
+	}
+	
+	/**
+	 * implements the visitor to find if/else code that checks types using
+	 * instanceof, and these types are related by inheritance.
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
+	@Override
+	public void sawOpcode(int seen) {
+		boolean processed = false;
+		Iterator<IfStatement> isi = ifStatements.iterator();
+		while (isi.hasNext()) {
+			int action = isi.next().processOpcode(this, bugReporter, seen);
+			if (action == IfStatement.REMOVE_ACTION)
+				isi.remove();
+			else if (action == IfStatement.PROCESSED_ACTION)
+				processed = true;
+		}
+		
+		if (!processed) {
+			if ((seen == ALOAD)
+			||  ((seen >= ALOAD_0) && (seen <= ALOAD_3))) {
+				IfStatement is = new IfStatement(this, seen);
+				ifStatements.add(is);
+			}
+		}
+	}
+	
+	private static class IfStatement {
+		public static final int NO_ACTION = 0;
+		public static final int REMOVE_ACTION = 1;
+		public static final int PROCESSED_ACTION = 2;
+		
+		private final static int SEEN_ALOAD = 1;
+		private final static int SEEN_INSTANCEOF = 2;
+		private final static int SEEN_IFEQ = 3;
+
+		private int state;
+		private int reg;
+		private int firstPC;
+		private int branchTarget;
+		private int matchCount;
+		private Set<String> instanceOfTypes;
+		
+		public IfStatement(BytecodeScanningDetector bsd, int seen) {
+			state = SEEN_ALOAD;
+			reg = RegisterUtils.getALoadReg(bsd, seen);
+			matchCount = 0;
+			firstPC = bsd.getPC();
+		}
+		
+		public int processOpcode(BytecodeScanningDetector bsd, BugReporter bugReporter, int seen) {
+			switch (state) {
+					case SEEN_ALOAD:
+					if (seen == INSTANCEOF) {
+						if (instanceOfTypes == null)
+							instanceOfTypes = new HashSet<String>();
+						instanceOfTypes.add(bsd.getClassConstantOperand());
+						state = SEEN_INSTANCEOF;
+						return PROCESSED_ACTION;						
+					}
+				break;
+
+				case SEEN_INSTANCEOF:
+					if (seen == IFEQ) {
+						branchTarget = bsd.getBranchTarget();
+						state = SEEN_IFEQ;
+						matchCount++;
+						return PROCESSED_ACTION;
+					}
+				break;
+				
+				case SEEN_IFEQ:
+					if (bsd.getPC() == branchTarget) {
+						if ((seen == ALOAD)
+						||  ((seen >= ALOAD_0) && (seen <= ALOAD_3))) {
+							if (reg == RegisterUtils.getALoadReg(bsd, seen)) {
+								state = SEEN_ALOAD;
+								return PROCESSED_ACTION;
+							}
+						}
+						if (matchCount > 1) {
+							bugReporter.reportBug(new BugInstance(bsd, "ITC_INHERITANCE_TYPE_CHECKING", NORMAL_PRIORITY)
+									   .addClass(bsd)
+									   .addMethod(bsd)
+									   .addSourceLine(bsd, firstPC));
+							return REMOVE_ACTION;
+						}
+					}
+					return NO_ACTION;
+			}
+			
+			return REMOVE_ACTION;
+		}
+	}
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [534] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 534
Author:   dbrosius
Date:     2006-05-14 19:06:52 -0700 (Sun, 14 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=534&view=rev

Log Message:
-----------
javadoc

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-12 04:39:51 UTC (rev 533)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-15 02:06:52 UTC (rev 534)
@@ -106,6 +106,12 @@
 		this.bugReporter = bugReporter;
 	}
 	
+	/**
+	 * implements the visitor to clear the collectionFields and stack
+	 * and to report collections that remain unmodified out of clinit or init
+	 * 
+	 * @param classContext the context object of the currently parsed class
+	 */
 	@Override
 	public void visitClassContext(ClassContext classContext) {
 		try {
@@ -131,6 +137,11 @@
 		}
 	}
 	
+	/**
+	 * implements the visitor to find collection fields
+	 * 
+	 * @param obj the context object of the currently parse field
+	 */
 	@Override
 	public void visitField(Field obj) {
 		if (obj.isPrivate()) {
@@ -149,6 +160,11 @@
 		}
 	}
 	
+	/**
+	 * implements the visitor to set the state based on the type of method being parsed
+	 * 
+	 * @param obj the context object of the currently parsed code block
+	 */
 	@Override
 	public void visitCode(Code obj) {
 		if (collectionFields.size() > 0) {
@@ -165,6 +181,11 @@
 		}
 	}
 	
+	/**
+	 * implements the visitor to call the approriate visitor based on state
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	@Override
 	public void sawOpcode(int seen) {
 		switch (state) {
@@ -182,6 +203,12 @@
 		}
 	}
 	
+	/**
+	 * handle <clinit> blocks by looking for putstatic calls referencing
+	 * synchronized collections
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	private void sawCLInitOpcode(int seen) {
 		boolean isSyncCollection = false;
 		try {
@@ -200,6 +227,12 @@
 		}
 	}
 	
+	/**
+	 * handle <init> blocks by looking for putfield calls referencing
+	 * synchronized collections
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	private void sawInitOpcode(int seen) {
 		boolean isSyncCollection = false;
 		try {
@@ -218,6 +251,12 @@
 		}
 	}
 	
+	/**
+	 * handles regular methods by looking for methods on collections that
+	 * are modifying and removes those collections from the ones under review
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	private void sawMethodOpcode(int seen) {
 		boolean isSyncCollection = false;
 		try {
@@ -290,6 +329,12 @@
 		
 	}
 	
+	/**
+	 * returns whether this instruction is creating a synchronized collection
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 * @return whether a synchronized collection has just been created
+	 */
 	private boolean isSyncCollectionCreation(int seen) {
 		if (seen == INVOKESPECIAL) {
 			if ("<init>".equals(getNameConstantOperand())) {
@@ -303,6 +348,12 @@
 		return false;
 	}
 	
+	/**
+	 * sets the source line annotation of a store to a collection if that collection
+	 * is synchronized.
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	private void processCollectionStore(int seen) {
 		String fieldClassName = getDottedClassConstantOperand();
 		String fieldName = getNameConstantOperand();
@@ -322,6 +373,10 @@
 		}
 	}
 	
+	/**
+	 * holds information about a field, namely the annotation and 
+	 * whether the collection is synchronized.
+	 */
 	static class FieldInfo {
 		private FieldAnnotation fieldAnnotation;
 		private boolean isSynchronized;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [533] trunk/fb-contrib/samples/NMCS_Sample.java

[<dbr...@us...>]

Revision: 533
Author:   dbrosius
Date:     2006-05-11 21:39:51 -0700 (Thu, 11 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=533&view=rev

Log Message:
-----------
more tests

Modified Paths:
--------------
    trunk/fb-contrib/samples/NMCS_Sample.java
Modified: trunk/fb-contrib/samples/NMCS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-12 04:36:39 UTC (rev 532)
+++ trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-12 04:39:51 UTC (rev 533)
@@ -48,9 +48,10 @@
 		return temp;
 	}
 	
-	public List<String> test4(boolean b)
+	public List<String> test4(boolean b1, boolean b2)
 	{
-		return b ? test4 : new Vector<String>();
+		return b1 ? test4 : 
+			b2 ? new Vector<String>() : test4;
 	}
 	
 }


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [532] trunk/fb-contrib/samples

[<dbr...@us...>]

Revision: 532
Author:   dbrosius
Date:     2006-05-11 21:36:39 -0700 (Thu, 11 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=532&view=rev

Log Message:
-----------
fix false positive when a field is 'returned' from the first part of a trinary.

Modified Paths:
--------------
    trunk/fb-contrib/samples/NMCS_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/samples/NMCS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-11 23:32:58 UTC (rev 531)
+++ trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-12 04:36:39 UTC (rev 532)
@@ -18,6 +18,8 @@
 	
 	private Set<String> test3 = new HashSet<String>();
 	
+	private List<String> test4 = new Vector<String>();
+	
 	public String test1()
 	{
 		StringBuffer sb = new StringBuffer();
@@ -46,4 +48,9 @@
 		return temp;
 	}
 	
+	public List<String> test4(boolean b)
+	{
+		return b ? test4 : new Vector<String>();
+	}
+	
 }

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 23:32:58 UTC (rev 531)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-12 04:36:39 UTC (rev 532)
@@ -266,10 +266,18 @@
 					String fieldName = getNameConstantOperand();
 					collectionFields.remove(fieldName);
 				break;
+				
+				case GOTO:
+				case GOTO_W:
+					if (stack.getStackDepth() > 0) {
+						OpcodeStack.Item item = stack.getStackItem(0);
+						FieldAnnotation fa = item.getField();
+						if (fa != null) {
+							collectionFields.remove(fa.getFieldName());
+						}
+					}
+				break;
 			}
-			
-			//look for code that alters the collection, passes it to a method, returns it, etc
-			//watch out for aliases
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [531] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 531
Author:   dbrosius
Date:     2006-05-11 16:32:58 -0700 (Thu, 11 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=531&view=rev

Log Message:
-----------
fix compare of dotted vs. slashed full classnames

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 04:43:40 UTC (rev 530)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 23:32:58 UTC (rev 531)
@@ -296,7 +296,7 @@
 	}
 	
 	private void processCollectionStore(int seen) {
-		String fieldClassName = getClassConstantOperand();
+		String fieldClassName = getDottedClassConstantOperand();
 		String fieldName = getNameConstantOperand();
 		if (fieldClassName.equals(className)) {
 			if (stack.getStackDepth() > 0) {


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [530] trunk/fb-contrib/samples

[<dbr...@us...>]

Revision: 530
Author:   dbrosius
Date:     2006-05-10 21:43:40 -0700 (Wed, 10 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=530&view=rev

Log Message:
-----------
more tests

Modified Paths:
--------------
    trunk/fb-contrib/samples/NMCS_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/samples/NMCS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-11 04:20:36 UTC (rev 529)
+++ trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-11 04:43:40 UTC (rev 530)
@@ -1,6 +1,8 @@
+import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.Vector;
 
 public class NMCS_Sample 
@@ -14,6 +16,8 @@
 	
 	private Map<String, String> test2 = new Hashtable<String, String>();
 	
+	private Set<String> test3 = new HashSet<String>();
+	
 	public String test1()
 	{
 		StringBuffer sb = new StringBuffer();
@@ -35,4 +39,11 @@
 		return test2.get("foo");
 	}
 	
+	public Set<String> test3()
+	{
+		Set<String> temp = test3;
+		temp.add("Foo");
+		return temp;
+	}
+	
 }

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 04:20:36 UTC (rev 529)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 04:43:40 UTC (rev 530)
@@ -178,6 +178,7 @@
 			
 			case IN_METHOD:
 				sawMethodOpcode(seen);
+			break;
 		}
 	}
 	


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [529] trunk/fb-contrib/samples

[<dbr...@us...>]

Revision: 529
Author:   dbrosius
Date:     2006-05-10 21:20:36 -0700 (Wed, 10 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=529&view=rev

Log Message:
-----------
stores to fields in regular methods make then ineligible for reporting on.

Modified Paths:
--------------
    trunk/fb-contrib/samples/NMCS_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/samples/NMCS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-10 04:20:11 UTC (rev 528)
+++ trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-11 04:20:36 UTC (rev 529)
@@ -1,20 +1,24 @@
+import java.util.Hashtable;
 import java.util.List;
+import java.util.Map;
 import java.util.Vector;
 
 public class NMCS_Sample 
 {
-	private static List<String> test = new Vector<String>();
+	private static List<String> test1 = new Vector<String>();
 	static {
-		test.add("one");
-		test.add("two");
-		test.add("three");
+		test1.add("one");
+		test1.add("two");
+		test1.add("three");
 	}
 	
+	private Map<String, String> test2 = new Hashtable<String, String>();
+	
 	public String test1()
 	{
 		StringBuffer sb = new StringBuffer();
 		String comma = "";
-		for (String s : test)
+		for (String s : test1)
 		{
 			sb.append(comma);
 			comma = ",";
@@ -24,4 +28,11 @@
 		return sb.toString();
 	}
 	
+	public String test2()
+	{
+		test2 = new Hashtable<String, String>();
+		
+		return test2.get("foo");
+	}
+	
 }

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 04:20:11 UTC (rev 528)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-11 04:20:36 UTC (rev 529)
@@ -218,8 +218,10 @@
 	}
 	
 	private void sawMethodOpcode(int seen) {
+		boolean isSyncCollection = false;
 		try {
 			stack.mergeJumps(this);
+			isSyncCollection = isSyncCollectionCreation(seen);
 			
 			switch (seen) {
 				case INVOKEVIRTUAL:
@@ -257,12 +259,24 @@
 						}
 					}
 				break;
+				
+				case PUTFIELD:
+				case PUTSTATIC:
+					String fieldName = getNameConstantOperand();
+					collectionFields.remove(fieldName);
+				break;
 			}
 			
 			//look for code that alters the collection, passes it to a method, returns it, etc
 			//watch out for aliases
 		} finally {
 			stack.sawOpcode(this, seen);
+			if (isSyncCollection) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item item = stack.getStackItem(0);
+					item.setUserValue(Boolean.TRUE);
+				}
+			}
 		}
 		
 	}
@@ -287,10 +301,12 @@
 			if (stack.getStackDepth() > 0) {
 				OpcodeStack.Item item = stack.getStackItem(0);
 				if (item.getUserValue() != null) {
-					FieldInfo fi = collectionFields.get(fieldName);
-					if (fi != null) {
-						fi.getFieldAnnotation().setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
-						fi.setSynchronized();
+					if (fieldName != null) {
+						FieldInfo fi = collectionFields.get(fieldName);
+						if (fi != null) {
+							fi.getFieldAnnotation().setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
+							fi.setSynchronized();
+						}
 					}
 				}
 			}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [528] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 528
Author:   dbrosius
Date:     2006-05-09 21:20:11 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=528&view=rev

Log Message:
-----------
relying on getSourceLines being non null to signify a synchronied collection is both brittle and just plain wrong, as getSourceLines generates one, whether or not it was set. So use a first class class to hold this info.

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:24:54 UTC (rev 527)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 04:20:11 UTC (rev 528)
@@ -51,10 +51,18 @@
 			collectionClass = null;
 		}
 	}
+	private static JavaClass mapClass;
+	static {
+		try {
+			mapClass = Repository.lookupClass("java/util/Map");
+		} catch (ClassNotFoundException cnfe) {
+			mapClass = null;
+		}
+	}
 	private static Set<String> syncCollections = new HashSet<String>();
 	static {
-		syncCollections.add("java.util.Vector");
-		syncCollections.add("java.util.Hashtable");
+		syncCollections.add("java/util/Vector");
+		syncCollections.add("java/util/Hashtable");
 	}
 	private static Set<String> modifyingMethods = new HashSet<String>();
 	static {
@@ -84,7 +92,7 @@
 	private static final int IN_INIT = 2;
 	
 	private BugReporter bugReporter;
-	private Map<String, FieldAnnotation> collectionFields;
+	private Map<String, FieldInfo> collectionFields;
 	private Map<Integer, String> aliases;
 	private OpcodeStack stack;
 	private int state;
@@ -101,18 +109,18 @@
 	@Override
 	public void visitClassContext(ClassContext classContext) {
 		try {
-			if (collectionClass != null) {
-				collectionFields = new HashMap<String, FieldAnnotation>();
+			if ((collectionClass != null) && (mapClass != null)) {
+				collectionFields = new HashMap<String, FieldInfo>();
 				aliases = new HashMap<Integer, String>();
 				stack = new OpcodeStack();
 				JavaClass cls = classContext.getJavaClass();
 				className = cls.getClassName();
 				super.visitClassContext(classContext);
-				for (FieldAnnotation fa : collectionFields.values()) {
-					if (fa.getSourceLines() != null) {
+				for (FieldInfo fi : collectionFields.values()) {
+					if (fi.isSynchronized()) {
 						bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
 									.addClass(this)
-									.addField(fa));
+									.addField(fi.getFieldAnnotation()));
 					}
 				}
 			}
@@ -130,9 +138,9 @@
 			if (signature.charAt(0) == 'L') {
 				try {
 					JavaClass cls = Repository.lookupClass(signature.substring(1, signature.length() - 1));
-					if (cls.implementationOf(collectionClass)) {
+					if (cls.implementationOf(collectionClass) || cls.implementationOf(mapClass)) {
 						FieldAnnotation fa = FieldAnnotation.fromVisitedField(this);
-						collectionFields.put(fa.getFieldName(), fa);
+						collectionFields.put(fa.getFieldName(), new FieldInfo(fa));
 					}
 				} catch (ClassNotFoundException cnfe) {
 					bugReporter.reportMissingClass(cnfe);
@@ -178,7 +186,8 @@
 		try {
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
-			processCollectionStore(PUTSTATIC, seen);
+			if (seen == PUTSTATIC)
+				processCollectionStore(seen);
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -195,7 +204,8 @@
 		try {
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
-			processCollectionStore(PUTFIELD, seen);
+			if (seen == PUTFIELD)
+				processCollectionStore(seen);
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -270,21 +280,42 @@
 		return false;
 	}
 	
-	private void processCollectionStore(int fieldOp, int seen) {
-		if (seen == fieldOp) {
-			String fieldClassName = getClassConstantOperand();
-			String fieldName = getNameConstantOperand();
-			if (fieldClassName.equals(className)) {
-				if (stack.getStackDepth() > 0) {
-					OpcodeStack.Item item = stack.getStackItem(0);
-					if (item.getUserValue() != null) {
-						FieldAnnotation fa = collectionFields.get(fieldName);
-						if (fa != null) {
-							fa.setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
-						}
+	private void processCollectionStore(int seen) {
+		String fieldClassName = getClassConstantOperand();
+		String fieldName = getNameConstantOperand();
+		if (fieldClassName.equals(className)) {
+			if (stack.getStackDepth() > 0) {
+				OpcodeStack.Item item = stack.getStackItem(0);
+				if (item.getUserValue() != null) {
+					FieldInfo fi = collectionFields.get(fieldName);
+					if (fi != null) {
+						fi.getFieldAnnotation().setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
+						fi.setSynchronized();
 					}
 				}
 			}
 		}
 	}
+	
+	static class FieldInfo {
+		private FieldAnnotation fieldAnnotation;
+		private boolean isSynchronized;
+		
+		public FieldInfo(FieldAnnotation fa) {
+			fieldAnnotation = fa;
+			isSynchronized = false;
+		}
+		
+		public void setSynchronized() {
+			isSynchronized = true;
+		}
+		
+		public FieldAnnotation getFieldAnnotation() {
+			return fieldAnnotation;
+		}
+		
+		public boolean isSynchronized() {
+			return isSynchronized;
+		}
+	}
 }


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [527] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 527
Author:   dbrosius
Date:     2006-05-09 20:24:54 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=527&view=rev

Log Message:
-----------
no need to differentiate static from instance collections

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:12:34 UTC (rev 526)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:24:54 UTC (rev 527)
@@ -53,8 +53,8 @@
 	}
 	private static Set<String> syncCollections = new HashSet<String>();
 	static {
-		syncCollections.add("java/util/Vector");
-		syncCollections.add("java/util/Hashtable");
+		syncCollections.add("java.util.Vector");
+		syncCollections.add("java.util.Hashtable");
 	}
 	private static Set<String> modifyingMethods = new HashSet<String>();
 	static {
@@ -84,10 +84,8 @@
 	private static final int IN_INIT = 2;
 	
 	private BugReporter bugReporter;
-	private Map<String, FieldAnnotation> staticCollectionFields;
-	private Map<String, FieldAnnotation> instanceCollectionFields;
-	private Map<Integer, String> staticAliases;
-	private Map<Integer, String> instanceAliases;
+	private Map<String, FieldAnnotation> collectionFields;
+	private Map<Integer, String> aliases;
 	private OpcodeStack stack;
 	private int state;
 	private String className;
@@ -104,35 +102,23 @@
 	public void visitClassContext(ClassContext classContext) {
 		try {
 			if (collectionClass != null) {
-				staticCollectionFields = new HashMap<String, FieldAnnotation>();
-				instanceCollectionFields = new HashMap<String, FieldAnnotation>();
-				staticAliases = new HashMap<Integer, String>();
-				instanceAliases = new HashMap<Integer, String>();
+				collectionFields = new HashMap<String, FieldAnnotation>();
+				aliases = new HashMap<Integer, String>();
 				stack = new OpcodeStack();
 				JavaClass cls = classContext.getJavaClass();
 				className = cls.getClassName();
 				super.visitClassContext(classContext);
-				for (FieldAnnotation fa : staticCollectionFields.values()) {
+				for (FieldAnnotation fa : collectionFields.values()) {
 					if (fa.getSourceLines() != null) {
 						bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
 									.addClass(this)
 									.addField(fa));
 					}
 				}
-				for (FieldAnnotation fa : instanceCollectionFields.values()) {
-					if (fa.getSourceLines() != null) {
-						bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
-									.addClass(this)
-									.addField(fa));
-					}
-				}
-					
 			}
 		} finally {
-			staticCollectionFields = null;
-			instanceCollectionFields = null;
-			staticAliases = null;
-			instanceAliases = null;
+			collectionFields = null;
+			aliases = null;
 			stack = null;
 		}
 	}
@@ -146,10 +132,7 @@
 					JavaClass cls = Repository.lookupClass(signature.substring(1, signature.length() - 1));
 					if (cls.implementationOf(collectionClass)) {
 						FieldAnnotation fa = FieldAnnotation.fromVisitedField(this);
-						if (obj.isStatic())
-							staticCollectionFields.put(fa.getFieldName(), fa);
-						else
-							instanceCollectionFields.put(fa.getFieldName(), fa);
+						collectionFields.put(fa.getFieldName(), fa);
 					}
 				} catch (ClassNotFoundException cnfe) {
 					bugReporter.reportMissingClass(cnfe);
@@ -160,9 +143,8 @@
 	
 	@Override
 	public void visitCode(Code obj) {
-		if ((staticCollectionFields.size() + instanceCollectionFields.size()) > 0) {
-			staticAliases.clear();
-			instanceAliases.clear();
+		if (collectionFields.size() > 0) {
+			aliases.clear();
 			String methodName = getMethodName();
 			if ("<clinit>".equals(methodName))
 				state = IN_CLINIT;
@@ -240,24 +222,15 @@
 							OpcodeStack.Item item = stack.getStackItem(parmCount);
 							FieldAnnotation fa = item.getField();
 							if (fa != null) {
-								if (fa.isStatic())
-									staticCollectionFields.remove(fa.getFieldName());
-								else
-									instanceCollectionFields.remove(fa.getFieldName());
+								collectionFields.remove(fa.getFieldName());
 							} else {
 								int reg = item.getRegisterNumber();
 								if (reg >= 0) {
 									Integer register = new Integer(reg);
-									String fName = staticAliases.get(register);
+									String fName = aliases.get(register);
 									if (fName != null) {
-										staticCollectionFields.remove(fName);
-										staticAliases.remove(register);
-									} else {
-										fName = instanceAliases.get(register);
-										if (fName != null) {
-											instanceCollectionFields.remove(fName);
-											instanceAliases.remove(register);
-										}
+										collectionFields.remove(fName);
+										aliases.remove(register);
 									}
 								}
 							}
@@ -270,10 +243,7 @@
 						OpcodeStack.Item item = stack.getStackItem(0);
 						FieldAnnotation fa = item.getField();
 						if (fa != null) {
-							if (fa.isStatic())
-								staticCollectionFields.remove(fa.getFieldName());
-							else
-								instanceCollectionFields.remove(fa.getFieldName());
+							collectionFields.remove(fa.getFieldName());
 						}
 					}
 				break;
@@ -308,13 +278,7 @@
 				if (stack.getStackDepth() > 0) {
 					OpcodeStack.Item item = stack.getStackItem(0);
 					if (item.getUserValue() != null) {
-						Map<String, FieldAnnotation> fields;
-						if (fieldOp == PUTFIELD)
-							fields = instanceCollectionFields;
-						else 
-							fields = staticCollectionFields;
-						
-						FieldAnnotation fa = fields.get(fieldName);
+						FieldAnnotation fa = collectionFields.get(fieldName);
 						if (fa != null) {
 							fa.setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
 						}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [526] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 526
Author:   dbrosius
Date:     2006-05-09 20:12:34 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=526&view=rev

Log Message:
-----------
don't report fields that don't have sourcelines. These collections haven't been checked to see that they are actually synchronized.

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:07:18 UTC (rev 525)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:12:34 UTC (rev 526)
@@ -113,14 +113,18 @@
 				className = cls.getClassName();
 				super.visitClassContext(classContext);
 				for (FieldAnnotation fa : staticCollectionFields.values()) {
-					bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
-								.addClass(this)
-								.addField(fa));
+					if (fa.getSourceLines() != null) {
+						bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
+									.addClass(this)
+									.addField(fa));
+					}
 				}
 				for (FieldAnnotation fa : instanceCollectionFields.values()) {
-					bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
-								.addClass(this)
-								.addField(fa));
+					if (fa.getSourceLines() != null) {
+						bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
+									.addClass(this)
+									.addField(fa));
+					}
 				}
 					
 			}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [525] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 525
Author:   dbrosius
Date:     2006-05-09 20:07:18 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=525&view=rev

Log Message:
-----------
start reporting bugs... it is now way to liberal and reports many false positives. No aliases checking yet.

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:41:02 UTC (rev 524)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-10 03:07:18 UTC (rev 525)
@@ -27,7 +27,9 @@
 import org.apache.bcel.classfile.Code;
 import org.apache.bcel.classfile.Field;
 import org.apache.bcel.classfile.JavaClass;
+import org.apache.bcel.generic.Type;
 
+import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.BytecodeScanningDetector;
 import edu.umd.cs.findbugs.FieldAnnotation;
@@ -54,6 +56,29 @@
 		syncCollections.add("java/util/Vector");
 		syncCollections.add("java/util/Hashtable");
 	}
+	private static Set<String> modifyingMethods = new HashSet<String>();
+	static {
+		modifyingMethods.add("add");
+		modifyingMethods.add("addAll");
+		modifyingMethods.add("addFirst");
+		modifyingMethods.add("addElement");
+		modifyingMethods.add("addLast");
+		modifyingMethods.add("clear");
+		modifyingMethods.add("insertElementAt");
+		modifyingMethods.add("put");
+		modifyingMethods.add("remove");
+		modifyingMethods.add("removeAll");
+		modifyingMethods.add("removeAllElements");
+		modifyingMethods.add("removeElement");
+		modifyingMethods.add("removeElementAt");
+		modifyingMethods.add("removeFirst");
+		modifyingMethods.add("removeLast");
+		modifyingMethods.add("removeRange");
+		modifyingMethods.add("retainAll");
+		modifyingMethods.add("set");
+		modifyingMethods.add("setElementAt");
+		modifyingMethods.add("setSize");
+	}
 	private static final int IN_METHOD = 0;
 	private static final int IN_CLINIT = 1;
 	private static final int IN_INIT = 2;
@@ -87,6 +112,17 @@
 				JavaClass cls = classContext.getJavaClass();
 				className = cls.getClassName();
 				super.visitClassContext(classContext);
+				for (FieldAnnotation fa : staticCollectionFields.values()) {
+					bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
+								.addClass(this)
+								.addField(fa));
+				}
+				for (FieldAnnotation fa : instanceCollectionFields.values()) {
+					bugReporter.reportBug(new BugInstance(this, "NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION", NORMAL_PRIORITY)
+								.addClass(this)
+								.addField(fa));
+				}
+					
 			}
 		} finally {
 			staticCollectionFields = null;
@@ -189,6 +225,56 @@
 		try {
 			stack.mergeJumps(this);
 			
+			switch (seen) {
+				case INVOKEVIRTUAL:
+				case INVOKEINTERFACE:
+					String methodName = getNameConstantOperand();
+					if (modifyingMethods.contains(methodName)) {
+						String signature = getSigConstantOperand();
+						int parmCount = Type.getArgumentTypes(signature).length;
+						if (stack.getStackDepth() > parmCount) {
+							OpcodeStack.Item item = stack.getStackItem(parmCount);
+							FieldAnnotation fa = item.getField();
+							if (fa != null) {
+								if (fa.isStatic())
+									staticCollectionFields.remove(fa.getFieldName());
+								else
+									instanceCollectionFields.remove(fa.getFieldName());
+							} else {
+								int reg = item.getRegisterNumber();
+								if (reg >= 0) {
+									Integer register = new Integer(reg);
+									String fName = staticAliases.get(register);
+									if (fName != null) {
+										staticCollectionFields.remove(fName);
+										staticAliases.remove(register);
+									} else {
+										fName = instanceAliases.get(register);
+										if (fName != null) {
+											instanceCollectionFields.remove(fName);
+											instanceAliases.remove(register);
+										}
+									}
+								}
+							}
+						}
+					}
+				break;
+				
+				case ARETURN:
+					if (stack.getStackDepth() > 0) {
+						OpcodeStack.Item item = stack.getStackItem(0);
+						FieldAnnotation fa = item.getField();
+						if (fa != null) {
+							if (fa.isStatic())
+								staticCollectionFields.remove(fa.getFieldName());
+							else
+								instanceCollectionFields.remove(fa.getFieldName());
+						}
+					}
+				break;
+			}
+			
 			//look for code that alters the collection, passes it to a method, returns it, etc
 			//watch out for aliases
 		} finally {


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [524] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 524
Author:   dbrosius
Date:     2006-05-09 00:41:02 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=524&view=rev

Log Message:
-----------
a lil more... far from done

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:25:51 UTC (rev 523)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:41:02 UTC (rev 524)
@@ -156,9 +156,7 @@
 		try {
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
-			if (seen == PUTFIELD) {
-				processCollectionStore(PUTSTATIC, seen);
-			}
+			processCollectionStore(PUTSTATIC, seen);
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -176,7 +174,6 @@
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
 			processCollectionStore(PUTFIELD, seen);
-			
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -191,9 +188,9 @@
 	private void sawMethodOpcode(int seen) {
 		try {
 			stack.mergeJumps(this);
-			if (isSyncCollectionCreation(seen)) {
-				
-			}
+			
+			//look for code that alters the collection, passes it to a method, returns it, etc
+			//watch out for aliases
 		} finally {
 			stack.sawOpcode(this, seen);
 		}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [523] trunk/fb-contrib/etc

[<dbr...@us...>]

Revision: 523
Author:   dbrosius
Date:     2006-05-09 00:25:51 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=523&view=rev

Log Message:
-----------
add xml infrastructure for NMCS

Modified Paths:
--------------
    trunk/fb-contrib/etc/findbugs.xml
    trunk/fb-contrib/etc/messages.xml
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml	2006-05-09 07:25:19 UTC (rev 522)
+++ trunk/fb-contrib/etc/findbugs.xml	2006-05-09 07:25:51 UTC (rev 523)
@@ -195,6 +195,10 @@
 			speed="fast"
 			reports="SIL_SQL_IN_LOOP" />
 			
+	<Detector class="com.mebigfatguy.fbcontrib.detect.NeedlessMemberCollectionSynchronization"
+			speed="moderate"
+			reports="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION" />
+			
    	<!-- BugPattern -->
 	
 	<BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" />
@@ -246,4 +250,5 @@
 	<BugPattern abbrev="S508C" type="S508C_SET_COMP_COLOR" category="CORRECTNESS" />
 	<BugPattern abbrev="UEC" type="UEC_USE_ENUM_COLLECTIONS" category="PERFORMANCE" />
 	<BugPattern abbrev="SIL" type="SIL_SQL_IN_LOOP" category="PERFORMANCE" experimental="true" />
+	<BugPattern abbrev="NMCS" type="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION" category="PERFORMANCE" experimental="true" />
 </FindbugsPlugin>
\ No newline at end of file

Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml	2006-05-09 07:25:19 UTC (rev 522)
+++ trunk/fb-contrib/etc/messages.xml	2006-05-09 07:25:51 UTC (rev 523)
@@ -557,6 +557,17 @@
 			]]>
 		</Details>
 	</Detector>
+	
+	<Detector class="com.mebigfatguy.fbcontrib.detect.NeedlessMemberCollectionSynchronization">
+		<Details>
+			<![CDATA[
+			<p>looks for classes that define private synchronized collections as static or instance 
+			members, that are only altered in a static initializer or constructor. Since the multithreaded
+			use of this collection is read-only, the use of synchronization is unnecessary.</p>
+			<p>It is a moderately fast detector</p>
+			]]>
+		</Details>
+	</Detector>
 
 	<!-- BugPattern -->
 	                
@@ -1193,11 +1204,24 @@
 				as the number of queries may mushroom in fencepost cases. It is probably more performant
 				to loop over the input and collect the key data needed for the query for all items, and
 				issue one query using an in clause, or similar construct, and then loop over this result
-				set, and fetch all the data at once.
+				set, and fetch all the data at once.</p>
 			]]>
 		</Details>
 	</BugPattern>
 	
+	<BugPattern type="NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION">
+		<ShortDescription>Class defines unneeded synchronization on member collection</ShortDescription>
+		<LongDescription>Class {0} defines unneeded synchronization on member collection</LongDescription>
+		<Details>
+			<![CDATA[
+				<p>This class defines a private collection member as synchronized. It appears however
+				that this collection isn't only modified in a static initializer, or constructor. As these
+				two areas are guaranteed to be thread safe, defining this collection as synchronized is
+				unnecessary and a potential performance bottleneck.</p>
+			]]>
+		</Details>
+	</BugPattern>
+	
 	<!-- BugCode -->
 	
 	<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -1245,4 +1269,5 @@
 	<BugCode abbrev="S508C">Section 508 Compliance Violations</BugCode>
 	<BugCode abbrev="UEC">Use Enum Collections</BugCode>
 	<BugCode abbrev="SIL">SQL In Loop</BugCode>
+	<BugCode abbrev="NMCS">Needless Member Collection Synchronization</BugCode>
 </MessageCollection>
\ No newline at end of file

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:25:19 UTC (rev 522)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:25:51 UTC (rev 523)
@@ -103,7 +103,7 @@
 			String signature = obj.getSignature();
 			if (signature.charAt(0) == 'L') {
 				try {
-					JavaClass cls = Repository.lookupClass(signature);
+					JavaClass cls = Repository.lookupClass(signature.substring(1, signature.length() - 1));
 					if (cls.implementationOf(collectionClass)) {
 						FieldAnnotation fa = FieldAnnotation.fromVisitedField(this);
 						if (obj.isStatic())


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [522] trunk/fb-contrib/samples/NMCS_Sample.java

[<dbr...@us...>]

Revision: 522
Author:   dbrosius
Date:     2006-05-09 00:25:19 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=522&view=rev

Log Message:
-----------
test case for NMCS

Added Paths:
-----------
    trunk/fb-contrib/samples/NMCS_Sample.java
Added: trunk/fb-contrib/samples/NMCS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/NMCS_Sample.java	                        (rev 0)
+++ trunk/fb-contrib/samples/NMCS_Sample.java	2006-05-09 07:25:19 UTC (rev 522)
@@ -0,0 +1,27 @@
+import java.util.List;
+import java.util.Vector;
+
+public class NMCS_Sample 
+{
+	private static List<String> test = new Vector<String>();
+	static {
+		test.add("one");
+		test.add("two");
+		test.add("three");
+	}
+	
+	public String test1()
+	{
+		StringBuffer sb = new StringBuffer();
+		String comma = "";
+		for (String s : test)
+		{
+			sb.append(comma);
+			comma = ",";
+			sb.append(s);
+		}
+		
+		return sb.toString();
+	}
+	
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [521] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 521
Author:   dbrosius
Date:     2006-05-09 00:08:59 -0700 (Tue, 09 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=521&view=rev

Log Message:
-----------
collect srcline annotations for creations of sync'ed collections

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 06:48:21 UTC (rev 520)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 07:08:59 UTC (rev 521)
@@ -32,6 +32,7 @@
 import edu.umd.cs.findbugs.BytecodeScanningDetector;
 import edu.umd.cs.findbugs.FieldAnnotation;
 import edu.umd.cs.findbugs.OpcodeStack;
+import edu.umd.cs.findbugs.SourceLineAnnotation;
 import edu.umd.cs.findbugs.ba.ClassContext;
 
 /**
@@ -64,6 +65,7 @@
 	private Map<Integer, String> instanceAliases;
 	private OpcodeStack stack;
 	private int state;
+	private String className;
 	
 	/**
      * constructs a NMCS detector given the reporter to report bugs on
@@ -82,6 +84,8 @@
 				staticAliases = new HashMap<Integer, String>();
 				instanceAliases = new HashMap<Integer, String>();
 				stack = new OpcodeStack();
+				JavaClass cls = classContext.getJavaClass();
+				className = cls.getClassName();
 				super.visitClassContext(classContext);
 			}
 		} finally {
@@ -152,6 +156,9 @@
 		try {
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
+			if (seen == PUTFIELD) {
+				processCollectionStore(PUTSTATIC, seen);
+			}
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -168,6 +175,8 @@
 		try {
 			stack.mergeJumps(this);
 			isSyncCollection = isSyncCollectionCreation(seen);
+			processCollectionStore(PUTFIELD, seen);
+			
 		} finally {
 			stack.sawOpcode(this, seen);
 			if (isSyncCollection) {
@@ -203,4 +212,28 @@
 		}
 		return false;
 	}
+	
+	private void processCollectionStore(int fieldOp, int seen) {
+		if (seen == fieldOp) {
+			String fieldClassName = getClassConstantOperand();
+			String fieldName = getNameConstantOperand();
+			if (fieldClassName.equals(className)) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item item = stack.getStackItem(0);
+					if (item.getUserValue() != null) {
+						Map<String, FieldAnnotation> fields;
+						if (fieldOp == PUTFIELD)
+							fields = instanceCollectionFields;
+						else 
+							fields = staticCollectionFields;
+						
+						FieldAnnotation fa = fields.get(fieldName);
+						if (fa != null) {
+							fa.setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this));
+						}
+					}
+				}
+			}
+		}
+	}
 }


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [520] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java

[<dbr...@us...>]

Revision: 520
Author:   dbrosius
Date:     2006-05-08 23:48:21 -0700 (Mon, 08 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=520&view=rev

Log Message:
-----------
Initial checkin - NMCS - far from working

Added Paths:
-----------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
Added: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	                        (rev 0)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java	2006-05-09 06:48:21 UTC (rev 520)
@@ -0,0 +1,206 @@
+/*
+ * fb-contrib - Auxilliary detectors for Java programs
+ * Copyright (C) 2005-2006 Dave Brosius
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+package com.mebigfatguy.fbcontrib.detect;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.bcel.Repository;
+import org.apache.bcel.classfile.Code;
+import org.apache.bcel.classfile.Field;
+import org.apache.bcel.classfile.JavaClass;
+
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.BytecodeScanningDetector;
+import edu.umd.cs.findbugs.FieldAnnotation;
+import edu.umd.cs.findbugs.OpcodeStack;
+import edu.umd.cs.findbugs.ba.ClassContext;
+
+/**
+ * looks for private collection members, either static or instance, that are only initialized in
+ * the clinit or init, but are synchronized. This is not necessary as the constructor or static 
+ * initializer are guaranteed to be thread safe.
+ */
+public class NeedlessMemberCollectionSynchronization extends BytecodeScanningDetector {
+	private static JavaClass collectionClass;
+	static {
+		try {
+			collectionClass = Repository.lookupClass("java/util/Collection");
+		} catch (ClassNotFoundException cnfe) {
+			collectionClass = null;
+		}
+	}
+	private static Set<String> syncCollections = new HashSet<String>();
+	static {
+		syncCollections.add("java/util/Vector");
+		syncCollections.add("java/util/Hashtable");
+	}
+	private static final int IN_METHOD = 0;
+	private static final int IN_CLINIT = 1;
+	private static final int IN_INIT = 2;
+	
+	private BugReporter bugReporter;
+	private Map<String, FieldAnnotation> staticCollectionFields;
+	private Map<String, FieldAnnotation> instanceCollectionFields;
+	private Map<Integer, String> staticAliases;
+	private Map<Integer, String> instanceAliases;
+	private OpcodeStack stack;
+	private int state;
+	
+	/**
+     * constructs a NMCS detector given the reporter to report bugs on
+     * @param bugReporter the sync of bug reports
+	 */	
+	public NeedlessMemberCollectionSynchronization(BugReporter bugReporter) {
+		this.bugReporter = bugReporter;
+	}
+	
+	@Override
+	public void visitClassContext(ClassContext classContext) {
+		try {
+			if (collectionClass != null) {
+				staticCollectionFields = new HashMap<String, FieldAnnotation>();
+				instanceCollectionFields = new HashMap<String, FieldAnnotation>();
+				staticAliases = new HashMap<Integer, String>();
+				instanceAliases = new HashMap<Integer, String>();
+				stack = new OpcodeStack();
+				super.visitClassContext(classContext);
+			}
+		} finally {
+			staticCollectionFields = null;
+			instanceCollectionFields = null;
+			staticAliases = null;
+			instanceAliases = null;
+			stack = null;
+		}
+	}
+	
+	@Override
+	public void visitField(Field obj) {
+		if (obj.isPrivate()) {
+			String signature = obj.getSignature();
+			if (signature.charAt(0) == 'L') {
+				try {
+					JavaClass cls = Repository.lookupClass(signature);
+					if (cls.implementationOf(collectionClass)) {
+						FieldAnnotation fa = FieldAnnotation.fromVisitedField(this);
+						if (obj.isStatic())
+							staticCollectionFields.put(fa.getFieldName(), fa);
+						else
+							instanceCollectionFields.put(fa.getFieldName(), fa);
+					}
+				} catch (ClassNotFoundException cnfe) {
+					bugReporter.reportMissingClass(cnfe);
+				}
+			}
+		}
+	}
+	
+	@Override
+	public void visitCode(Code obj) {
+		if ((staticCollectionFields.size() + instanceCollectionFields.size()) > 0) {
+			staticAliases.clear();
+			instanceAliases.clear();
+			String methodName = getMethodName();
+			if ("<clinit>".equals(methodName))
+				state = IN_CLINIT;
+			else if ("<init>".equals(methodName))
+				state = IN_INIT;
+			else
+				state = IN_METHOD;
+			stack.resetForMethodEntry(this);
+			super.visitCode(obj);
+		}
+	}
+	
+	@Override
+	public void sawOpcode(int seen) {
+		switch (state) {
+			case IN_CLINIT:
+				sawCLInitOpcode(seen);
+			break;
+			
+			case IN_INIT:
+				sawInitOpcode(seen);
+			break;
+			
+			case IN_METHOD:
+				sawMethodOpcode(seen);
+		}
+	}
+	
+	private void sawCLInitOpcode(int seen) {
+		boolean isSyncCollection = false;
+		try {
+			stack.mergeJumps(this);
+			isSyncCollection = isSyncCollectionCreation(seen);
+		} finally {
+			stack.sawOpcode(this, seen);
+			if (isSyncCollection) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item item = stack.getStackItem(0);
+					item.setUserValue(Boolean.TRUE);
+				}
+			}
+		}
+	}
+	
+	private void sawInitOpcode(int seen) {
+		boolean isSyncCollection = false;
+		try {
+			stack.mergeJumps(this);
+			isSyncCollection = isSyncCollectionCreation(seen);
+		} finally {
+			stack.sawOpcode(this, seen);
+			if (isSyncCollection) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item item = stack.getStackItem(0);
+					item.setUserValue(Boolean.TRUE);
+				}
+			}
+		}
+	}
+	
+	private void sawMethodOpcode(int seen) {
+		try {
+			stack.mergeJumps(this);
+			if (isSyncCollectionCreation(seen)) {
+				
+			}
+		} finally {
+			stack.sawOpcode(this, seen);
+		}
+		
+	}
+	
+	private boolean isSyncCollectionCreation(int seen) {
+		if (seen == INVOKESPECIAL) {
+			if ("<init>".equals(getNameConstantOperand())) {
+				return (syncCollections.contains(getClassConstantOperand()));
+			}
+		} else if (seen == INVOKESTATIC) {
+			String methodName = getNameConstantOperand();
+			return "java/util/Collections".equals(getClassConstantOperand())
+			    &&  ("synchronizedMap".equals(methodName) || "synchronizedSet".equals(methodName));
+		}
+		return false;
+	}
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [519] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/Section508Compliance.java

[<dbr...@us...>]

Revision: 519
Author:   dbrosius
Date:     2006-05-04 22:56:44 -0700 (Thu, 04 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=519&view=rev

Log Message:
-----------
copyright

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/Section508Compliance.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/Section508Compliance.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/Section508Compliance.java	2006-05-05 05:56:04 UTC (rev 518)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/Section508Compliance.java	2006-05-05 05:56:44 UTC (rev 519)
@@ -1,3 +1,21 @@
+/*
+ * fb-contrib - Auxilliary detectors for Java programs
+ * Copyright (C) 2005-2006 Dave Brosius
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 package com.mebigfatguy.fbcontrib.detect;
 
 import java.util.HashMap;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [518] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/UseEnumCollections.java

[<dbr...@us...>]

Revision: 518
Author:   dbrosius
Date:     2006-05-04 22:56:04 -0700 (Thu, 04 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=518&view=rev

Log Message:
-----------
copyright

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/UseEnumCollections.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/UseEnumCollections.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/UseEnumCollections.java	2006-05-05 05:55:31 UTC (rev 517)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/UseEnumCollections.java	2006-05-05 05:56:04 UTC (rev 518)
@@ -1,3 +1,21 @@
+/*
+ * fb-contrib - Auxilliary detectors for Java programs
+ * Copyright (C) 2005-2006 Dave Brosius
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 package com.mebigfatguy.fbcontrib.detect;
 
 import java.util.HashSet;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [517] trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java

[<dbr...@us...>]

Revision: 517
Author:   dbrosius
Date:     2006-05-04 22:55:31 -0700 (Thu, 04 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=517&view=rev

Log Message:
-----------
javadoc

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java	2006-05-05 05:51:49 UTC (rev 516)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java	2006-05-05 05:55:31 UTC (rev 517)
@@ -1,3 +1,21 @@
+/*
+ * fb-contrib - Auxilliary detectors for Java programs
+ * Copyright (C) 2005-2006 Dave Brosius
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 package com.mebigfatguy.fbcontrib.detect;
 
 import java.util.ArrayList;
@@ -12,6 +30,11 @@
 import edu.umd.cs.findbugs.BytecodeScanningDetector;
 import edu.umd.cs.findbugs.ba.ClassContext;
 
+/**
+ * looks for the execution of sql queries inside a loop. This pattern tends to be inefficient,
+ * and often can be improved upon, by collecting all the keys needed for the query and issuing just
+ * one query using an in clause with all the keys for all the queries previously needed in the loop.
+ */
 public class SQLInLoop extends BytecodeScanningDetector 
 {
 	private static final Set<String> queryClasses = new HashSet<String>();
@@ -30,10 +53,19 @@
 	List<Integer> queryLocations;
 	List<LoopLocation> loops;
 	
+	/**
+     * constructs a SIL detector given the reporter to report bugs on
+     * @param bugReporter the sync of bug reports
+	 */
 	public SQLInLoop(BugReporter bugReporter) {
 		this.bugReporter = bugReporter;
 	}
 	
+	/**
+	 * implements the visitor to create and clear the query locations and loops collections
+	 * 
+	 * @param classContext the context object for the currently parsed java class
+	 */
 	@Override
 	public void visitClassContext(ClassContext classContext) {
 		try {
@@ -46,6 +78,11 @@
 		}
 	}
 	
+	/**
+	 * implements the visitor to clear the collections, and report the query locations that are in loops
+	 * 
+	 * @param obj the context object for the currently parsed code block
+	 */
 	@Override
 	public void visitCode(Code obj) {
 		queryLocations.clear();
@@ -64,6 +101,11 @@
 		}
 	}
 	
+	/**
+	 * implements the visitor to collect positions of queries and loops
+	 * 
+	 * @param seen the opcode of the currently parsed instruction
+	 */
 	@Override
 	public void sawOpcode(int seen) {
 		if (seen == INVOKEINTERFACE) {
@@ -81,6 +123,9 @@
 		}
 	}
 
+	/**
+	 * holds the start and end position of a loop
+	 */
 	private static class LoopLocation {
 		private int startPC;
 		private int endPC;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

[Fb-contrib-commit] SF.net SVN: fb-contrib: [516] trunk/fb-contrib/etc

[<dbr...@us...>]

Revision: 516
Author:   dbrosius
Date:     2006-05-04 22:51:49 -0700 (Thu, 04 May 2006)
ViewCVS:  http://svn.sourceforge.net/fb-contrib/?rev=516&view=rev

Log Message:
-----------
Initial checkin: SIL detector

Modified Paths:
--------------
    trunk/fb-contrib/etc/findbugs.xml
    trunk/fb-contrib/etc/messages.xml

Added Paths:
-----------
    trunk/fb-contrib/samples/SIL_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml	2006-05-02 02:24:14 UTC (rev 515)
+++ trunk/fb-contrib/etc/findbugs.xml	2006-05-05 05:51:49 UTC (rev 516)
@@ -191,6 +191,10 @@
 			speed="fast"
 			reports="UEC_USE_ENUM_COLLECTIONS" />
 			
+	<Detector class="com.mebigfatguy.fbcontrib.detect.SQLInLoop"
+			speed="fast"
+			reports="SIL_SQL_IN_LOOP" />
+			
    	<!-- BugPattern -->
 	
 	<BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" />
@@ -232,13 +236,14 @@
 	<BugPattern abbrev="JVR" type="JVR_JDBC_VENDOR_RELIANCE" category="CORRECTNESS" />
 	<BugPattern abbrev="PMB" type="PMB_POSSIBLE_MEMORY_BLOAT" category="CORRECTNESS" />
 	<BugPattern abbrev="LSYC" type="LSYC_LOCAL_SYNCHRONIZED_COLLECTION" category="CORRECTNESS" />
-	<BugPattern abbrev="FCBL" type="FCBL_FIELD_COULD_BE_LOCAL" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="NOS" type="NOS_NON_OWNED_SYNCHRONIZATION" category="STYLE" experimental="true" />
-	<BugPattern abbrev="NRTL" type="NRTL_NON_RECYCLEABLE_TAG_LIB" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="S508C" type="S508C_NULL_LAYOUT" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="S508C" type="S508C_NO_SETLABELFOR" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="S508C" type="S508C_NO_SETSIZE" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="S508C" type="S508C_NON_ACCESSIBLE_JCOMPONENT" category="CORRECTNESS" experimental="true"/>
-	<BugPattern abbrev="S508C" type="S508C_SET_COMP_COLOR" category="CORRECTNESS" experimental="true" />
-	<BugPattern abbrev="UEC" type="UEC_USE_ENUM_COLLECTIONS" category="PERFORMANCE" experimental="true" />
+	<BugPattern abbrev="FCBL" type="FCBL_FIELD_COULD_BE_LOCAL" category="CORRECTNESS" />
+	<BugPattern abbrev="NOS" type="NOS_NON_OWNED_SYNCHRONIZATION" category="STYLE" />
+	<BugPattern abbrev="NRTL" type="NRTL_NON_RECYCLEABLE_TAG_LIB" category="CORRECTNESS" />
+	<BugPattern abbrev="S508C" type="S508C_NULL_LAYOUT" category="CORRECTNESS" />
+	<BugPattern abbrev="S508C" type="S508C_NO_SETLABELFOR" category="CORRECTNESS" />
+	<BugPattern abbrev="S508C" type="S508C_NO_SETSIZE" category="CORRECTNESS" />
+	<BugPattern abbrev="S508C" type="S508C_NON_ACCESSIBLE_JCOMPONENT" category="CORRECTNESS" />
+	<BugPattern abbrev="S508C" type="S508C_SET_COMP_COLOR" category="CORRECTNESS" />
+	<BugPattern abbrev="UEC" type="UEC_USE_ENUM_COLLECTIONS" category="PERFORMANCE" />
+	<BugPattern abbrev="SIL" type="SIL_SQL_IN_LOOP" category="PERFORMANCE" experimental="true" />
 </FindbugsPlugin>
\ No newline at end of file

Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml	2006-05-02 02:24:14 UTC (rev 515)
+++ trunk/fb-contrib/etc/messages.xml	2006-05-05 05:51:49 UTC (rev 516)
@@ -546,6 +546,17 @@
 			]]>
 		</Details>
 	</Detector>
+	
+	<Detector class="com.mebigfatguy.fbcontrib.detect.SQLInLoop">
+		<Details>
+			<![CDATA[
+			<p>looks for the execution of sql queries inside a loop. This pattern tends to be inefficient,
+			and often can be improved upon, by collecting all the keys needed for the query and issuing just
+			one query using an in clause with all the keys for all the queries previously needed in the loop.</p>
+			<p>It is a fast detector</p>
+			]]>
+		</Details>
+	</Detector>
 
 	<!-- BugPattern -->
 	                
@@ -1173,6 +1184,20 @@
 		</Details>
 	</BugPattern>
 	
+	<BugPattern type="SIL_SQL_IN_LOOP">
+		<ShortDescription>Method executes sql queries inside of loops</ShortDescription>
+		<LongDescription>Method {1} executes sql queries inside of loops</LongDescription>
+		<Details>
+			<![CDATA[
+				<p>This method executes sql queries inside of a loop. This pattern is often inefficient
+				as the number of queries may mushroom in fencepost cases. It is probably more performant
+				to loop over the input and collect the key data needed for the query for all items, and
+				issue one query using an in clause, or similar construct, and then loop over this result
+				set, and fetch all the data at once.
+			]]>
+		</Details>
+	</BugPattern>
+	
 	<!-- BugCode -->
 	
 	<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -1219,4 +1244,5 @@
 	<BugCode abbrev="NRTL">Non Recycleable Taglib</BugCode>
 	<BugCode abbrev="S508C">Section 508 Compliance Violations</BugCode>
 	<BugCode abbrev="UEC">Use Enum Collections</BugCode>
+	<BugCode abbrev="SIL">SQL In Loop</BugCode>
 </MessageCollection>
\ No newline at end of file

Added: trunk/fb-contrib/samples/SIL_Sample.java
===================================================================
--- trunk/fb-contrib/samples/SIL_Sample.java	                        (rev 0)
+++ trunk/fb-contrib/samples/SIL_Sample.java	2006-05-05 05:51:49 UTC (rev 516)
@@ -0,0 +1,20 @@
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class SIL_Sample 
+{
+	public void test(ResultSet rs) throws SQLException
+	{
+		Connection c = rs.getStatement().getConnection();
+		PreparedStatement ps = c.prepareStatement("select foo from boo where moo = ?");
+		
+		while (rs.next()) 
+		{
+			int key = rs.getInt(1);
+			ps.setInt(1, key);
+			ResultSet mrs = ps.executeQuery();
+		}
+	}
+}

Added: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java	                        (rev 0)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SQLInLoop.java	2006-05-05 05:51:49 UTC (rev 516)
@@ -0,0 +1,97 @@
+package com.mebigfatguy.fbcontrib.detect;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.bcel.classfile.Code;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.BytecodeScanningDetector;
+import edu.umd.cs.findbugs.ba.ClassContext;
+
+public class SQLInLoop extends BytecodeScanningDetector 
+{
+	private static final Set<String> queryClasses = new HashSet<String>();
+	static {
+		queryClasses.add("java/sql/Statement");
+		queryClasses.add("java/sql/PreparedStatement");
+		queryClasses.add("java/sql/CallableStatement");
+	}
+	private static final Set<String> queryMethods = new HashSet<String>();
+	static {
+		queryMethods.add("execute");
+		queryMethods.add("executeQuery");
+	};
+	
+	private BugReporter bugReporter;
+	List<Integer> queryLocations;
+	List<LoopLocation> loops;
+	
+	public SQLInLoop(BugReporter bugReporter) {
+		this.bugReporter = bugReporter;
+	}
+	
+	@Override
+	public void visitClassContext(ClassContext classContext) {
+		try {
+			queryLocations = new ArrayList<Integer>();
+			loops = new ArrayList<LoopLocation>();
+			super.visitClassContext(classContext);
+		} finally {
+			queryLocations = null;
+			loops = null;
+		}
+	}
+	
+	@Override
+	public void visitCode(Code obj) {
+		queryLocations.clear();
+		loops.clear();
+		super.visitCode(obj);
+		for (Integer qLoc : queryLocations) {
+			for (LoopLocation lLoc : loops) {
+				if (lLoc.isInLoop(qLoc.intValue())) {
+					bugReporter.reportBug(new BugInstance(this, "SIL_SQL_IN_LOOP", NORMAL_PRIORITY)
+							   .addClass(this)
+							   .addMethod(this)
+							   .addSourceLine(this, qLoc.intValue()));
+					break;
+				}
+			}
+		}
+	}
+	
+	@Override
+	public void sawOpcode(int seen) {
+		if (seen == INVOKEINTERFACE) {
+			String clsName = getClassConstantOperand();
+			String methodName = getNameConstantOperand();
+			
+			if (queryClasses.contains(clsName) && queryMethods.contains(methodName))
+				queryLocations.add(new Integer(getPC()));
+		} else if ((seen == GOTO) || (seen == GOTO_W)) {
+			int branchTarget = getBranchTarget();
+			int pc = getPC();
+			if (branchTarget < pc) {
+				loops.add(new LoopLocation(branchTarget, pc));
+			}
+		}
+	}
+
+	private static class LoopLocation {
+		private int startPC;
+		private int endPC;
+		
+		public LoopLocation(int start, int end) {
+			startPC = start;
+			endPC = end;
+		}
+		
+		public boolean isInLoop(int pc) {
+			return (pc >= startPC) && (pc <= endPC);
+		}
+	}
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.