Bug#1418 Improve annotation of SIC_INNER_SHOULD_BE_STATIC_ANON

spotbugs · Oct 4, 2015 · 6c91c0c · 6c91c0c
1 parent 5e216b9
commit 6c91c0c
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 4 deletions.
diff --git a/findbugs/src/java/edu/umd/cs/findbugs/ClassAnnotation.java b/findbugs/src/java/edu/umd/cs/findbugs/ClassAnnotation.java
@@ -45,6 +45,8 @@ public class ClassAnnotation extends PackageMemberAnnotation {
 
     public static final String SUPERCLASS_ROLE = "CLASS_SUPERCLASS";
 
+    public static final String ANONYMOUS_ROLE = "CLASS_ANONYMOUS";
+
     public static final String RECOMMENDED_SUPERCLASS_ROLE = "CLASS_RECOMMENDED_SUPERCLASS";
 
     public static final String IMPLEMENTED_INTERFACE_ROLE = "CLASS_IMPLEMENTED_INTERFACE";

diff --git a/findbugs/src/java/edu/umd/cs/findbugs/FindBugsAnnotationDescriptions.properties b/findbugs/src/java/edu/umd/cs/findbugs/FindBugsAnnotationDescriptions.properties
@@ -3,6 +3,7 @@ CLASS_EXCEPTION			=Exception class {0}
 CLASS_REFTYPE			=Reference type {0}
 CLASS_SUBCLASS          =has subclass {0}
 CLASS_SUPERCLASS        =superclass is {0}
+CLASS_ANONYMOUS         =Anonymous class {0}
 CLASS_RECOMMENDED_SUPERCLASS =Perhaps class should be refactored to extend {0}
 CLASS_IMPLEMENTED_INTERFACE =implements interface {0}
 CLASS_ANNOTATION        =annotated with {0.simpleName}

diff --git a/findbugs/src/java/edu/umd/cs/findbugs/detect/UnreadFields.java b/findbugs/src/java/edu/umd/cs/findbugs/detect/UnreadFields.java
@@ -19,12 +19,14 @@
 
 package edu.umd.cs.findbugs.detect;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
@@ -44,10 +46,16 @@
 import org.apache.bcel.generic.Type;
 
 import edu.umd.cs.findbugs.BugAccumulator;
+import edu.umd.cs.findbugs.BugAnnotation;
 import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.ClassAnnotation;
 import edu.umd.cs.findbugs.DeepSubtypeAnalysis;
+import edu.umd.cs.findbugs.FieldAnnotation;
+import edu.umd.cs.findbugs.LocalVariableAnnotation;
+import edu.umd.cs.findbugs.MethodAnnotation;
 import edu.umd.cs.findbugs.OpcodeStack;
+import edu.umd.cs.findbugs.OpcodeStack.Item;
 import edu.umd.cs.findbugs.Priorities;
 import edu.umd.cs.findbugs.ProgramPoint;
 import edu.umd.cs.findbugs.SourceLineAnnotation;
@@ -56,6 +64,7 @@
 import edu.umd.cs.findbugs.ba.XClass;
 import edu.umd.cs.findbugs.ba.XFactory;
 import edu.umd.cs.findbugs.ba.XField;
+import edu.umd.cs.findbugs.ba.XMethod;
 import edu.umd.cs.findbugs.ba.ch.Subtypes2;
 import edu.umd.cs.findbugs.ba.generic.GenericObjectType;
 import edu.umd.cs.findbugs.ba.generic.GenericUtilities;
@@ -104,6 +113,8 @@ public boolean isContainerField(XField f) {
 
     boolean publicOrProtectedConstructor;
 
+    private final Map<String, List<BugAnnotation>> anonymousClassAnnotation = new HashMap<>();
+
     /**
      * @deprecated Use {@link edu.umd.cs.findbugs.detect.UnreadFieldsData#getReadFields()} instead
      */
@@ -571,6 +582,32 @@ public void sawOpcode(int seen) {
             }
         }
 
+        // Store annotation for the anonymous class creation
+        if (seen == INVOKESPECIAL && getMethodDescriptorOperand().getName().equals("<init>") && ClassName.isAnonymous(getClassConstantOperand())) {
+            List<BugAnnotation> annotation = new ArrayList<>();
+            annotation.add(ClassAnnotation.fromClassDescriptor(getClassDescriptor()));
+            annotation.add(MethodAnnotation.fromVisitedMethod(this));
+            annotation.add(SourceLineAnnotation.fromVisitedInstruction(this));
+            anonymousClassAnnotation.put(getClassDescriptorOperand().getDottedClassName(), annotation);
+        }
+
+        if (seen == PUTFIELD || seen == ASTORE || seen == ASTORE_0 || seen == ASTORE_1 || seen == ASTORE_2 || seen == ASTORE_3) {
+            Item item = stack.getStackItem(0);
+            XMethod xMethod = item.getReturnValueOf();
+            if(xMethod != null && xMethod.getName().equals("<init>") && ClassName.isAnonymous(xMethod.getClassName())) {
+                List<BugAnnotation> annotations = anonymousClassAnnotation.get(xMethod.getClassName());
+                if(annotations == null) {
+                    annotations = new ArrayList<>();
+                }
+                if(seen == PUTFIELD) {
+                    annotations.add(FieldAnnotation.fromReferencedField(this));
+                } else {
+                    annotations.add(LocalVariableAnnotation.getLocalVariableAnnotation(getMethod(), getRegisterOperand(), getPC(), getNextPC()));
+                }
+                anonymousClassAnnotation.put(xMethod.getClassName(), annotations);
+            }
+        }
+
         if (seen == INVOKEVIRTUAL || seen == INVOKEINTERFACE || seen == INVOKESPECIAL || seen == INVOKESTATIC) {
 
             String sig = getSigConstantOperand();
@@ -1135,14 +1172,23 @@ public void report() {
                             priority = NORMAL_PRIORITY;
                         }
 
-                        String bug = "SIC_INNER_SHOULD_BE_STATIC";
+                        BugInstance bugInstance;
                         if (isAnonymousInnerClass) {
-                            bug = "SIC_INNER_SHOULD_BE_STATIC_ANON";
+                            bugInstance = new BugInstance(this, "SIC_INNER_SHOULD_BE_STATIC_ANON", priority);
+                            List<BugAnnotation> annotations = anonymousClassAnnotation.remove(f.getClassDescriptor().getDottedClassName());
+                            if(annotations != null) {
+                                bugInstance.addClass(className).describe(ClassAnnotation.ANONYMOUS_ROLE);
+                                bugInstance.addAnnotations(annotations);
+                            } else {
+                                bugInstance.addClass(className);
+                            }
                         } else if (!easyChange) {
-                            bug = "SIC_INNER_SHOULD_BE_STATIC_NEEDS_THIS";
+                            bugInstance = new BugInstance(this, "SIC_INNER_SHOULD_BE_STATIC_NEEDS_THIS", priority).addClass(className);
+                        } else {
+                            bugInstance = new BugInstance(this, "SIC_INNER_SHOULD_BE_STATIC", priority).addClass(className);
                         }
 
-                        bugReporter.reportBug(new BugInstance(this, bug, priority).addClass(className));
+                        bugReporter.reportBug(bugInstance);
 
                     }
                 }

diff --git a/findbugsTestCases/src/java/sfBugsNew/Bug1418.java b/findbugsTestCases/src/java/sfBugsNew/Bug1418.java
@@ -0,0 +1,47 @@
+package sfBugsNew;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import edu.umd.cs.findbugs.annotations.ExpectWarning;
+import edu.umd.cs.findbugs.annotations.NoWarning;
+
+public class Bug1418 {
+    @NoWarning("SIC")
+    @SuppressFBWarnings("SIC")
+    private final ThreadLocal<Integer> field=new ThreadLocal<Integer>() {
+        @Override 
+        protected Integer initialValue() {
+            return 0;
+        }
+    };
+
+    @NoWarning("SIC")
+    @SuppressFBWarnings("SIC")
+    public void test() {
+        ThreadLocal<Integer> local=new ThreadLocal<Integer>() {
+            @Override 
+            protected Integer initialValue() {
+                return 0;
+            }
+        };
+        System.out.println(local+"/"+field);
+    }
+
+    @ExpectWarning("SIC")
+    private final ThreadLocal<Integer> field2=new ThreadLocal<Integer>() {
+        @Override 
+        protected Integer initialValue() {
+            return 0;
+        }
+    };
+
+    @ExpectWarning("SIC")
+    public void test2() {
+        ThreadLocal<Integer> local=new ThreadLocal<Integer>() {
+            @Override 
+            protected Integer initialValue() {
+                return 0;
+            }
+        };
+        System.out.println(local+"/"+field2);
+    }
+}