Merge pull request #2 from maddalab/master

Added a new detector CommonsEqualsBuilderToEquals && CommonsHashcodeBuilderToHashcode

.classpath

@@ -1,18 +1,18 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <classpath>
-	<classpathentry kind="src" path="samples"/>
 	<classpathentry excluding="**/*.*" kind="src" path="etc"/>
 	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="src" path="samples"/>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
 	<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4"/>
-	<classpathentry kind="lib" path="samples/lib/jsp-api.jar"/>
-	<classpathentry kind="lib" path="samples/lib/junit.jar"/>
-	<classpathentry kind="lib" path="samples/lib/log4j.jar" sourcepath="/home/dave/.m2/repository/log4j/log4j/1.2.15/log4j-1.2.15-sources.jar"/>
-	<classpathentry kind="lib" path="samples/lib/servlet-api.jar"/>
-	<classpathentry combineaccessrules="false" kind="src" path="/findbugs"/>
-	<classpathentry kind="lib" path="lib/bcel-2.0.0.jar"/>
-	<classpathentry kind="lib" path="lib/findbugs-2.0.0.jar"/>
 	<classpathentry kind="lib" path="lib/annotations-2.0.0.jar"/>
 	<classpathentry kind="lib" path="lib/asm-tree-3.3.1.jar"/>
+	<classpathentry kind="lib" path="lib/bcel-2.0.0.jar"/>
+	<classpathentry kind="lib" path="lib/findbugs-2.0.0.jar"/>
+	<classpathentry kind="lib" path="samples/lib/commons-lang3-3.1.jar"/>
+	<classpathentry kind="lib" path="samples/lib/jsp-api-2.2.1.jar"/>
+	<classpathentry kind="lib" path="samples/lib/junit-4.10.jar"/>
+	<classpathentry kind="lib" path="samples/lib/log4j-1.2.16.jar"/>
+	<classpathentry kind="lib" path="samples/lib/servlet-api-3.0.1.jar"/>
 	<classpathentry kind="output" path="classes"/>
 </classpath>

.gitignore

@@ -8,3 +8,5 @@ classes
 *.xlr
 *.jar
 *.zip
+.settings/org.eclipse.core.resources.prefs
+.settings/org.eclipse.m2e.core.prefs

build.xml

@@ -27,10 +27,23 @@
 	<property name="annotations.version" value="2.0.0"/>
 	<property name="asm-tree.version" value="3.3.1"/>
 
-    <property name="findbugs-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/findbugs/${findbugs.version}/findbugs-${findbugs.version}.jar"/> 
-    <property name="findbugs-bcel-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/bcel/${findbugs-bcel.version}/bcel-${findbugs-bcel.version}.jar"/> 
-    <property name="annotations-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/annotations/${annotations.version}/annotations-${annotations.version}.jar"/> 
-    <property name="asm-tree-url" value="http://repo1.maven.org/maven2/asm/asm-tree/${asm-tree.version}/asm-tree-${asm-tree.version}.jar"/> 
+        <property name="findbugs-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/findbugs/${findbugs.version}/findbugs-${findbugs.version}.jar"/> 
+        <property name="findbugs-bcel-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/bcel/${findbugs-bcel.version}/bcel-${findbugs-bcel.version}.jar"/> 
+        <property name="annotations-url" value="http://repo1.maven.org/maven2/com/google/code/findbugs/annotations/${annotations.version}/annotations-${annotations.version}.jar"/> 
+        <property name="asm-tree-url" value="http://repo1.maven.org/maven2/asm/asm-tree/${asm-tree.version}/asm-tree-${asm-tree.version}.jar"/> 
+
+       <!-- properties for samples lib dependencies -->
+       <property name="commons-lang3.version" value="3.1"/>
+       <property name="jsp-api.version" value="2.2.1" />
+       <property name="junit.version" value="4.10" />
+       <property name="log4j.version" value="1.2.16" />
+       <property name="servlet-api.version" value="3.0.1" />
+
+       <property name="commons-lang3-url" value="http://repo1.maven.org/maven2/org/apache/commons/commons-lang3/${commons-lang3.version}/commons-lang3-${commons-lang3.version}.jar"/> 
+       <property name="jsp-api-url" value="http://repo1.maven.org/maven2/javax/servlet/jsp/javax.servlet.jsp-api/${jsp-api.version}/javax.servlet.jsp-api-${jsp-api.version}.jar" />
+       <property name="junit-url" value="http://repo1.maven.org/maven2/junit/junit/${junit.version}/junit-${junit.version}.jar"/> 
+       <property name="log4j-url" value="http://repo1.maven.org/maven2/log4j/log4j/${log4j.version}/log4j-${log4j.version}.jar"/> 
+       <property name="servlet-api-url" value="http://repo1.maven.org/maven2/javax/servlet/javax.servlet-api/${servlet-api.version}/javax.servlet-api-${servlet-api.version}.jar"/> 
 
 	<target name="clean" description="removes all generated collateral">
 		<delete dir="${classes.dir}"/>
@@ -48,6 +61,7 @@
 
 	<target name="-init" description="prepares repository for a build">
 		<mkdir dir="${lib.dir}"/>
+		<mkdir dir="${sampleslib.dir}"/>
 		<mkdir dir="${classes.dir}"/>
 		<mkdir dir="${javadoc.dir}"/>
 		<path id="fb-contrib.classpath">
@@ -57,10 +71,11 @@
 			<pathelement location="${lib.dir}/asm-tree-${asm-tree.version}.jar"/>
 		</path>
 		<path id="fb-contrib.samples.classpath">
-			<pathelement location="${sampleslib.dir}/jsp-api.jar"/>
-			<pathelement location="${sampleslib.dir}/junit.jar"/>
-			<pathelement location="${sampleslib.dir}/servlet-api.jar"/>
-			<pathelement location="${sampleslib.dir}/log4j.jar"/>
+			<pathelement location="${sampleslib.dir}/jsp-api-${jsp-api.version}.jar"/>
+			<pathelement location="${sampleslib.dir}/junit-${junit.version}.jar"/>
+			<pathelement location="${sampleslib.dir}/servlet-api-${servlet-api.version}.jar"/>
+			<pathelement location="${sampleslib.dir}/log4j-${log4j.version}.jar"/>
+			<pathelement location="${sampleslib.dir}/commons-lang3-${commons-lang3.version}.jar"/>
 		</path>
 		<mkdir dir="${classes.dir}/com"/>
 		<mkdir dir="${classes.dir}/com/mebigfatguy"/>
@@ -69,26 +84,46 @@
 		<echo message="*.class" file="${classes.dir}/com/mebigfatguy/fbcontrib/detect/.cvsignore"/>
 	</target>
 
-	<target name="findbugs-check">
+    <target name="findbugs-check">
         <available file="${basedir}/lib/findbugs-${findbugs.version}.jar" property="findbugs-exists"/>
     </target>
 
-	<target name="findbugs-bcel-check">
+    <target name="findbugs-bcel-check">
         <available file="${basedir}/lib/bcel-${findbugs-bcel.version}.jar" property="findbugs-bcel-exists"/>
     </target>
 
-	<target name="annotations-check">
+    <target name="annotations-check">
         <available file="${basedir}/lib/annotations-${annotations.version}.jar" property="annotations-exists"/>
     </target>
 
-	<target name="asm-tree-check">
+    <target name="asm-tree-check">
         <available file="${basedir}/lib/asm-tree-${asm-tree.version}.jar" property="asm-tree-exists"/>
     </target>
 
-	<target name="install-findbugs" depends="findbugs-check" unless="findbugs-exists" description="installs findbugs.jar into lib">
+    <target name="commons-lang3-check">
+        <available file="${basedir}/samples/lib/commons-lang3-${commons-lang3.version}.jar" property="commons-lang3-exists"/>
+    </target>
+
+    <target name="servlet-api-check">
+        <available file="${basedir}/samples/lib/servlet-api-${servlet-api.version}.jar" property="servlet-api-exists"/>
+    </target>
+
+    <target name="jsp-api-check">
+        <available file="${basedir}/samples/lib/jsp-api-${jsp-api.version}.jar" property="jsp-api-exists"/>
+    </target>
+
+    <target name="log4j-check">
+        <available file="${basedir}/samples/lib/log4j-${log4j.version}.jar" property="log4j-exists"/>
+    </target>
+
+    <target name="junit-check">
+        <available file="${basedir}/samples/lib/junit-${junit.version}.jar" property="junit-exists"/>
+    </target>
+
+    <target name="install-findbugs" depends="findbugs-check" unless="findbugs-exists" description="installs findbugs.jar into lib">
         <get src="${findbugs-url}" dest="${basedir}/lib/findbugs-${findbugs.version}.jar" verbose="true" ignoreerrors="true"/>
     </target>
-	
+
     <target name="install-findbugs-bcel" depends="findbugs-bcel-check" unless="findbugs-bcel-exists" description="installs findbugs-bcel.jar into lib">
         <get src="${findbugs-bcel-url}" dest="${basedir}/lib/bcel-${findbugs-bcel.version}.jar" verbose="true" ignoreerrors="true"/>
     </target>
@@ -101,7 +136,27 @@
         <get src="${asm-tree-url}" dest="${basedir}/lib/asm-tree-${asm-tree.version}.jar" verbose="true" ignoreerrors="true"/>
     </target>
 
-    <target name="pull" depends="install-findbugs, install-findbugs-bcel, install-annotations, install-asm-tree" description="pull 3rdparty jars to the lib directory"/>
+    <target name="install-commons-lang3" depends="commons-lang3-check" unless="commons-lang3-exists" description="installs commons-lang3 into samples/lib">
+        <get src="${commons-lang3-url}" dest="${basedir}/samples/lib/commons-lang3-${commons-lang3.version}.jar" verbose="true" ignoreerrors="true"/>
+    </target>
+
+    <target name="install-servlet-api" depends="servlet-api-check" unless="servlet-api-exists" description="installs servlet-api into samples/lib">
+        <get src="${servlet-api-url}" dest="${basedir}/samples/lib/servlet-api-${servlet-api.version}.jar" verbose="true" ignoreerrors="true"/>
+    </target>
+
+    <target name="install-jsp-api" depends="jsp-api-check" unless="jsp-api-exists" description="installs jsp-api into samples/lib">
+        <get src="${jsp-api-url}" dest="${basedir}/samples/lib/jsp-api-${jsp-api.version}.jar" verbose="true" ignoreerrors="true"/>
+    </target>
+
+    <target name="install-log4j" depends="log4j-check" unless="log4j-exists" description="installs log4j into samples/lib">
+        <get src="${log4j-url}" dest="${basedir}/samples/lib/log4j-${log4j.version}.jar" verbose="true" ignoreerrors="true"/>
+    </target>
+
+    <target name="install-junit" depends="junit-check" unless="junit-exists" description="installs junit into samples/lib">
+        <get src="${junit-url}" dest="${basedir}/samples/lib/junit-${junit.version}.jar" verbose="true" ignoreerrors="true"/>
+    </target>
+
+    <target name="pull" depends="install-findbugs, install-findbugs-bcel, install-annotations, install-asm-tree, install-commons-lang3, install-servlet-api, install-jsp-api, install-log4j, install-junit" description="pull 3rdparty jars to the lib directory"/>
 
 	<target name="validate_xml" depends="-init" description="validates the xml files">
 		<xmlvalidate lenient="false" failonerror="yes">

etc/bugrank.txt

@@ -125,4 +125,5 @@
 0 BugPattern PDP_POORLY_DEFINED_PARAMETER
 0 BugPattern NSE_NON_SYMMETRIC_EQUALS
 0 BugPattern CVAA_CONTRAVARIANT_ARRAY_ASSIGNMENT
-0 BugPattern NFF_NON_FUNCTIONAL_FIELD
+0 BugPattern NFF_NON_FUNCTIONAL_FIELD
+0 BugPattern CEBE_COMMONS_EQUAL_BUILDER_TOEQUALS

etc/findbugs.xml

@@ -223,6 +223,10 @@
 	<Detector class="com.mebigfatguy.fbcontrib.detect.LingeringGraphicsObjects" speed="fast" reports="LGO_LINGERING_GRAPHICS_OBJECT" />
 
 	<Detector class="com.mebigfatguy.fbcontrib.detect.StackedTryBlocks" speed="fast" reports="STB_STACKED_TRY_BLOCKS" />
+
+	<Detector class="com.mebigfatguy.fbcontrib.detect.CommonsEqualsBuilderToEquals" speed="fast" reports="CEBE_COMMONS_EQUALS_BUILDER_ISEQUALS" />
+
+	<Detector class="com.mebigfatguy.fbcontrib.detect.CommonsHashcodeBuilderToHashcode" speed="fast" reports="CHTH_COMMONS_HASHCODE_BUILDER_TOHASHCODE" />
 
 	<!-- BugPattern -->
 
@@ -391,4 +395,6 @@
 	<BugPattern abbrev="SGSU" type="SGSU_SUSPICIOUS_GETTER_SETTER_USE" category="CORRECTNESS" experimental="true" />
 	<BugPattern abbrev="LGO" type="LGO_LINGERING_GRAPHICS_OBJECT" category="PERFORMANCE" experimental="true" />
 	<BugPattern abbrev="STB" type="STB_STACKED_TRY_BLOCKS" category="STYLE" experimental="true" />
+	<BugPattern abbrev="CEBE" type="CEBE_COMMONS_EQUALS_BUILDER_ISEQUALS" category="CORRECTNESS" />
+	<BugPattern abbrev="CHTH" type="CHTH_COMMONS_HASHCODE_BUILDER_TOHASHCODE" category="CORRECTNESS" />
 </FindbugsPlugin>

etc/messages.xml

@@ -1217,6 +1217,26 @@
 		</Details>
 	</Detector>
 
+    <Detector class="com.mebigfatguy.fbcontrib.detect.CommonsEqualsBuilderToEquals">
+        <Details>
+            <![CDATA[
+            <p>This detector looks for uses for Commons-lang EqualsBuilder where the
+            result of equals() is returned instead of calling the method isEquals().</p>
+            <p>It is a fast detector</p>
+            ]]>
+        </Details>
+    </Detector>
+
+   <Detector class="com.mebigfatguy.fbcontrib.detect.CommonsHashcodeBuilderToHashcode">
+        <Details>
+            <![CDATA[
+            <p>This detector looks for uses for Commons-lang HashCodeBuilder where the
+            result of hashCode() is returned instead of calling the method toHashCode().</p>
+            <p>It is a fast detector</p>
+            ]]>
+        </Details>
+    </Detector>
+
 	<!-- BugPattern -->
 
 	<BugPattern type="ISB_INEFFICIENT_STRING_BUFFERING">
@@ -3340,6 +3360,28 @@
 		</Details>
 	</BugPattern>
 
+    <BugPattern type="CEBE_COMMONS_EQUALS_BUILDER_ISEQUALS">
+        <ShortDescription>Method returns the result of invoking equals() on EqualsBuilder</ShortDescription>
+        <LongDescription>Method {1} returns the result of invoking equals() in EqualsBuilder</LongDescription>
+        <Details>
+            <![CDATA[
+            <p>This method returns the result of equals on the EqualsBuilder type
+            instead of calling the method isEqual().</p>
+            ]]>
+        </Details>
+    </BugPattern>
+
+    <BugPattern type="CHTH_COMMONS_HASHCODE_BUILDER_TOHASHCODE">
+        <ShortDescription>Method returns the result of invoking hashCode() on HashCodeBuilder</ShortDescription>
+        <LongDescription>Method {1} returns the result of invoking hashCode() in HashCodeBuilder</LongDescription>
+        <Details>
+            <![CDATA[
+            <p>This method returns the result of hashCode on the HashCodeBuilder type
+            instead of calling the method toHashCode().</p>
+            ]]>
+        </Details>
+    </BugPattern>            
+
 	<!-- BugCode -->
 
 	<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -3442,4 +3484,5 @@
 	<BugCode abbrev="SGSU">Suspicious Getter Setter Use</BugCode>
 	<BugCode abbrev="LGO">Lingering Graphics Object</BugCode>
 	<BugCode abbrev="STB">Stacked Try Blocks</BugCode>
+	<BugCode abbrev="CEBE">Commons EqualsBuilder To Equals</BugCode>
 </MessageCollection>

samples/CEBE_EqualsToEqualsSample.java

@@ -0,0 +1,31 @@
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+
+public class CEBE_EqualsToEqualsSample {
+	public final String name;
+	public final int age;
+
+	CEBE_EqualsToEqualsSample(String name, int age) {
+		this.name = name;
+		this.age = age;
+	}
+
+	@Override
+	public int hashCode() {
+		return new HashCodeBuilder().append(name).append(age).toHashCode();
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		CEBE_EqualsToEqualsSample other = (CEBE_EqualsToEqualsSample) obj;
+		return new EqualsBuilder().append(this.name, other.name)
+				.append(this.age, other.age).equals(obj);
+	}
+
+}

samples/CHBH_HashcodeToHashcodeSample.java

@@ -0,0 +1,30 @@
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+
+public class CHBH_HashcodeToHashcodeSample {
+	public final String name;
+	public final int age;
+
+	CHBH_HashcodeToHashcodeSample(String name, int age) {
+		this.name = name;
+		this.age = age;
+	}
+
+	@Override
+	public int hashCode() {
+		return new HashCodeBuilder().append(name).append(age).hashCode();
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		CHBH_HashcodeToHashcodeSample other = (CHBH_HashcodeToHashcodeSample) obj;
+		return new EqualsBuilder().append(this.name, other.name)
+				.append(this.age, other.age).isEquals();
+	}
+}

src/com/mebigfatguy/fbcontrib/detect/CommonsEqualsBuilderToEquals.java

@@ -0,0 +1,79 @@
+package com.mebigfatguy.fbcontrib.detect;
+
+import org.apache.bcel.classfile.Code;
+import org.apache.bcel.classfile.LocalVariableTable;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.BytecodeScanningDetector;
+import edu.umd.cs.findbugs.OpcodeStack;
+
+/**
+ * Find usage of EqualsBuilder from Apache commons, where the code invoke
+ * equals() on the constructed object rather than isEquals()
+ * 
+ * <pre>
+ * new EqualsBuilder().append(this.name, other.name).equals(other);
+ * </pre>
+ */
+public class CommonsEqualsBuilderToEquals extends BytecodeScanningDetector {
+
+	private final OpcodeStack stack;
+	private final BugReporter bugReporter;
+
+	/**
+	 * constructs a CEBE detector given the reporter to report bugs on.
+	 * 
+	 * @param bugReporter
+	 *            the sync of bug reports
+	 */
+	public CommonsEqualsBuilderToEquals(final BugReporter bugReporter) {
+		stack = new OpcodeStack();
+		this.bugReporter = bugReporter;
+	}
+
+	/**
+	 * implements the visitor to pass through constructors and static
+	 * initializers to the byte code scanning code. These methods are not
+	 * reported, but are used to build SourceLineAnnotations for fields, if
+	 * accessed.
+	 * 
+	 * @param obj
+	 *            the context object of the currently parsed code attribute
+	 */
+	@Override
+	public void visitCode(Code obj) {
+		stack.resetForMethodEntry(this);
+		LocalVariableTable lvt = getMethod().getLocalVariableTable();
+		if (lvt != null) {
+			super.visitCode(obj);
+		}
+	}
+
+	@Override
+	public void sawOpcode(int seen) {
+		try {
+			switch (seen) {
+			case INVOKEVIRTUAL:
+				String methodName = getNameConstantOperand();
+				if ("equals".equals(methodName)
+						&& "(Ljava/lang/Object;)Z"
+								.equals(getSigConstantOperand())) {
+					String calledClass = stack.getStackItem(1).getSignature();
+					if ("Lorg/apache/commons/lang3/builder/EqualsBuilder;"
+							.equals(calledClass)
+							|| "org/apache/commons/lang/builder/EqualsBuilder"
+									.equals(calledClass)) {
+						bugReporter.reportBug(new BugInstance(this,
+								"CEBE_COMMONS_EQUALS_BUILDER_ISEQUALS",
+								HIGH_PRIORITY).addClass(this).addMethod(this)
+								.addSourceLine(this));
+					}
+				}
+			}
+		} finally {
+			super.sawOpcode(seen);
+			stack.sawOpcode(this, seen);
+		}
+	}
+}