Merge "Fix handling of @SuppressLint on fields"

2 files changed, 58 insertions, 10 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
index 7f6c0ee..fc9487d 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java
@@ -60,8 +60,10 @@ import com.google.common.io.Closeables;
 
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.AnnotationNode;
 import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.FieldInsnNode;
 import org.objectweb.asm.tree.FieldNode;
 import org.objectweb.asm.tree.MethodNode;
 import org.w3c.dom.Attr;
@@ -1800,27 +1802,62 @@ public class LintDriver {
     // pointers, so we have to have multiple methods which pass in each type
     // of node (class, method, field) to be checked.
 
-    // TODO: The Quickfix should look for lint warnings placed *inside* warnings
-    // and warn that they won't apply to checks that are bytecode oriented!
-
     /**
      * Returns whether the given issue is suppressed in the given method.
      *
      * @param issue the issue to be checked, or null to just check for "all"
+     * @param classNode the class containing the issue
      * @param method the method containing the issue
+     * @param instruction the instruction within the method, if any
      * @return true if there is a suppress annotation covering the specific
      *         issue on this method
      */
-    public boolean isSuppressed(@Nullable Issue issue, @NonNull MethodNode method) {
+    public boolean isSuppressed(
+            @Nullable Issue issue,
+            @NonNull ClassNode classNode,
+            @NonNull MethodNode method,
+            @Nullable AbstractInsnNode instruction) {
         if (method.invisibleAnnotations != null) {
             @SuppressWarnings("unchecked")
             List<AnnotationNode> annotations = method.invisibleAnnotations;
             return isSuppressed(issue, annotations);
         }
 
+        // Initializations of fields end up placed in generated methods (<init>
+        // for members and <clinit> for static fields).
+        if (instruction != null && method.name.charAt(0) == '<') {
+            AbstractInsnNode next = LintUtils.getNextInstruction(instruction);
+            if (next != null && next.getType() == AbstractInsnNode.FIELD_INSN) {
+                FieldInsnNode fieldRef = (FieldInsnNode) next;
+                FieldNode field = findField(classNode, fieldRef.owner, fieldRef.name);
+                if (field != null && isSuppressed(issue, field)) {
+                    return true;
+                }
+            }
+        }
+
         return false;
     }
 
+    @Nullable
+    private FieldNode findField(ClassNode classNode, String owner, String name) {
+        while (classNode != null) {
+            if (owner.equals(classNode.name)) {
+                @SuppressWarnings("rawtypes") // ASM API
+                List fieldList = classNode.fields;
+                for (Object f : fieldList) {
+                    FieldNode field = (FieldNode) f;
+                    if (field.name.equals(name)) {
+                        return field;
+                    }
+                }
+                return null;
+            }
+            classNode = getOuterClassNode(classNode);
+        }
+        return null;
+    }
+
     /**
      * Returns whether the given issue is suppressed for the given field.
      *
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
index 68c25d9..159180d 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
@@ -280,7 +280,7 @@ public class ClassContext extends Context {
      * Detectors should only call this method if an error applies to the whole class
      * scope and there is no specific method or field that applies to the error.
      * If so, use
-     * {@link #report(Issue, MethodNode, Location, String, Object)} or
+     * {@link #report(Issue, MethodNode, AbstractInsnNode, Location, String, Object)} or
      * {@link #report(Issue, FieldNode, Location, String, Object)}, such that
      * suppress annotations are checked.
      *
@@ -313,7 +313,8 @@ public class ClassContext extends Context {
                             // Found the outer method for this anonymous class; continue
                             // reporting on it (which will also work its way up the parent
                             // class hierarchy)
-                            if (method != null && mDriver.isSuppressed(issue, method)) {
+                            if (method != null && mDriver.isSuppressed(issue, mClassNode, method,
+                                    null)) {
                                 return;
                             }
                             break;
@@ -337,9 +338,18 @@ public class ClassContext extends Context {
      * Reports an issue applicable to a given method node.
      *
      * @param issue the issue to report
-     * @param method the method scope the error applies to. The lint infrastructure
-     *    will check whether there are suppress annotations on this method (or its enclosing
-     *    class) and if so suppress the warning without involving the client.
+     * @param method the method scope the error applies to. The lint
+     *            infrastructure will check whether there are suppress
+     *            annotations on this method (or its enclosing class) and if so
+     *            suppress the warning without involving the client.
+     * @param instruction the instruction within the method the error applies
+     *            to. You cannot place annotations on individual method
+     *            instructions (for example, annotations on local variables are
+     *            allowed, but are not kept in the .class file). However, this
+     *            instruction is needed to handle suppressing errors on field
+     *            initializations; in that case, the errors may be reported in
+     *            the {@code <clinit>} method, but the annotation is found not
+     *            on that method but for the {@link FieldNode}'s.
      * @param location the location of the issue, or null if not known
      * @param message the message for this warning
      * @param data any associated data, or null
@@ -347,10 +357,11 @@ public class ClassContext extends Context {
     public void report(
             @NonNull Issue issue,
             @Nullable MethodNode method,
+            @Nullable AbstractInsnNode instruction,
             @Nullable Location location,
             @NonNull String message,
             @Nullable Object data) {
-        if (method != null && mDriver.isSuppressed(issue, method)) {
+        if (method != null && mDriver.isSuppressed(issue, mClassNode, method, instruction)) {
             return;
         }
         report(issue, location, message, data); // also checks the class node