[Fb-contrib-commit] SF.net SVN: fb-contrib: [535] trunk/fb-contrib/etc
Brought to you by:
dbrosius
From: <dbr...@us...> - 2006-05-15 03:27:08
|
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. |