Add 3 more rules to layoutopt/uix:

- InefficientWeight
- NestedScrollingWidgets
- TooManyChildren

Change-Id: Ic8fe0b36e0a7cac523d223e5f8d96d7959919da6

7 files changed, 96 insertions, 7 deletions

diff --git a/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java b/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java
index 852fd60..c3da141 100644
--- a/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java
+++ b/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java
@@ -61,6 +61,15 @@ public class LayoutAnalysis {
     void setName(String name) {
         mName = name;
     }
+    
+    /**
+     * Adds an issue to the layout analysis.
+     *
+     * @param issue The issue to add.
+     */
+    public void addIssue(Issue issue) {
+        mIssues.add(issue);
+    }
 
     /**
      * Adds an issue to the layout analysis.
@@ -116,7 +125,12 @@ public class LayoutAnalysis {
         private final String mDescription;
         private final LayoutNode mNode;
 
-        Issue(String description) {
+        /**
+         * Creates a new issue with the specified description.
+         * 
+         * @param description The description of the issue.
+         */
+        public Issue(String description) {
             mNode = null;
             if (description == null) {
                 throw new IllegalArgumentException("The description must be non-null");
@@ -124,6 +138,12 @@ public class LayoutAnalysis {
             mDescription = description;
         }
 
+        /**
+         * Creates a new issue with the specified description.
+         * 
+         * @param node The node in which the issue was found.
+         * @param description The description of the issue.
+         */
         public Issue(LayoutNode node, String description) {
             mNode = node;
             if (description == null) {
diff --git a/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java b/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java
index 6f84d5c..bc5da6d 100644
--- a/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java
+++ b/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java
@@ -20,16 +20,30 @@ import com.android.layoutopt.uix.LayoutAnalysis;
 import com.android.layoutopt.uix.LayoutNode;
 
 import java.util.Map;
+import java.util.List;
+import java.util.ArrayList;
 
 import groovy.lang.GString;
+import groovy.xml.dom.DOMCategory;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
+import org.w3c.dom.Element;
 
 /**
  * Support class for Groovy rules. This class adds new Groovy capabilities
  * to {@link com.android.layoutopt.uix.LayoutAnalysis} and {@link org.w3c.dom.Node}.
  */
 public class LayoutAnalysisCategory {
+    public static List<Node> all(Element element) {
+        NodeList list = DOMCategory.depthFirst(element);
+        int count = list.getLength();
+        List<Node> nodes = new ArrayList<Node>(count - 1);
+        for (int i = 1; i < count; i++) {
+            nodes.add(list.item(i));
+        }
+        return nodes;
+    }
+    
     /**
      * xmlNode.isRoot()
      */
diff --git a/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule b/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule
new file mode 100644
index 0000000..ea93b1b
--- /dev/null
+++ b/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule
@@ -0,0 +1,19 @@
+// Rule: InefficientWeight
+//
+// Description: Checks whether a layout_weight is declared inefficiently.
+//
+// Conditions:
+// - The node has a LinearLayout parent
+// - The node is the only sibling with a weight
+// - The node has a height/width != 0
+
+def parent = xml.'..'
+if (parent.is("LinearLayout") && xml.'@android:layout_weight' &&
+        parent.'*'.findAll{ it.'@android:layout_weight' }.size() == 1) {
+    def dimension = parent.'@android:orientation' == "vertical" ?
+        "android:layout_height" : "android:layout_width"
+    if (xml."@${dimension}"[0] != 0) {
+        analysis << [node: node, description: "Use an ${dimension} of 0dip instead of " +
+                "${xml."@${dimension}"} for better performance"]
+    }
+}
diff --git a/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule b/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule
new file mode 100644
index 0000000..15d54bc
--- /dev/null
+++ b/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule
@@ -0,0 +1,19 @@
+// Rule: NestedScrollingWidgets
+//
+// Description: Checks whether a scrolling widget has nested scrolling widgets.
+//
+// Conditions:
+// - The node is a scrolling widget
+// - The node has a descendant who is also a scrolling widget
+
+def widgets = ["ScrollView", "ListView", "GridView"]
+if (xml.name() in widgets && xml.all().any{ it.name() in widgets }) {
+    analysis << [node: node, description: "The vertically scrolling ${xml.name()} should " +
+            "not contain another vertically scrolling widget"]
+}
+
+widgets = ["HorizontalScrollView", "Gallery"]
+if (xml.name() in widgets && xml.all().any{ it.name() in widgets }) {
+    analysis << [node: node, description: "The horizontally scrolling ${xml.name()} should " +
+            "not contain another horizontally scrolling widget"]
+}
diff --git a/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule b/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule
new file mode 100644
index 0000000..134dbd5
--- /dev/null
+++ b/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule
@@ -0,0 +1,15 @@
+// Rule: TooManyChildren
+//
+// Description: Checks whether the layout has too many children.
+//
+// Conditions:
+// - The layout is a ScrollView and has more than 1 child
+// - The layout is a list or grid ans has at least 1 child
+
+if (xml.name() in ["ScrollView", "HorizontalScrollView"] && xml.'*'.size() > 1) {
+    analysis << [node: node, description: "A scroll view can have only one child"]
+}
+
+if (xml.name() in ["ListView", "GridView"] && xml.'*'.size() > 0) {
+    analysis << [node: node, description: "A list/grid should have no children declared in XML"]
+}
diff --git a/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule b/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule
index d2421e6..ceaf9a0 100644
--- a/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule
+++ b/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule
@@ -8,8 +8,8 @@
 // - The node has two children, ImageView and TextView
 // - The ImageView does not have a weight
 
-if (xml.is("LinearLayout") && xml['*'].size() == 2 && xml.'TextView'.size() == 1 &&
-        xml.'ImageView'.size() == 1 && !xml.'ImageView'.'@android:layout_weight') {
+if (xml.is("LinearLayout") && xml.'*'.size() == 2 && xml.'TextView'.size() == 1 &&
+        xml.'ImageView'.size() == 1 && !xml.'ImageView'[0].'@android:layout_weight') {
     analysis << [node: node, description: "This tag and its children can be replaced by one " +
-            "<TextView/> and a compound drawable for the image"]
+            "<TextView/> and a compound drawable"]
 }
diff --git a/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule b/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule
index d9bd6f0..70381b0 100644
--- a/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule
+++ b/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule
@@ -5,13 +5,15 @@
 // Conditions:
 // - The node has children
 // - The node does not have siblings
+// - The node's parent is not a scroll view (horizontal or vertical)
 // - The node does not have a background or its parent does not have a
 //   background or neither the node and its parent have a background
 // - The parent is not a <merge/>
 
-if (!xml.isRoot() && xml['..'].name() != "merge" && xml['..']['*'].size() == 1 &&
-        xml['*'].size() > 0 && ((xml.'@android:background' || xml['..'].'@android:background') ||
-        (!xml.'@android:background' && !xml['..'].'@android:background'))) {
+if (!xml.isRoot() && !(xml['..'].name() in ["ScrollView", "HorizontalScrollView"]) &&
+        xml['..']['*'].size() == 1 && xml['*'].size() > 0 && ((xml.'@android:background' ||
+        xml['..'].'@android:background') || (!xml.'@android:background' &&
+        !xml['..'].'@android:background'))) {
     analysis << [node: node, description: "This ${xml.name()} layout or " +
             "its ${xml['..'].name()} parent is " +
             "${xml['..'].'@android:id' ? "possibly useless" : "useless"}"]