Add 3 more rules to layoutopt/uix:

- InefficientWeight
- NestedScrollingWidgets
- TooManyChildren

Change-Id: Ic8fe0b36e0a7cac523d223e5f8d96d7959919da6
This commit is contained in:
Romain Guy
2009-10-05 18:05:44 -07:00
parent b6e85c0548
commit 4796120e05
11 changed files with 166 additions and 22 deletions

View File

@@ -61,6 +61,15 @@ public class LayoutAnalysis {
void setName(String name) { void setName(String name) {
mName = 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. * Adds an issue to the layout analysis.
@@ -116,7 +125,12 @@ public class LayoutAnalysis {
private final String mDescription; private final String mDescription;
private final LayoutNode mNode; 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; mNode = null;
if (description == null) { if (description == null) {
throw new IllegalArgumentException("The description must be non-null"); throw new IllegalArgumentException("The description must be non-null");
@@ -124,6 +138,12 @@ public class LayoutAnalysis {
mDescription = description; 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) { public Issue(LayoutNode node, String description) {
mNode = node; mNode = node;
if (description == null) { if (description == null) {

View File

@@ -20,16 +20,30 @@ import com.android.layoutopt.uix.LayoutAnalysis;
import com.android.layoutopt.uix.LayoutNode; import com.android.layoutopt.uix.LayoutNode;
import java.util.Map; import java.util.Map;
import java.util.List;
import java.util.ArrayList;
import groovy.lang.GString; import groovy.lang.GString;
import groovy.xml.dom.DOMCategory;
import org.w3c.dom.Node; import org.w3c.dom.Node;
import org.w3c.dom.NodeList; import org.w3c.dom.NodeList;
import org.w3c.dom.Element;
/** /**
* Support class for Groovy rules. This class adds new Groovy capabilities * Support class for Groovy rules. This class adds new Groovy capabilities
* to {@link com.android.layoutopt.uix.LayoutAnalysis} and {@link org.w3c.dom.Node}. * to {@link com.android.layoutopt.uix.LayoutAnalysis} and {@link org.w3c.dom.Node}.
*/ */
public class LayoutAnalysisCategory { 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() * xmlNode.isRoot()
*/ */

View File

@@ -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"]
}
}

View File

@@ -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"]
}

View File

@@ -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"]
}

View File

@@ -8,8 +8,8 @@
// - The node has two children, ImageView and TextView // - The node has two children, ImageView and TextView
// - The ImageView does not have a weight // - The ImageView does not have a weight
if (xml.is("LinearLayout") && xml['*'].size() == 2 && xml.'TextView'.size() == 1 && if (xml.is("LinearLayout") && xml.'*'.size() == 2 && xml.'TextView'.size() == 1 &&
xml.'ImageView'.size() == 1 && !xml.'ImageView'.'@android:layout_weight') { xml.'ImageView'.size() == 1 && !xml.'ImageView'[0].'@android:layout_weight') {
analysis << [node: node, description: "This tag and its children can be replaced by one " + 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"]
} }

View File

@@ -5,13 +5,15 @@
// Conditions: // Conditions:
// - The node has children // - The node has children
// - The node does not have siblings // - 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 // - 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 // background or neither the node and its parent have a background
// - The parent is not a <merge/> // - The parent is not a <merge/>
if (!xml.isRoot() && xml['..'].name() != "merge" && xml['..']['*'].size() == 1 && if (!xml.isRoot() && !(xml['..'].name() in ["ScrollView", "HorizontalScrollView"]) &&
xml['*'].size() > 0 && ((xml.'@android:background' || xml['..'].'@android:background') || xml['..']['*'].size() == 1 && xml['*'].size() > 0 && ((xml.'@android:background' ||
(!xml.'@android:background' && !xml['..'].'@android:background'))) { xml['..'].'@android:background') || (!xml.'@android:background' &&
!xml['..'].'@android:background'))) {
analysis << [node: node, description: "This ${xml.name()} layout or " + analysis << [node: node, description: "This ${xml.name()} layout or " +
"its ${xml['..'].name()} parent is " + "its ${xml['..'].name()} parent is " +
"${xml['..'].'@android:id' ? "possibly useless" : "useless"}"] "${xml['..'].'@android:id' ? "possibly useless" : "useless"}"]

View File

@@ -1,23 +1,17 @@
<?xml version="1.0" encoding="utf-8"?> <?xml version="1.0" encoding="utf-8"?>
<FrameLayout <LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android" xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent" android:layout_width="fill_parent"
android:layout_height="fill_parent"> android:layout_height="fill_parent">
<LinearLayout <ImageView
android:layout_width="fill_parent" android:layout_width="wrap_content"
android:layout_height="fill_parent"> android:layout_height="wrap_content" />
<ImageView
android:layout_width="wrap_content"
android:layout_height="wrap_content" />
<TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content" />
</LinearLayout>
</FrameLayout> <TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content" />
</LinearLayout>

View File

@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="utf-8"?>
<ListView
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent"
android:layout_height="fill_parent">
<ListView
android:layout_width="fill_parent"
android:layout_height="fill_parent" />
</ListView>

View File

@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent"
android:layout_height="fill_parent">
<Button
android:layout_width="fill_parent"
android:layout_height="wrap_content"
android:layout_weight="1.0" />
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent"
android:layout_height="fill_parent"
android:orientation="vertical">
<Button
android:layout_width="fill_parent"
android:layout_height="wrap_content"
android:layout_weight="1.0" />
</LinearLayout>
</LinearLayout>

View File

@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<ScrollView
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent"
android:layout_height="fill_parent">
<LinearLayout
android:layout_width="fill_parent"
android:layout_height="fill_parent">
<ListView
android:layout_width="fill_parent"
android:layout_height="fill_parent" />
</LinearLayout>
</ScrollView>