[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.