[Fb-contrib-commit] fb-contrib/src/com/mebigfatguy/fbcontrib/detect DubiousListCollection.java,NONE,1.1

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

Update of /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv17178/src/com/mebigfatguy/fbcontrib/detect

Added Files:
	DubiousListCollection.java 
Log Message:
Initial Checkin: New detector DLC, find members declared as Lists that probably should be defined as Sets

--- NEW FILE: DubiousListCollection.java ---
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.Constants;
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;
import edu.umd.cs.findbugs.OpcodeStack;
import edu.umd.cs.findbugs.ba.ClassContext;

public class DubiousListCollection extends BytecodeScanningDetector
{
	private static final Integer FIELD_UNKNOWN = new Integer(0);
	
	private static Set<String> setMethods = new HashSet<String>();
	private static Set<String> listMethods = new HashSet<String>();
	static {
		setMethods.add("contains(Ljava/lang/Object;)Z");
		setMethods.add("containsAll(Ljava/util/Collection;)Z");
		setMethods.add("remove(Ljava/lang/Object;)Ljava/lang/Object;");
		setMethods.add("removeAll(Ljava/util/Collection;)Z");
		setMethods.add("retainAll(Ljava/util/Collection;)Z");
		
		listMethods.add("add(ILjava/lang/Object;)V");
		listMethods.add("addAll(ILjava/util/Collection;)Z");
		listMethods.add("lastIndexOf(Ljava/lang/Object;)I");
		listMethods.add("remove(I)Ljava/lang/Object;");
		listMethods.add("set(ILjava/lang/Object;)Ljava/lang/Object;");
		listMethods.add("subList(II)Ljava/util/List;");
		
		//Theoretically get(i) and indexOf(Object) are setMethods but are so abused, as to be meaningless
	}
	private BugReporter bugReporter;
	private OpcodeStack stack = new OpcodeStack();
	private Map<String, Integer> fieldsReported = new HashMap<String, Integer>();
	
	public DubiousListCollection(BugReporter bugReporter) {
		this.bugReporter = bugReporter;
	}
	
	public void visitClassContext(ClassContext classContext) {
		JavaClass cls = classContext.getJavaClass();
		Field[] flds = cls.getFields();
		for (Field f : flds) {
			String sig = f.getSignature();
			if (sig.charAt(0) == 'L') {
				sig = sig.substring(1, sig.length() - 1);
				if (sig.startsWith("java/util/") && sig.endsWith("List")) {
					fieldsReported.put(f.getName(), FIELD_UNKNOWN);
				}
			}
		}
		
		if (fieldsReported.size() > 0) {
			super.visitClassContext(classContext);
			reportBugs();
		}
	}
	
	public void visitCode(Code obj) {
		stack.resetForMethodEntry(this);
		super.visitCode(obj);
	}
	
	public void sawOpcode(int seen) {
		try {
			if (seen == INVOKEINTERFACE) {
				String className = this.getClassConstantOperand();
				String methodName = getNameConstantOperand();
				String signature = getSigConstantOperand();
				
				if (className.startsWith("java/util/") && className.endsWith("List")) {
					int parmCount = Type.getArgumentTypes(signature).length;
					if (stack.getStackDepth() > parmCount) {
						OpcodeStack.Item itm = stack.getStackItem(parmCount);
						FieldAnnotation fa = itm.getField();
						if (fa != null) {
							String field = fa.getFieldName();
							Integer cnt = fieldsReported.get(field);
							if (cnt != null) {
								String methodInfo = methodName + signature;
								if (listMethods.contains(methodInfo))
									fieldsReported.remove(field);
								else if (setMethods.contains(methodInfo)) {
									cnt = new Integer(cnt.intValue() + 1);
									fieldsReported.put(field, cnt);
								}
							}
						}
					}
				}
			}
		} finally {
			stack.sawOpcode(this, seen);
		}
	}
	
	private void reportBugs() {
		for (Map.Entry<String, Integer> entry : fieldsReported.entrySet()) {
			String field = entry.getKey();
			int cnt = entry.getValue().intValue();
			if (cnt > 0) {
				FieldAnnotation fa = getFieldAnnotation(field);
				if (fa != null) {
					bugReporter.reportBug(new BugInstance(this, "DLC_DUBIOUS_LIST_COLLECTION", NORMAL_PRIORITY)
							.addClass(this)
							.addField(fa));
				}
			}
		}
	}
	
	private FieldAnnotation getFieldAnnotation(String fieldName) {
		JavaClass cls = getClassContext().getJavaClass();
		Field[] fields = cls.getFields();
		for (Field f : fields) {
			if (f.getName().equals(fieldName))
				return new FieldAnnotation(cls.getClassName(), fieldName, f.getSignature(), (f.getAccessFlags() & Constants.ACC_STATIC) != 0);
		}
		return null; //shouldn't happen
	}
}