From 10e5e0f5df59378a5ef9f553855588b9d2e936c5 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Mon, 5 Oct 2009 18:05:44 -0700 Subject: Add 3 more rules to layoutopt/uix: - InefficientWeight - NestedScrollingWidgets - TooManyChildren Change-Id: Ic8fe0b36e0a7cac523d223e5f8d96d7959919da6 --- .../com/android/layoutopt/uix/LayoutAnalysis.java | 22 +++++++++++++++++++++- .../uix/groovy/LayoutAnalysisCategory.java | 14 ++++++++++++++ .../uix/src/resources/rules/InefficientWeight.rule | 19 +++++++++++++++++++ .../resources/rules/NestedScrollingWidgets.rule | 19 +++++++++++++++++++ .../uix/src/resources/rules/TooManyChildren.rule | 15 +++++++++++++++ .../src/resources/rules/UseCompoundDrawables.rule | 6 +++--- .../uix/src/resources/rules/UselessLayout.rule | 8 +++++--- 7 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule create mode 100644 layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule create mode 100644 layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule (limited to 'layoutopt/libs/uix') 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 all(Element element) { + NodeList list = DOMCategory.depthFirst(element); + int count = list.getLength(); + List nodes = new ArrayList(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 " + - " and a compound drawable for the image"] + " 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 -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"}"] -- cgit v1.1