Merge "Detect incorrect state lists"

3 files changed, 101 insertions, 13 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StateListDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StateListDetector.java
index d7f0d99..c08b0c6 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StateListDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StateListDetector.java
@@ -23,6 +23,7 @@ import com.android.resources.ResourceFolderType;
 import com.android.tools.lint.detector.api.Category;
 import com.android.tools.lint.detector.api.Issue;
 import com.android.tools.lint.detector.api.LintUtils;
+import com.android.tools.lint.detector.api.Location;
 import com.android.tools.lint.detector.api.ResourceXmlDetector;
 import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
@@ -34,7 +35,11 @@ import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NamedNodeMap;
 
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * Checks for unreachable states in an Android state list definition
@@ -53,6 +58,8 @@ public class StateListDetector extends ResourceXmlDetector {
             StateListDetector.class,
             Scope.RESOURCE_FILE_SCOPE);
 
+    private static final String STATE_PREFIX = "state_"; //$NON-NLS-1$
+
     /** Constructs a new {@link StateListDetector} */
     public StateListDetector() {
     }
@@ -77,18 +84,23 @@ public class StateListDetector extends ResourceXmlDetector {
         Element root = document.getDocumentElement();
         if (root != null && root.getTagName().equals("selector")) { //$NON-NLS-1$
             List<Element> children = LintUtils.getChildren(root);
-            for (int i = 0; i < children.size() - 1; i++) {
+            Map<Element, Set<String>> states =
+                    new HashMap<Element, Set<String>>(children.size());
+
+            for (int i = 0; i < children.size(); i++) {
                 Element child = children.get(i);
-                boolean hasState = false;
                 NamedNodeMap attributes = child.getAttributes();
+                Set<String> stateNames = new HashSet<String>(attributes.getLength());
+                states.put(child, stateNames);
+
                 for (int j = 0; j < attributes.getLength(); j++) {
                     Attr attribute = (Attr) attributes.item(j);
-                    if (attribute.getLocalName() == null) {
+                    String name = attribute.getLocalName();
+                    if (name == null) {
                         continue;
                     }
-                    if (attribute.getLocalName().startsWith("state_")) {
-                        hasState = true;
-                        break;
+                    if (name.startsWith(STATE_PREFIX)) {
+                        stateNames.add(name + '=' + attribute.getValue());
                     } else {
                         String namespaceUri = attribute.getNamespaceURI();
                         if (namespaceUri != null && namespaceUri.length() > 0 &&
@@ -97,14 +109,34 @@ public class StateListDetector extends ResourceXmlDetector {
                             // This could be a state, see
                             //   http://code.google.com/p/android/issues/detail?id=22339
                             // so don't flag this one.
-                            hasState = true;
+                            stateNames.add(attribute.getName() + '=' + attribute.getValue());
                         }
                     }
                 }
-                if (!hasState) {
-                    context.report(ISSUE, child, context.getLocation(child),
-                        String.format("No android:state_ attribute found on <item> %1$d, later states not reachable",
-                                i), null);
+            }
+
+            // See if for each state, any subsequent state fully contains all the same
+            // state requirements
+
+            for (int i = 0; i < children.size() - 1; i++) {
+                Element prev = children.get(i);
+                Set<String> prevStates = states.get(prev);
+                assert prevStates != null : prev;
+                for (int j = i + 1; j < children.size(); j++) {
+                    Element current = children.get(j);
+                    Set<String> currentStates = states.get(current);
+                    assert currentStates != null : current;
+                    if (currentStates.containsAll(prevStates)) {
+                        Location location = context.getLocation(current);
+                        Location secondary = context.getLocation(prev);
+                        secondary.setMessage("Earlier item which masks item");
+                        location.setSecondary(secondary);
+                        context.report(ISSUE, current, location, String.format(
+                                "This item is unreachable because a previous item (item #%1$d) is a more general match than this one",
+                                i + 1),  null);
+                        // Don't keep reporting errors for all the remaining cases in this file
+                        return;
+                    }
                 }
             }
         }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StateListDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StateListDetectorTest.java
index b0994fe..9176773 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StateListDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StateListDetectorTest.java
@@ -27,8 +27,8 @@ public class StateListDetectorTest extends AbstractCheckTest {
 
     public void testStates() throws Exception {
         assertEquals(
-                "states.xml:2: Warning: No android:state_ attribute found on <item> 0, " +
-                        "later states not reachable",
+                "drawable/states.xml:3: Warning: This item is unreachable because a previous item (item #1) is a more general match than this one\n" +
+                "=> drawable/states.xml:2: Earlier item which masks item",
                 lintProject("res/drawable/states.xml"));
     }
 
@@ -37,4 +37,11 @@ public class StateListDetectorTest extends AbstractCheckTest {
                 "No warnings.",
                 lintProject("res/drawable/states2.xml"));
     }
+
+    public void testStates3() throws Exception {
+        assertEquals(
+                "drawable/states3.xml:24: Warning: This item is unreachable because a previous item (item #1) is a more general match than this one\n" +
+                "=> drawable/states3.xml:18: Earlier item which masks item",
+                lintProject("res/drawable/states3.xml"));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/states3.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/states3.xml
new file mode 100644
index 0000000..c3fc533
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/drawable/states3.xml
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2008 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.
+-->
+
+<selector xmlns:android="http://schemas.android.com/apk/res/android">
+    <item android:state_checked="false" android:state_window_focused="false"
+          android:drawable="@drawable/btn_star_big_off" />
+    <item android:state_checked="true" android:state_window_focused="false"
+          android:drawable="@drawable/btn_star_big_on" />
+    <item android:state_checked="true" android:state_window_focused="false"
+          android:state_enabled="false" android:drawable="@drawable/btn_star_big_on_disable" />
+    <item android:state_checked="false" android:state_window_focused="false"
+          android:state_enabled="false" android:drawable="@drawable/btn_star_big_off_disable" />
+
+    <item android:state_checked="true" android:state_pressed="true"
+          android:drawable="@drawable/btn_star_big_on_pressed" />
+    <item android:state_checked="false" android:state_pressed="true"
+          android:drawable="@drawable/btn_star_big_off_pressed" />
+
+    <item android:state_checked="true" android:state_focused="true"
+          android:drawable="@drawable/btn_star_big_on_selected" />
+    <item android:state_checked="false" android:state_focused="true"
+          android:drawable="@drawable/btn_star_big_off_selected" />
+
+    <item android:state_checked="true" android:state_focused="true" android:state_enabled="false"
+          android:drawable="@drawable/btn_star_big_on_disable_focused" />
+    <item android:state_checked="true" android:state_focused="false" android:state_enabled="false"
+          android:drawable="@drawable/btn_star_big_on_disable" />
+
+    <item android:state_checked="false" android:state_focused="true" android:state_enabled="false"
+          android:drawable="@drawable/btn_star_big_off_disable_focused" />
+    <item android:state_checked="false" android:state_focused="false" android:state_enabled="false"
+          android:drawable="@drawable/btn_star_big_off_disable" />
+
+    <item android:state_checked="false" android:drawable="@drawable/btn_star_big_off" />
+    <item android:state_checked="true" android:drawable="@drawable/btn_star_big_on" />
+</selector>
\ No newline at end of file