Add lint rule to catch incorrect setColor calls

Android has various setColor methods (such as setTextColor) which take
an integer, where it expects RGB values in the bytes. Since this is
an integer, and since color resources are integers, sometimes code
incorrectly passes the color resource id rather than a resolved color
to the setter:

     paint.setColor(R.color.red)

Obviously, the color should be "resolved" first via
getResource().getColor().

This changeset adds a lint detector which catches these kinds of bugs.

Change-Id: I2970e5220d24ad3b844a9f19fbe99b932d9f7fb4

10 files changed, 199 insertions, 17 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java
index 930f7a7..f8d6cd9 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java
@@ -197,7 +197,7 @@ class DefaultSdkInfo extends SdkInfo {
         PARENTS.put("MultiAutoCompleteTextView",          //$NON-NLS-1$
                 "AutoCompleteTextView");                  //$NON-NLS-1$
 
-        assert PARENTS.size() == CLASS_COUNT : PARENTS.size();
+        assert PARENTS.size() <= CLASS_COUNT : PARENTS.size();
 
         /*
         // Check that all widgets lead to the root view
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 63b51b5..fcf58f4 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
@@ -16,6 +16,9 @@
 
 package com.android.tools.lint.client.api;
 
+import static com.android.tools.lint.detector.api.LintConstants.ANDROID_PKG;
+import static com.android.tools.lint.detector.api.LintConstants.R_CLASS;
+
 import com.android.annotations.NonNull;
 import com.android.tools.lint.detector.api.Detector;
 import com.android.tools.lint.detector.api.Detector.JavaScanner;
@@ -1112,18 +1115,36 @@ public class JavaVisitor {
         @Override
         public boolean visitVariableReference(@NonNull VariableReference node) {
             if (mVisitResources) {
-                if (node.astIdentifier().getDescription().equals("R") && //$NON-NLS-1$
+                String identifier = node.astIdentifier().getDescription();
+                if (identifier.equals(R_CLASS) &&
                         node.getParent() instanceof Select &&
                         node.getParent().getParent() instanceof Select) {
-                    for (VisitingDetector v : mResourceFieldDetectors) {
-                        Select parentSelect = (Select) node.getParent();
-                        Select grandParentSelect = (Select) parentSelect.getParent();
-                        String type = parentSelect.astIdentifier().astValue();
-                        String name = grandParentSelect.astIdentifier().astValue();
+                    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);
+                                type, name, false /* isFramework */);
+                    }
+                } 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 */);
+                        }
                     }
                 }
             }
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 b0942f2..82ac76a 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
@@ -182,13 +182,16 @@ public abstract class Detector {
          * @param type the resource type, such as "layout" or "string"
          * @param name the resource name, such as "main" from
          *            {@code R.layout.main}
+         * @param isFramework whether the resource is a framework resource
+         *            (android.R) or a local project resource (R)
          */
         void visitResourceReference(
                 @NonNull JavaContext context,
                 @Nullable AstVisitor visitor,
                 @NonNull VariableReference node,
                 @NonNull String type,
-                @NonNull String name);
+                @NonNull String name,
+                boolean isFramework);
     }
 
     /** Specialized interface for detectors that scan Java class files */
@@ -441,7 +444,7 @@ public abstract class Detector {
 
     @SuppressWarnings("javadoc")
     public void visitResourceReference(@NonNull JavaContext context, @Nullable AstVisitor visitor,
-            @NonNull VariableReference node, @NonNull String type, @NonNull String name) {
+            @NonNull VariableReference node, @NonNull String type, @NonNull String name, boolean isFramework) {
     }
 
     // ---- Dummy implementations to make implementing a ClassScanner easier: ----
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
index b3eb1b7..cb47f92 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
@@ -36,6 +36,8 @@ public class LintConstants {
     /** Default prefix used for tools attributes */
     public static final String TOOLS_PREFIX = "tools";                 //$NON-NLS-1$
     public static final String XMLNS_PREFIX = "xmlns:";                //$NON-NLS-1$
+    public static final String R_CLASS = "R";                          //$NON-NLS-1$
+    public static final String ANDROID_PKG = "android";                //$NON-NLS-1$
 
     // Tags: Manifest
     public static final String TAG_SERVICE = "service";                //$NON-NLS-1$
@@ -265,6 +267,7 @@ public class LintConstants {
     public static final String STYLE_RESOURCE_PREFIX = "@style/";       //$NON-NLS-1$
     public static final String STRING_RESOURCE_PREFIX = "@string/";     //$NON-NLS-1$
     public static final String RESOURCE_CLZ_ID = "id";                  //$NON-NLS-1$
+    public static final String RESOURCE_CLZ_COLOR = "color";            //$NON-NLS-1$
     public static final String RESOURCE_CLZ_ARRAY = "array";            //$NON-NLS-1$
     public static final String RESOURCE_CLZ_ATTR = "attr";              //$NON-NLS-1$
     public static final String RESOURCE_CLR_STYLEABLE = "styleable";    //$NON-NLS-1$
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
index 6f8f19b..258a267 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
@@ -53,7 +53,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        final int initialCapacity = 80;
+        final int initialCapacity = 81;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -134,6 +134,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(NamespaceDetector.UNUSED);
         issues.add(NamespaceDetector.TYPO);
         issues.add(AlwaysShowActionDetector.ISSUE);
+        issues.add(ColorUsageDetector.ISSUE);
         issues.add(JavaPerformanceDetector.PAINT_ALLOC);
         issues.add(JavaPerformanceDetector.USE_SPARSEARRAY);
 
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
new file mode 100644
index 0000000..f380784
--- /dev/null
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ColorUsageDetector.java
@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.tools.lint.checks;
+
+import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLZ_COLOR;
+
+import com.android.tools.lint.detector.api.Category;
+import com.android.tools.lint.detector.api.Context;
+import com.android.tools.lint.detector.api.Detector;
+import com.android.tools.lint.detector.api.Issue;
+import com.android.tools.lint.detector.api.JavaContext;
+import com.android.tools.lint.detector.api.Scope;
+import com.android.tools.lint.detector.api.Severity;
+import com.android.tools.lint.detector.api.Speed;
+
+import java.io.File;
+
+import lombok.ast.AstVisitor;
+import lombok.ast.MethodInvocation;
+import lombok.ast.Node;
+import lombok.ast.VariableReference;
+
+/**
+ * Looks for cases where the code attempts to set a resource id, rather than
+ * a resolved color, as the RGB int.
+ */
+public class ColorUsageDetector extends Detector implements Detector.JavaScanner {
+    /** Attempting to set a resource id as a color */
+    public static final Issue ISSUE = Issue.create(
+            "ResourceAsColor", //$NON-NLS-1$
+            "Looks for calls to setColor where a resource id is passed instead of a " +
+            "resolved color",
+
+            "Methods that take a color in the form of an integer should be passed " +
+            "an RGB triple, not the actual color resource id. You must call " +
+            "getResources().getColor(resource) to resolve the actual color value first.",
+
+            Category.PERFORMANCE,
+            7,
+            Severity.ERROR,
+            ColorUsageDetector.class,
+            Scope.JAVA_FILE_SCOPE);
+
+    /** Constructs a new {@link ColorUsageDetector} check */
+    public ColorUsageDetector() {
+    }
+
+    @Override
+    public boolean appliesTo(Context context, File file) {
+        return true;
+    }
+
+    @Override
+    public Speed getSpeed() {
+        return Speed.FAST;
+    }
+
+    // ---- Implements JavaScanner ----
+
+    @Override
+    public boolean appliesToResourceRefs() {
+        return true;
+    }
+
+    @Override
+    public void visitResourceReference(JavaContext context, AstVisitor visitor,
+            VariableReference node, 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) {
+                select = select.getParent();
+            }
+            if (select.getParent() instanceof MethodInvocation) {
+                MethodInvocation call = (MethodInvocation) select.getParent();
+                String methodName = call.astName().astValue();
+                if (methodName.endsWith("Color")              //$NON-NLS-1$
+                        && methodName.startsWith("set")) {    //$NON-NLS-1$
+                    context.report(
+                            ISSUE, context.getLocation(select), String.format(
+                                "Should pass resolved color instead of resource id here: " +
+                                "getResources().getColor(%1$s)", select.toString()),
+                            null);
+                }
+            }
+        }
+    }
+}
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
index 5ed783a..3cd3a5c 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
@@ -415,7 +415,7 @@ public class TranslationDetector extends ResourceXmlDetector {
                 return;
             }
             String name = attribute.getValue();
-            if (mMissingLocations.containsKey(name)) {
+            if (mMissingLocations != null && mMissingLocations.containsKey(name)) {
                 String language = getLanguage(context.file.getParentFile().getName());
                 if (language == null) {
                     if (context.getDriver().isSuppressed(MISSING, element)) {
@@ -429,7 +429,7 @@ public class TranslationDetector extends ResourceXmlDetector {
                     mMissingLocations.put(name, location);
                 }
             }
-            if (mExtraLocations.containsKey(name)) {
+            if (mExtraLocations != null && mExtraLocations.containsKey(name)) {
                 if (context.getDriver().isSuppressed(EXTRA, element)) {
                     mExtraLocations.remove(name);
                     return;
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 b2ad4d6..a5ae165 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
@@ -28,6 +28,7 @@ import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLZ_ARR
 import static com.android.tools.lint.detector.api.LintConstants.RESOURCE_CLZ_ID;
 import static com.android.tools.lint.detector.api.LintConstants.RES_FOLDER;
 import static com.android.tools.lint.detector.api.LintConstants.R_ATTR_PREFIX;
+import static com.android.tools.lint.detector.api.LintConstants.R_CLASS;
 import static com.android.tools.lint.detector.api.LintConstants.R_ID_PREFIX;
 import static com.android.tools.lint.detector.api.LintConstants.R_PREFIX;
 import static com.android.tools.lint.detector.api.LintConstants.TAG_ARRAY;
@@ -434,8 +435,8 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
 
     @Override
     public void visitResourceReference(JavaContext context, AstVisitor visitor,
-            VariableReference node, String type, String name) {
-        if (mReferences != null) {
+            VariableReference node, String type, String name, boolean isFramework) {
+        if (mReferences != null && !isFramework) {
             String reference = R_PREFIX + type + '.' + name;
             mReferences.add(reference);
         }
@@ -457,7 +458,7 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
         // and store them in mReferences
         @Override
         public boolean visitVariableReference(VariableReference node) {
-            if (node.astIdentifier().getDescription().equals("R") &&
+            if (node.astIdentifier().getDescription().equals(R_CLASS) &&
                     node.getParent() instanceof Select &&
                     node.getParent().getParent() instanceof Select) {
                 String reference = node.getParent().getParent().toString();
@@ -472,7 +473,7 @@ public class UnusedResourceDetector extends ResourceXmlDetector implements Detec
             // Look for declarations of R class fields and store them in
             // mDeclarations
             String description = node.getDescription();
-            if (description.equals("R")) { //$NON-NLS-1$
+            if (description.equals(R_CLASS)) {
                 // This is an R class. We can process this class very deliberately.
                 // The R class has a very specific AST format:
                 // ClassDeclaration ("R")
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ColorUsageDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ColorUsageDetectorTest.java
new file mode 100644
index 0000000..be60027
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ColorUsageDetectorTest.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Eclipse Public License, Version 1.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.eclipse.org/org/documents/epl-v10.php
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.tools.lint.checks;
+
+import com.android.tools.lint.detector.api.Detector;
+
+@SuppressWarnings("javadoc")
+public class ColorUsageDetectorTest extends AbstractCheckTest {
+    @Override
+    protected Detector getDetector() {
+        return new ColorUsageDetector();
+    }
+
+    public void test() throws Exception {
+        assertEquals(
+            "WrongColor.java:11: Error: Should pass resolved color instead of resource id here: getResources().getColor(R.color.red)\n" +
+            "WrongColor.java:12: Error: Should pass resolved color instead of resource id here: getResources().getColor(android.R.color.red)\n" +
+            "WrongColor.java:9: Error: Should pass resolved color instead of resource id here: getResources().getColor(R.color.blue)",
+
+            lintProject("src/test/pkg/WrongColor.java.txt=>src/test/pkg/WrongColor.java"));
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongColor.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongColor.java.txt
new file mode 100644
index 0000000..cacd834
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongColor.java.txt
@@ -0,0 +1,16 @@
+package test.pkg;
+
+import android.view.*;
+import android.widget.*;
+
+public class WrongColor {
+    public void foo(TextView textView) {
+        Paint paint2 = new Paint();
+        paint2.setColor(R.color.blue);
+        // Wrong
+        textView.setTextColor(R.color.red);
+        textView.setTextColor(android.R.color.red);
+        // OK
+        textView.setTextColor(getResources().getColor(R.color.red));
+    }
+}