More tests, immutable fields support

runTests.sh

@@ -21,7 +21,7 @@ cd ../..
 
 # Now running findbugs and grabbing the output
 echo "Running findbugs ...."
-COMPILED_TEST_FILES=`find build/test -name *Test.class`
+COMPILED_TEST_FILES=`find build/test -name *.class`
 FB_AUXCLASPATH=dist/staticaccess-detector-annotations.jar
 FB_PLUGINLIST=dist/staticaccess-detector.jar
 FB_OUTPUT=`java -jar $FINDBUGS_JAR -textui -auxclasspath $FB_AUXCLASPATH -pluginList $FB_PLUGINLIST $COMPILED_TEST_FILES 2>/dev/null`

src/main/detectors/staticaccess/StaticAccessDetector.java

@@ -2,10 +2,13 @@
 
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.util.StringMatcher;
+import edu.umd.cs.findbugs.util.ExactStringMatcher;
 import edu.umd.cs.findbugs.classfile.DescriptorFactory;
 import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
 import edu.umd.cs.findbugs.ba.ClassContext;
 import edu.umd.cs.findbugs.ba.XMethod;
+import edu.umd.cs.findbugs.ba.XField;
 import edu.umd.cs.findbugs.ba.bcp.Invoke;
 import edu.umd.cs.findbugs.ba.jsr305.TypeQualifierApplications;
 import edu.umd.cs.findbugs.ba.jsr305.TypeQualifierValue;
@@ -16,6 +19,8 @@
 import java.util.List;
 import java.util.Arrays;
 
+import detectors.staticaccess.matcher.MethodMatcher;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  */
@@ -39,8 +44,6 @@ public void visitClassContext(ClassContext classContext) {
 
 	@Override
 	public void visitMethod(Method obj) {
-		System.out.println("obj.getName() = " + obj.getName());
-		System.out.println("obj.getSignature() = " + obj.getSignature());
 		visitingStaticIndependentMethod = isMethodStaticIndependent(getXMethod()) && !ignoreMethod(getXMethod());
 		super.visitMethod(obj);
 	}
@@ -56,6 +59,19 @@ public void sawOpcode(int seen) {
 				case GETSTATIC:
 				case GETSTATIC_QUICK:
 				case GETSTATIC2_QUICK:
+					// Checking if we aren't getting a constant (static final immutable)
+					XField referencedField = getXFieldOperand();
+					// The field may be null if the class is missing
+					if (referencedField != null && referencedField.isStatic() && referencedField.isFinal()) {
+						String referencedFieldSignature = referencedField.getSignature();
+						// Checking if it's immutable
+						for (StringMatcher immutableClassSignature : immutableClassSignatures) {
+							if (immutableClassSignature.matches(referencedFieldSignature)) {
+								return;
+							}
+						}
+					}
+
 				case PUTSTATIC:
 				case PUTSTATIC_QUICK:
 				case PUTSTATIC2_QUICK:
@@ -133,6 +149,10 @@ private boolean ignoreMethod(XMethod method) {
 	 * A list of ignored methods, which aren't checked for being static-independent, regardless of the annotations.
 	 */
 	private static final List<MethodMatcher> ignoreMethodMatchers = new ArrayList<MethodMatcher>();
+	/**
+	 * A list of patterns for matching immutable classes.
+	 */
+	private static final List<StringMatcher> immutableClassSignatures;
 
 	static {
 		MethodMatcher[] implicitMethods = new MethodMatcher[] {
@@ -150,5 +170,18 @@ private boolean ignoreMethod(XMethod method) {
 	static {
 		ignoreMethodMatchers.add(new MethodMatcher("/.*", "<clinit>", "()V", Invoke.STATIC));
 	}
+
+	static {
+		StringMatcher[] immutableClasses = new StringMatcher[] {
+				new ExactStringMatcher("Ljava/lang/String;"),
+				new ExactStringMatcher("Ljava/lang/Integer;"),
+				new ExactStringMatcher("Ljava/lang/Float;"),
+				new ExactStringMatcher("Ljava/lang/Double;"),
+				new ExactStringMatcher("Ljava/lang/Long;"),
+				new ExactStringMatcher("Ljava/lang/Boolean;")
+		};
+
+		immutableClassSignatures = Arrays.asList(immutableClasses);
+	}
 }
 

src/main/detectors/staticaccess/MethodMatcher.java → src/main/detectors/staticaccess/matcher/MethodMatcher.java

@@ -1,10 +1,7 @@
-package detectors.staticaccess;
+package detectors.staticaccess.matcher;
 
-import edu.umd.cs.findbugs.ba.AnalysisContext;
-import edu.umd.cs.findbugs.ba.Hierarchy;
 import edu.umd.cs.findbugs.ba.XMethod;
-
-import java.util.regex.Pattern;
+import edu.umd.cs.findbugs.util.StringMatcher;
 
 /**
  * Based on {@link edu.umd.cs.findbugs.ba.bcp.Invoke}. Most of the classes/code there is private, hence the copying ...
@@ -37,76 +34,19 @@ public class MethodMatcher {
 	 * Match both static and instance invocations.
 	 */
 	public static final int ANY = INSTANCE | STATIC | CONSTRUCTOR;
-
-	private static interface StringMatcher {
-		public boolean match(String s);
-	}
-
-	private static class ExactStringMatcher implements StringMatcher {
-		private String value;
-
-		public ExactStringMatcher(String value) {
-			this.value = value;
-		}
-
-		public boolean match(String s) {
-			return s.equals(value);
-		}
-	}
-
-	private static class RegexpStringMatcher implements StringMatcher {
-		private Pattern pattern;
-
-		public RegexpStringMatcher(String re) {
-			pattern = Pattern.compile(re);
-		}
-
-		public boolean match(String s) {
-			return pattern.matcher(s).matches();
-		}
-	}
-
-	private static class SubclassMatcher implements StringMatcher {
-		private String className;
-
-		public SubclassMatcher(String className) {
-			this.className = className;
-		}
-
-		public boolean match(String s) {
-			try {
-				return Hierarchy.isSubtype(s, className);
-			} catch (ClassNotFoundException e) {
-				AnalysisContext.reportMissingClass(e);
-				return false;
-			}
-		}
-	}
 
 	private final StringMatcher classNameMatcher;
 	private final StringMatcher methodNameMatcher;
 	private final StringMatcher methodSigMatcher;
 	private final int mode;
 
 	public MethodMatcher(String className, String methodName, String methodSig, int mode) {
-		this.classNameMatcher = createClassMatcher(className);
-		this.methodNameMatcher = createMatcher(methodName);
-		this.methodSigMatcher = createMatcher(methodSig);
+		this.classNameMatcher = StringMatcherFactory.createClassMatcher(className);
+		this.methodNameMatcher = StringMatcherFactory.createMatcher(methodName);
+		this.methodSigMatcher = StringMatcherFactory.createMatcher(methodSig);
 		this.mode = mode;
 	}
 
-	private StringMatcher createClassMatcher(String s) {
-		return s.startsWith("+")
-				? new SubclassMatcher(s.substring(1))
-				: createMatcher(s);
-	}
-
-	private StringMatcher createMatcher(String s) {
-		return s.startsWith("/")
-				? new RegexpStringMatcher(s.substring(1))
-				: new ExactStringMatcher(s);
-	}
-
 	public boolean matches(XMethod method) {
 		String methodName = method.getName();
 		boolean isStatic = method.isStatic();
@@ -124,9 +64,9 @@ public boolean matches(XMethod method) {
 		}
 
 		// Check class name, method name, and method signature.
-		if (!methodNameMatcher.match(methodName) ||
-				!methodSigMatcher.match(method.getSignature()) ||
-				!classNameMatcher.match(method.getClassName())) {
+		if (!methodNameMatcher.matches(methodName) ||
+				!methodSigMatcher.matches(method.getSignature()) ||
+				!classNameMatcher.matches(method.getClassName())) {
 			return false;
 		}
 

src/main/detectors/staticaccess/matcher/StringMatcherFactory.java

@@ -0,0 +1,22 @@
+package detectors.staticaccess.matcher;
+
+import edu.umd.cs.findbugs.util.StringMatcher;
+import edu.umd.cs.findbugs.util.ExactStringMatcher;
+import edu.umd.cs.findbugs.util.RegexStringMatcher;
+
+/**
+ * @author Adam Warski (adam at warski dot org)
+ */
+public class StringMatcherFactory {
+	public static StringMatcher createClassMatcher(String s) {
+		return s.startsWith("+")
+				? new SubclassMatcher(s.substring(1))
+				: createMatcher(s);
+	}
+
+	public static StringMatcher createMatcher(String s) {
+		return s.startsWith("/")
+				? new RegexStringMatcher(s.substring(1))
+				: new ExactStringMatcher(s);
+	}
+}

src/main/detectors/staticaccess/matcher/SubclassMatcher.java

@@ -0,0 +1,25 @@
+package detectors.staticaccess.matcher;
+
+import edu.umd.cs.findbugs.ba.Hierarchy;
+import edu.umd.cs.findbugs.ba.AnalysisContext;
+import edu.umd.cs.findbugs.util.StringMatcher;
+
+/**
+ * @author Adam Warski (adam at warski dot org)
+*/
+public class SubclassMatcher implements StringMatcher {
+	private String className;
+
+	public SubclassMatcher(String className) {
+		this.className = className;
+	}
+
+	public boolean matches(String s) {
+		try {
+			return Hierarchy.isSubtype(s, className);
+		} catch (ClassNotFoundException e) {
+			AnalysisContext.reportMissingClass(e);
+			return false;
+		}
+	}
+}

test/src/detectors/staticaccess/InheritanceTest.java

@@ -0,0 +1,22 @@
+package detectors.staticaccess;
+
+/**
+ * @author Adam Warski (adam at warski dot org)
+ */
+public class InheritanceTest {
+	private static interface Base {
+		@StaticIndependent
+		void compute();
+	}
+
+	private static class Impl implements Base {
+		private static String a;
+
+		public void compute() {
+			a = "z";		// error
+			other();		// error
+		}
+
+		private void other() { }
+	}
+}

test/src/detectors/staticaccess/InheritanceTest.out

@@ -0,0 +1,2 @@
+SANA 16
+NSIMI 17

test/src/detectors/staticaccess/ReadConstantTest.java

@@ -15,7 +15,12 @@ public void m1() {
 	    String a = c1;
 		Long b = c2;
 		OtherClass c = nc1;		// error
+		String d = OtherClass2.c3;
 	}
 
 	private static class OtherClass { }
+
+	private static class OtherClass2 {
+		static final String c3 = "y";
+	}
 }

test/working/example/Test3.java

@@ -1,37 +1,21 @@
 package example;
 
-import edu.umd.cs.findbugs.annotations.DefaultAnnotationForMethods;
 import detectors.staticaccess.StaticIndependent;
-import detectors.staticaccess.StaticDependent;
 
-@DefaultAnnotationForMethods(StaticIndependent.class)
 public class Test3 {
-	static int c;
-	static final Integer z = 12;
-
-	@StaticDependent
-	static void b() {
-		c = 10;
-	}
-
-	static void d() {
-		c = 12;
-	}
-
-	static void e() {
-		compute(z);
+	private static interface Base {
+		@StaticIndependent
+		void compute();
 	}
 
-	static void compute(Integer a) { }
-}
+	private static String a;
 
-interface Z {
-	@StaticIndependent
-	void i();
-}
+	private static class Impl implements Base {
+		public void compute() {
+			a = "z";		// error
+			other();		// error
+		}
 
-class ZZ implements Z {
-	public void i() {
-		Test3.c = 102;
+		private void other() { }
 	}
 }