From 1e2cbdfa84b6f1db8c9dc5bde77022003558bf6f Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Tue, 6 Oct 2009 11:37:46 -0700 Subject: [PATCH] New rules for layoutopt: IncorrectHeight/WidthIn[Horizontal]ScrollView, UselessView. This change also refactors the uix library to remove an unnecessary class (LayoutNode.) The refactoring helps writing cleaner script by accessing only the node object instead of xml and node. This change also augment the capabilities of the node in Groovy scripts. Change-Id: Id7515f9a79826909834d82496a6d9dfbc19988ad --- .../android/layoutopt/uix/LayoutAnalysis.java | 33 +++- .../android/layoutopt/uix/LayoutAnalyzer.java | 4 +- .../com/android/layoutopt/uix/LayoutNode.java | 184 ------------------ .../uix/groovy/LayoutAnalysisCategory.java | 108 +++++++++- .../layoutopt/uix/rules/GroovyRule.java | 4 +- .../com/android/layoutopt/uix/rules/Rule.java | 6 +- .../rules/IncorrectHeightInScrollView.rule | 11 ++ .../IncorrectWidthInHorizontalScrollView.rule | 11 ++ .../resources/rules/InefficientWeight.rule | 10 +- .../resources/rules/MergeRootFrameLayout.rule | 9 +- .../rules/NestedScrollingWidgets.rule | 12 +- .../src/resources/rules/TooManyChildren.rule | 8 +- .../src/resources/rules/TooManyLevels.rule | 5 +- .../uix/src/resources/rules/TooManyViews.rule | 5 +- .../resources/rules/UseCompoundDrawables.rule | 8 +- .../src/resources/rules/UselessLayout.rule | 13 +- .../uix/src/resources/rules/UselessView.rule | 14 ++ tools/layoutopt/samples/wrong_dimension.xml | 13 ++ 18 files changed, 220 insertions(+), 238 deletions(-) delete mode 100644 tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutNode.java create mode 100644 tools/layoutopt/libs/uix/src/resources/rules/IncorrectHeightInScrollView.rule create mode 100644 tools/layoutopt/libs/uix/src/resources/rules/IncorrectWidthInHorizontalScrollView.rule create mode 100644 tools/layoutopt/libs/uix/src/resources/rules/UselessView.rule create mode 100644 tools/layoutopt/samples/wrong_dimension.xml diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java index c3da141ca..bc6f0b6e0 100644 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java +++ b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalysis.java @@ -16,9 +16,13 @@ package com.android.layoutopt.uix; +import org.w3c.dom.Node; + import java.util.List; import java.util.ArrayList; +import com.android.layoutopt.uix.groovy.LayoutAnalysisCategory; + /** * Contains the results of a layout analysis. Instances of this class are * generated by {@link com.android.layoutopt.uix.LayoutAnalyzer}. @@ -40,6 +44,7 @@ public class LayoutAnalysis { private final List mIssues = new ArrayList(); private String mName; private boolean mAnalyzed; + private Node mNode; /** * Creates a new analysis. An analysis is always considered invalid by default. @@ -77,16 +82,16 @@ public class LayoutAnalysis { * @param description Description of the issue. */ public void addIssue(String description) { - mIssues.add(new Issue(description)); + mIssues.add(new Issue(mNode, description)); } /** * Adds an issue to the layout analysis. * - * @param node The layout node containing the issue. + * @param node The node containing the issue. * @param description Description of the issue. */ - public void addIssue(LayoutNode node, String description) { + public void addIssue(Node node, String description) { mIssues.add(new Issue(node, description)); } @@ -111,10 +116,22 @@ public class LayoutAnalysis { /** * Validates the analysis. This must be call before this analysis can - * be considered valid. + * be considered valid. Calling this method resets the current node to null. + * + * @see #setCurrentNode(org.w3c.dom.Node) */ void validate() { mAnalyzed = true; + mNode = null; + } + + /** + * Sets the current node to be automatically added to created issues. + * + * @param node An XML node. + */ + void setCurrentNode(Node node) { + mNode = node; } /** @@ -123,7 +140,7 @@ public class LayoutAnalysis { */ public static class Issue { private final String mDescription; - private final LayoutNode mNode; + private final Node mNode; /** * Creates a new issue with the specified description. @@ -144,7 +161,7 @@ public class LayoutAnalysis { * @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(Node node, String description) { mNode = node; if (description == null) { throw new IllegalArgumentException("The description must be non-null"); @@ -168,7 +185,7 @@ public class LayoutAnalysis { * @return The start line or -1 if the line is unknown. */ public int getStartLine() { - return mNode == null ? -1 : mNode.getStartLine(); + return LayoutAnalysisCategory.getStartLine(mNode); } /** @@ -177,7 +194,7 @@ public class LayoutAnalysis { * @return The end line or -1 if the line is unknown. */ public int getEndLine() { - return mNode == null ? -1 : mNode.getEndLine(); + return LayoutAnalysisCategory.getEndLine(mNode); } } } diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalyzer.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalyzer.java index c130782b4..da274145c 100644 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalyzer.java +++ b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutAnalyzer.java @@ -241,9 +241,9 @@ public class LayoutAnalyzer { } private void applyRules(LayoutAnalysis analysis, Node node) { - LayoutNode layoutNode = new LayoutNode(node); + analysis.setCurrentNode(node); for (Rule rule : mRules) { - rule.run(analysis, layoutNode, node); + rule.run(analysis, node); } } } diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutNode.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutNode.java deleted file mode 100644 index 6f0a078a8..000000000 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/LayoutNode.java +++ /dev/null @@ -1,184 +0,0 @@ -/* - * Copyright (C) 2009 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.layoutopt.uix; - -import org.w3c.dom.Node; -import org.w3c.dom.Element; -import org.w3c.dom.NamedNodeMap; -import org.w3c.dom.Attr; -import org.w3c.dom.NodeList; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.HashMap; - -import com.android.layoutopt.uix.xml.XmlDocumentBuilder; - -/** - * Wrapper class for W3C Node objects. Provides extra utilities specific - * to Android XML layouts. - */ -public class LayoutNode { - private static final String ANDROID_LAYOUT_WIDTH = "android:layout_width"; - private static final String ANDROID_LAYOUT_HEIGHT = "android:layout_height"; - private static final String VALUE_FILL_PARENT = "fill_parent"; - private static final String VALUE_WRAP_CONTENT = "wrap_content"; - - private Map mAttributes; - private final Element mNode; - private LayoutNode[] mChildren; - - LayoutNode(Node node) { - if (node == null) throw new IllegalArgumentException("The node cannot be null"); - if (node.getNodeType() != Node.ELEMENT_NODE) { - throw new IllegalArgumentException("The node must be an element type"); - } - mNode = (Element) node; - } - - /** - * Returns the start line of this node. - * - * @return The start line or -1 if the line is unknown. - */ - public int getStartLine() { - final Object data = mNode.getUserData(XmlDocumentBuilder.NODE_START_LINE); - return data == null ? -1 : (Integer) data; - } - - /** - * Returns the end line of this node. - * - * @return The end line or -1 if the line is unknown. - */ - public int getEndLine() { - final Object data = mNode.getUserData(XmlDocumentBuilder.NODE_END_LINE); - return data == null ? -1 : (Integer) data; - } - - /** - * Returns the wrapped W3C XML node object. - * - * @return An XML node. - */ - public Node getNode() { - return mNode; - } - - /** - * Indicates whether the node is of the specified type. - * - * @param name The name of the node. - * - * @return True if this node has the same name as tagName, false otherwise. - */ - public boolean is(String name) { - return mNode.getNodeName().equals(name); - } - - /** - * Indicates whether the node has declared the specified attribute. - * - * @param attribute The name of the attribute to check. - * - * @return True if the attribute is specified, false otherwise. - */ - public boolean has(String attribute) { - return mNode.hasAttribute(attribute); - } - - /** - * Returns whether this node is the document root. - * - * @return True if the wrapped node is the root of the document, - * false otherwise. - */ - public boolean isRoot() { - return mNode == mNode.getOwnerDocument().getDocumentElement(); - } - - /** - * Returns whether this node's width is fill_parent. - */ - public boolean isWidthFillParent() { - return mNode.getAttribute(ANDROID_LAYOUT_WIDTH).equals(VALUE_FILL_PARENT); - } - - /** - * Returns whether this node's width is wrap_content. - */ - public boolean isWidthWrapContent() { - return mNode.getAttribute(ANDROID_LAYOUT_WIDTH).equals(VALUE_WRAP_CONTENT); - } - - /** - * Returns whether this node's height is fill_parent. - */ - public boolean isHeightFillParent() { - return mNode.getAttribute(ANDROID_LAYOUT_HEIGHT).equals(VALUE_FILL_PARENT); - } - - /** - * Returns whether this node's height is wrap_content. - */ - public boolean isHeightWrapContent() { - return mNode.getAttribute(ANDROID_LAYOUT_HEIGHT).equals(VALUE_WRAP_CONTENT); - } - - /** - * Returns a map of all the attributes declared for this node. - * - * The name of the attributes contains the namespace. - * - * @return A map of [name, value] describing the attributes of this node. - */ - public Map getAttributes() { - if (mAttributes == null) { - NamedNodeMap attributes = mNode.getAttributes(); - int count = attributes.getLength(); - mAttributes = new HashMap(count); - - for (int i = 0; i < count; i++) { - Node node = attributes.item(i); - Attr attribute = (Attr) node; - mAttributes.put(attribute.getName(), attribute.getValue()); - } - } - - return mAttributes; - } - - /** - * Returns all the children of this node. - */ - public LayoutNode[] getChildren() { - if (mChildren == null) { - NodeList list = mNode.getChildNodes(); - int count = list.getLength(); - List children = new ArrayList(count); - for (int i = 0; i < count; i++) { - Node child = list.item(i); - if (child.getNodeType() == Node.ELEMENT_NODE) { - children.add(new LayoutNode(child)); - } - } - mChildren = children.toArray(new LayoutNode[children.size()]); - } - return mChildren; - } -} diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java index bc5da6d3c..e2d8c3536 100644 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java +++ b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/groovy/LayoutAnalysisCategory.java @@ -17,11 +17,12 @@ package com.android.layoutopt.uix.groovy; import com.android.layoutopt.uix.LayoutAnalysis; -import com.android.layoutopt.uix.LayoutNode; +import com.android.layoutopt.uix.xml.XmlDocumentBuilder; import java.util.Map; import java.util.List; import java.util.ArrayList; +import java.util.Arrays; import groovy.lang.GString; import groovy.xml.dom.DOMCategory; @@ -34,6 +35,38 @@ import org.w3c.dom.Element; * to {@link com.android.layoutopt.uix.LayoutAnalysis} and {@link org.w3c.dom.Node}. */ public class LayoutAnalysisCategory { + private static final String ANDROID_LAYOUT_WIDTH = "android:layout_width"; + private static final String ANDROID_LAYOUT_HEIGHT = "android:layout_height"; + private static final String VALUE_FILL_PARENT = "fill_parent"; + private static final String VALUE_WRAP_CONTENT = "wrap_content"; + + private static final String[] sContainers = new String[] { + "FrameLayout", "LinearLayout", "RelativeLayout", "SlidingDrawer", + "AbsoluteLayout", "TableLayout", "Gallery", "GridView", "ListView", + "RadioGroup", "ScrollView", "HorizontalScrollView", "Spinner", + "ViewSwitcher", "ViewFlipper", "ViewAnimator", "ImageSwitcher", + "TextSwitcher", "android.gesture.GestureOverlayView", "TabHost" + }; + static { + Arrays.sort(sContainers); + } + + /** + * xmlNode.isContainer() + * + * @return True if the specified node corresponds to a container widget. + */ + public static boolean isContainer(Element element) { + return Arrays.binarySearch(sContainers, element.getNodeName()) >= 0; + } + + /** + * xmlNode.all() + * + * Same as xmlNode.'**' but excludes xmlNode from the results. + * + * @return All descendants, this node excluded. + */ public static List all(Element element) { NodeList list = DOMCategory.depthFirst(element); int count = list.getLength(); @@ -43,9 +76,62 @@ public class LayoutAnalysisCategory { } return nodes; } + + /** + * Returns the start line of this node. + * + * @return The start line or -1 if the line is unknown. + */ + public static int getStartLine(Node node) { + final Object data = node == null ? null : + node.getUserData(XmlDocumentBuilder.NODE_START_LINE); + return data == null ? -1 : (Integer) data; + } + + /** + * Returns the end line of this node. + * + * @return The end line or -1 if the line is unknown. + */ + public static int getEndLine(Node node) { + final Object data = node == null ? null : + node.getUserData(XmlDocumentBuilder.NODE_END_LINE); + return data == null ? -1 : (Integer) data; + } + + /** + * Returns whether this node's width is fill_parent. + */ + public static boolean isWidthFillParent(Element element) { + return element.getAttribute(ANDROID_LAYOUT_WIDTH).equals(VALUE_FILL_PARENT); + } + + /** + * Returns whether this node's width is wrap_content. + */ + public static boolean isWidthWrapContent(Element element) { + return element.getAttribute(ANDROID_LAYOUT_WIDTH).equals(VALUE_WRAP_CONTENT); + } + + /** + * Returns whether this node's height is fill_parent. + */ + public static boolean isHeightFillParent(Element element) { + return element.getAttribute(ANDROID_LAYOUT_HEIGHT).equals(VALUE_FILL_PARENT); + } + + /** + * Returns whether this node's height is wrap_content. + */ + public static boolean isHeightWrapContent(Element element) { + return element.getAttribute(ANDROID_LAYOUT_HEIGHT).equals(VALUE_WRAP_CONTENT); + } + /** * xmlNode.isRoot() + * + * @return True if xmlNode is the root of the document, false otherwise */ public static boolean isRoot(Node node) { return node.getOwnerDocument().getDocumentElement() == node; @@ -53,6 +139,8 @@ public class LayoutAnalysisCategory { /** * xmlNode.is("tagName") + * + * @return True if xmlNode.getNodeName().equals(name), false otherwise. */ public static boolean is(Node node, String name) { return node.getNodeName().equals(name); @@ -60,6 +148,8 @@ public class LayoutAnalysisCategory { /** * xmlNode.depth() + * + * @return The maximum depth of the node. */ public static int depth(Node node) { int maxDepth = 0; @@ -75,17 +165,31 @@ public class LayoutAnalysisCategory { /** * analysis << "The issue" + * + * @return The analysis itself to chain calls. */ public static LayoutAnalysis leftShift(LayoutAnalysis analysis, GString description) { analysis.addIssue(description.toString()); return analysis; } + /** + * analysis << "The issue" + * + * @return The analysis itself to chain calls. + */ + public static LayoutAnalysis leftShift(LayoutAnalysis analysis, String description) { + analysis.addIssue(description); + return analysis; + } + /** * analysis << [node: node, description: "The issue"] + * + * @return The analysis itself to chain calls. */ public static LayoutAnalysis leftShift(LayoutAnalysis analysis, Map issue) { - analysis.addIssue((LayoutNode) issue.get("node"), issue.get("description").toString()); + analysis.addIssue((Node) issue.get("node"), issue.get("description").toString()); return analysis; } } diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/GroovyRule.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/GroovyRule.java index 85d60ef56..785b8f448 100644 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/GroovyRule.java +++ b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/GroovyRule.java @@ -21,7 +21,6 @@ import groovy.lang.Binding; import groovy.lang.Closure; import groovy.xml.dom.DOMCategory; import com.android.layoutopt.uix.LayoutAnalysis; -import com.android.layoutopt.uix.LayoutNode; import com.android.layoutopt.uix.groovy.LayoutAnalysisCategory; import org.w3c.dom.Node; import org.codehaus.groovy.runtime.GroovyCategorySupport; @@ -59,10 +58,9 @@ public class GroovyRule implements Rule { return mName; } - public void run(LayoutAnalysis analysis, LayoutNode node, Node xml) { + public void run(LayoutAnalysis analysis, Node node) { mBinding.setVariable("analysis", analysis); mBinding.setVariable("node", node); - mBinding.setVariable("xml", xml); GroovyCategorySupport.use(mCategories, mClosure); } diff --git a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/Rule.java b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/Rule.java index 3112499f1..0168992fb 100644 --- a/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/Rule.java +++ b/tools/layoutopt/libs/uix/src/com/android/layoutopt/uix/rules/Rule.java @@ -17,7 +17,6 @@ package com.android.layoutopt.uix.rules; import com.android.layoutopt.uix.LayoutAnalysis; -import com.android.layoutopt.uix.LayoutNode; import org.w3c.dom.Node; /** @@ -36,8 +35,7 @@ public interface Rule { * issue to the analysis. * * @param analysis The resulting analysis. - * @param node The layout node to analyse. - * @param xml The original XML node. + * @param node The original XML node. */ - void run(LayoutAnalysis analysis, LayoutNode node, Node xml); + void run(LayoutAnalysis analysis, Node node); } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/IncorrectHeightInScrollView.rule b/tools/layoutopt/libs/uix/src/resources/rules/IncorrectHeightInScrollView.rule new file mode 100644 index 000000000..d29ebd5f0 --- /dev/null +++ b/tools/layoutopt/libs/uix/src/resources/rules/IncorrectHeightInScrollView.rule @@ -0,0 +1,11 @@ +// Rule: IncorrectHeightInScrollView +// +// Description: Checks whether a scrollview's child has the wrong dimension. +// +// Conditions: +// - The node has a ScrollView parent +// - The node has a height set to fill_parent + +if (node.'..'.is("ScrollView") && node.isHeightFillParent()) { + analysis << "This ${node.name()} tag should use android:layout_height=\"wrap_content\"" +} diff --git a/tools/layoutopt/libs/uix/src/resources/rules/IncorrectWidthInHorizontalScrollView.rule b/tools/layoutopt/libs/uix/src/resources/rules/IncorrectWidthInHorizontalScrollView.rule new file mode 100644 index 000000000..17da843bd --- /dev/null +++ b/tools/layoutopt/libs/uix/src/resources/rules/IncorrectWidthInHorizontalScrollView.rule @@ -0,0 +1,11 @@ +// Rule: IncorrectWidthInScrollView +// +// Description: Checks whether a scrollview's child has the wrong dimension. +// +// Conditions: +// - The node has a HorizontalScrollView parent +// - The node has a width set to fill_parent + +if (node.'..'.is("HorizontalScrollView") && node.isWidthFillParent()) { + analysis << "This ${node.name()} tag should use android:layout_width=\"wrap_content\"" +} diff --git a/tools/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule b/tools/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule index ea93b1bb9..0de935072 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/InefficientWeight.rule @@ -7,13 +7,13 @@ // - 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' && +def parent = node.'..' +if (parent.is("LinearLayout") && node.'@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"] + if (node."@${dimension}"[0] != 0) { + analysis << "Use an ${dimension} of 0dip instead of ${node."@${dimension}"} " + + "for better performance" } } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/MergeRootFrameLayout.rule b/tools/layoutopt/libs/uix/src/resources/rules/MergeRootFrameLayout.rule index f92c2b1ca..2221036ae 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/MergeRootFrameLayout.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/MergeRootFrameLayout.rule @@ -9,9 +9,8 @@ // - The node is fill_parent in both orientation *or* it has no layout_gravity // - The node does not have a background nor a foreground -if (xml.isRoot() && xml.is("FrameLayout") && !xml.'@android:background' && - !xml.'@android:foreground' && ((node.isWidthFillParent() && - node.isHeightFillParent()) || !xml.'@android:layout_gravity')) { - analysis << [node: node, description: "The root-level can be " + - "replaced with "] +if (node.isRoot() && node.is("FrameLayout") && !node.'@android:background' && + !node.'@android:foreground' && ((node.isWidthFillParent() && + node.isHeightFillParent()) || !node.'@android:layout_gravity')) { + analysis << "The root-level can be replaced with " } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule b/tools/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule index 15d54bc5d..f48c24e3e 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/NestedScrollingWidgets.rule @@ -7,13 +7,13 @@ // - 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"] +if (node.name() in widgets && node.all().any{ it.name() in widgets }) { + analysis << "The vertically scrolling ${node.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"] +if (node.name() in widgets && node.all().any{ it.name() in widgets }) { + analysis << "The horizontally scrolling ${node.name()} should not contain another " + + "horizontally scrolling widget" } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule b/tools/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule index 134dbd5c7..516d91fd7 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/TooManyChildren.rule @@ -6,10 +6,10 @@ // - 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 (node.name() in ["ScrollView", "HorizontalScrollView"] && node.'*'.size() > 1) { + analysis << "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"] +if (node.name() in ["ListView", "GridView"] && node.'*'.size() > 0) { + analysis << "A list/grid should have no children declared in node" } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/TooManyLevels.rule b/tools/layoutopt/libs/uix/src/resources/rules/TooManyLevels.rule index da16c5f75..f04e08210 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/TooManyLevels.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/TooManyLevels.rule @@ -5,6 +5,7 @@ // Conditions: // - The depth of the layout is > 10 -if (xml.isRoot() && (depth = xml.depth()) > 10) { - analysis << "This layout has too many nested layouts: ${depth} levels, it should have <= 10!" +if (node.isRoot() && (depth = node.depth()) > 10) { + analysis << [node: null, description: "This layout has too many nested layouts: " + + "${depth} levels, it should have <= 10!"] } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/TooManyViews.rule b/tools/layoutopt/libs/uix/src/resources/rules/TooManyViews.rule index 30553e58b..544a888ac 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/TooManyViews.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/TooManyViews.rule @@ -5,6 +5,7 @@ // Conditions: // - The document contains more than 80 views -if (xml.isRoot && (size = xml.'**'.size()) > 80) { - analysis << "This layout has too many views: ${size} views, it should have <= 80!" +if (node.isRoot && (size = node.'**'.size()) > 80) { + analysis << [node: null, + description: "This layout has too many views: ${size} views, it should have <= 80!"] } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule b/tools/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule index ceaf9a0ab..4e4a6baab 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/UseCompoundDrawables.rule +++ b/tools/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'[0].'@android:layout_weight') { - analysis << [node: node, description: "This tag and its children can be replaced by one " + - " and a compound drawable"] +if (node.is("LinearLayout") && node.'*'.size() == 2 && node.'TextView'.size() == 1 && + node.'ImageView'.size() == 1 && !node.'ImageView'[0].'@android:layout_weight') { + analysis << "This tag and its children can be replaced by one and " + + "a compound drawable" } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule b/tools/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule index 70381b0f3..9326333dd 100644 --- a/tools/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule +++ b/tools/layoutopt/libs/uix/src/resources/rules/UselessLayout.rule @@ -10,11 +10,10 @@ // background or neither the node and its parent have a background // - The parent is not a -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"}"] +if (!node.isRoot() && !(node['..'].name() in ["ScrollView", "HorizontalScrollView"]) && + node['..']['*'].size() == 1 && node['*'].size() > 0 && ((node.'@android:background' || + node['..'].'@android:background') || (!node.'@android:background' && + !node['..'].'@android:background'))) { + analysis << "This ${node.name()} layout or its ${node['..'].name()} parent is " + + "${node['..'].'@android:id' ? "possibly useless" : "useless"}" } diff --git a/tools/layoutopt/libs/uix/src/resources/rules/UselessView.rule b/tools/layoutopt/libs/uix/src/resources/rules/UselessView.rule new file mode 100644 index 000000000..c3a77e5ff --- /dev/null +++ b/tools/layoutopt/libs/uix/src/resources/rules/UselessView.rule @@ -0,0 +1,14 @@ +// Rule: UselessView +// +// Description: Checks whether a container view can be removed. +// +// Conditions: +// - The node is a container view (LinearLayout, etc.) +// - The node has no id +// - The node has no background +// - The node has no children + +if (node.isContainer() && node.'*'.size() == 0 && !node.'@android:id' && + !node.'@android:background') { + analysis << "This ${node.name()} view is useless (no children, no background, no id)" +} diff --git a/tools/layoutopt/samples/wrong_dimension.xml b/tools/layoutopt/samples/wrong_dimension.xml new file mode 100644 index 000000000..c0b292b74 --- /dev/null +++ b/tools/layoutopt/samples/wrong_dimension.xml @@ -0,0 +1,13 @@ + + + + + + +