Merge "Fix lint issues 27108, 27109 and 27110"

11 files changed, 132 insertions, 56 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/JavaVisitor.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/JavaVisitor.java
index fc7e03b..1dbf915 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/JavaVisitor.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/JavaVisitor.java
@@ -65,6 +65,7 @@ import lombok.ast.EmptyStatement;
 import lombok.ast.EnumConstant;
 import lombok.ast.EnumDeclaration;
 import lombok.ast.EnumTypeBody;
+import lombok.ast.Expression;
 import lombok.ast.ExpressionStatement;
 import lombok.ast.FloatingPointLiteral;
 import lombok.ast.For;
@@ -1114,43 +1115,60 @@ public class JavaVisitor {
         }
 
         @Override
-        public boolean visitVariableReference(@NonNull VariableReference node) {
+        public boolean visitSelect(Select node) {
             if (mVisitResources) {
-                String identifier = node.astIdentifier().getDescription();
-                if (identifier.equals(R_CLASS) &&
-                        node.getParent() instanceof Select &&
-                        node.getParent().getParent() instanceof Select) {
-                    Select parentSelect = (Select) node.getParent();
-                    Select grandParentSelect = (Select) parentSelect.getParent();
-                    String type = parentSelect.astIdentifier().astValue();
-                    String name = grandParentSelect.astIdentifier().astValue();
-
-                    for (VisitingDetector v : mResourceFieldDetectors) {
-                        JavaScanner detector = v.getJavaScanner();
-                        detector.visitResourceReference(mContext, v.getVisitor(), node,
-                                type, name, false /* isFramework */);
+                // R.type.name
+                if (node.astOperand() instanceof Select) {
+                    Select select = (Select) node.astOperand();
+                    if (select.astOperand() instanceof VariableReference) {
+                        VariableReference reference = (VariableReference) select.astOperand();
+                        if (reference.astIdentifier().astValue().equals(R_CLASS)) {
+                            String type = select.astIdentifier().astValue();
+                            String name = node.astIdentifier().astValue();
+
+                            // R -could- be referenced locally and really have been
+                            // imported as "import android.R;" in the import statements,
+                            // but this is not recommended (and in fact there's a specific
+                            // lint rule warning against it)
+                            boolean isFramework = false;
+
+                            for (VisitingDetector v : mResourceFieldDetectors) {
+                                JavaScanner detector = v.getJavaScanner();
+                                detector.visitResourceReference(mContext, v.getVisitor(),
+                                        node, type, name, isFramework);
+                            }
+
+                            return super.visitSelect(node);
+                        }
                     }
-                } else if (identifier.equals(ANDROID_PKG)
-                            && node.getParent() instanceof Select) {
-                    Select parentSelect = (Select) node.getParent();
-                    if (parentSelect.astIdentifier().astValue().equals(R_CLASS)
-                            && parentSelect.getParent() instanceof Select
-                            && parentSelect.getParent().getParent() instanceof Select) {
-                        Select p2 = (Select) parentSelect.getParent();
-                        Select p3 = (Select) p2.getParent();
-                        String type = p2.astIdentifier().astValue();
-                        String name = p3.astIdentifier().astValue();
-
-                        for (VisitingDetector v : mResourceFieldDetectors) {
-                            JavaScanner detector = v.getJavaScanner();
-                            detector.visitResourceReference(mContext, v.getVisitor(), node,
-                                    type, name, true /* isFramework */);
+                }
+
+                // Arbitrary packages -- android.R.type.name, foo.bar.R.type.name
+                if (node.astIdentifier().astValue().equals(R_CLASS)) {
+                    Node parent = node.getParent();
+                    if (parent instanceof Select) {
+                        Node grandParent = parent.getParent();
+                        if (grandParent instanceof Select) {
+                            Select select = (Select) grandParent;
+                            String name = select.astIdentifier().astValue();
+                            Expression typeOperand = select.astOperand();
+                            if (typeOperand instanceof Select) {
+                                Select typeSelect = (Select) typeOperand;
+                                String type = typeSelect.astIdentifier().astValue();
+                                boolean isFramework =
+                                        node.astIdentifier().astValue().equals(ANDROID_PKG);
+                                for (VisitingDetector v : mResourceFieldDetectors) {
+                                    JavaScanner detector = v.getJavaScanner();
+                                    detector.visitResourceReference(mContext, v.getVisitor(),
+                                            node, type, name, isFramework);
+                                }
+                            }
                         }
                     }
                 }
             }
 
-            return super.visitVariableReference(node);
+            return super.visitSelect(node);
         }
 
         @Override
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Detector.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Detector.java
index 82ac76a..c795606 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Detector.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Detector.java
@@ -33,7 +33,6 @@ import java.util.List;
 
 import lombok.ast.AstVisitor;
 import lombok.ast.MethodInvocation;
-import lombok.ast.VariableReference;
 
 /**
  * A detector is able to find a particular problem. It might also be thought of as enforcing
@@ -188,7 +187,7 @@ public abstract class Detector {
         void visitResourceReference(
                 @NonNull JavaContext context,
                 @Nullable AstVisitor visitor,
-                @NonNull VariableReference node,
+                @NonNull lombok.ast.Node node,
                 @NonNull String type,
                 @NonNull String name,
                 boolean isFramework);
@@ -444,7 +443,8 @@ public abstract class Detector {
 
     @SuppressWarnings("javadoc")
     public void visitResourceReference(@NonNull JavaContext context, @Nullable AstVisitor visitor,
-            @NonNull VariableReference node, @NonNull String type, @NonNull String name, boolean isFramework) {
+            @NonNull lombok.ast.Node node, @NonNull String type, @NonNull String name,
+            boolean isFramework) {
     }
 
     // ---- Dummy implementations to make implementing a ClassScanner easier: ----
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ColorUsageDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ColorUsageDetector.java
index 07a1f03..e6d077a 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ColorUsageDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ColorUsageDetector.java
@@ -32,7 +32,7 @@ import java.io.File;
 import lombok.ast.AstVisitor;
 import lombok.ast.MethodInvocation;
 import lombok.ast.Node;
-import lombok.ast.VariableReference;
+import lombok.ast.Select;
 
 /**
  * Looks for cases where the code attempts to set a resource id, rather than
@@ -78,13 +78,13 @@ public class ColorUsageDetector extends Detector implements Detector.JavaScanner
 
     @Override
     public void visitResourceReference(JavaContext context, AstVisitor visitor,
-            VariableReference node, String type, String name, boolean isFramework) {
+            Node select, String type, String name, boolean isFramework) {
         if (type.equals(RESOURCE_CLZ_COLOR)) {
-            // See if this method is being called on a setter
-            Node select = node.getParent().getParent();
-            if (isFramework) {
+            while (select.getParent() instanceof Select) {
                 select = select.getParent();
             }
+
+            // See if this method is being called on a setter
             if (select.getParent() instanceof MethodInvocation) {
                 MethodInvocation call = (MethodInvocation) select.getParent();
                 String methodName = call.astName().astValue();
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java
index b810f47..5fae363 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java
@@ -368,6 +368,10 @@ public class StringFormatDetector extends ResourceXmlDetector implements Detecto
 
                     index = matcher.end(); // Ensure loop proceeds
                     String str = formatString.substring(matchStart, matcher.end());
+                    if (str.equals("%%")) { //$NON-NLS-1$
+                        // Just an escaped %
+                        continue;
+                    }
 
                     if (checkValid) {
                         // Make sure it's a valid format string
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java
index 49fdab3..3ea1c14 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UnusedResourceDetector.java
@@ -74,10 +74,8 @@ import lombok.ast.AstVisitor;
 import lombok.ast.ClassDeclaration;
 import lombok.ast.ForwardingAstVisitor;
 import lombok.ast.NormalTypeBody;
-import lombok.ast.Select;
 import lombok.ast.VariableDeclaration;
 import lombok.ast.VariableDefinition;
-import lombok.ast.VariableReference;
 
 /**
  * Finds unused resources.
@@ -147,7 +145,8 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
         File file = context.file;
 
         String fileName = file.getName();
-        if (endsWith(fileName, DOT_XML)
+        boolean isXmlFile = endsWith(fileName, DOT_XML);
+        if (isXmlFile
                 || endsWith(fileName, DOT_PNG)
                 || endsWith(fileName, DOT_JPG)
                 || endsWith(fileName, DOT_GIF)) {
@@ -163,6 +162,24 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
                 } else {
                     assert context.getPhase() == 2;
                     if (mUnused.containsKey(resource)) {
+                        // Check whether this is an XML document that has a tools:ignore attribute
+                        // on the document element: if so don't record it as a declaration.
+                        if (isXmlFile && context instanceof XmlContext) {
+                            XmlContext xmlContext = (XmlContext) context;
+                            if (xmlContext.document != null
+                                    && xmlContext.document.getDocumentElement() != null) {
+                                Element root = xmlContext.document.getDocumentElement();
+                                if (xmlContext.getDriver().isSuppressed(ISSUE, root)) {
+                                    //  Also remove it from consideration such that even the
+                                    // presence of this field in the R file is ignored.
+                                    if (mUnused != null) {
+                                        mUnused.remove(resource);
+                                    }
+                                    return;
+                                }
+                            }
+                        }
+
                         recordLocation(resource, Location.create(file));
                     }
                 }
@@ -436,7 +453,7 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
 
     @Override
     public void visitResourceReference(JavaContext context, AstVisitor visitor,
-            VariableReference node, String type, String name, boolean isFramework) {
+            lombok.ast.Node node, String type, String name, boolean isFramework) {
         if (mReferences != null && !isFramework) {
             String reference = R_PREFIX + type + '.' + name;
             mReferences.add(reference);
@@ -455,20 +472,6 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
 
     // Look for references and declarations
     private class UnusedResourceVisitor extends ForwardingAstVisitor {
-        // Look for references to field R.<class>.<name>
-        // and store them in mReferences
-        @Override
-        public boolean visitVariableReference(VariableReference node) {
-            if (node.astIdentifier().getDescription().equals(R_CLASS) &&
-                    node.getParent() instanceof Select &&
-                    node.getParent().getParent() instanceof Select) {
-                String reference = node.getParent().getParent().toString();
-                mReferences.add(reference);
-            }
-
-            return false;
-        }
-
         @Override
         public boolean visitClassDeclaration(ClassDeclaration node) {
             // Look for declarations of R class fields and store them in
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java
index 28cc268..7dda069 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java
@@ -117,5 +117,10 @@ public class StringFormatDetectorTest  extends AbstractCheckTest {
                 ));
     }
 
+    public void testIssue27108() throws Exception {
+        assertEquals(
+            "No warnings.",
 
+            lintProject("res/values/formatstrings3.xml"));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
index 691cb60..ccdebf7 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UnusedResourceDetectorTest.java
@@ -121,4 +121,24 @@ public class UnusedResourceDetectorTest extends AbstractCheckTest {
                 "multiproject/strings.xml=>../LibraryProject/res/values/strings.xml"
             ));
     }
+
+    public void testFqcnReference() throws Exception {
+        assertEquals(
+           "No warnings.",
+
+            lintProject(
+                "res/layout/layout1.xml=>res/layout/main.xml",
+                "src/test/pkg/UnusedReference.java.txt=>src/test/pkg/UnusedReference.java",
+                "AndroidManifest.xml"));
+    }
+
+    public void testIgnoreXmlDrawable() throws Exception {
+        assertEquals(
+           "No warnings.",
+
+            lintProject(
+                    "res/drawable/ic_menu_help.xml",
+                    "gen/my/pkg/R2.java.txt=>gen/my/pkg/R.java"
+            ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/gen/my/pkg/R2.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/gen/my/pkg/R2.java.txt
new file mode 100644
index 0000000..ed8fd7d
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/gen/my/pkg/R2.java.txt
@@ -0,0 +1,5 @@
+public final class R {
+    public static final class drawable {
+        public static final int ic_menu_help=0x7f020000;
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/ic_menu_help.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/ic_menu_help.xml
new file mode 100644
index 0000000..41f840f
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/ic_menu_help.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>
+<bitmap xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:src="@android:drawable/ic_menu_help"
+    tools:ignore="UnusedResources" />
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/formatstrings3.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/formatstrings3.xml
new file mode 100644
index 0000000..2424078
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/formatstrings3.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+    <string name="multiple_formats_with_percentage">%1$s 3%% %2$s</string>
+</resources>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/UnusedReference.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/UnusedReference.java.txt
new file mode 100644
index 0000000..5d7cb87
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/UnusedReference.java.txt
@@ -0,0 +1,12 @@
+package test.pkg;
+
+import android.app.Activity;
+import android.os.Bundle;
+
+public class UnusedReference extends Activity {
+    @Override
+    public void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        setContentView(test.pkg.R.layout.main);
+    }
+}