Merge "Add lint detector for suspicious 0dp sizes"

4 files changed, 223 insertions, 1 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
index 22a6c9d..147100d 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
@@ -55,7 +55,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        final int initialCapacity = 121;
+        final int initialCapacity = 122;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -73,6 +73,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(InefficientWeightDetector.INEFFICIENT_WEIGHT);
         issues.add(InefficientWeightDetector.NESTED_WEIGHTS);
         issues.add(InefficientWeightDetector.BASELINE_WEIGHTS);
+        issues.add(InefficientWeightDetector.WRONG_0DP);
         issues.add(ScrollViewChildDetector.ISSUE);
         issues.add(DeprecationDetector.ISSUE);
         issues.add(ObsoleteLayoutParamsDetector.ISSUE);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java
index 98d1301..9c92c4b 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java
@@ -24,6 +24,8 @@ import static com.android.SdkConstants.ATTR_LAYOUT_WIDTH;
 import static com.android.SdkConstants.ATTR_ORIENTATION;
 import static com.android.SdkConstants.LINEAR_LAYOUT;
 import static com.android.SdkConstants.VALUE_VERTICAL;
+import static com.android.SdkConstants.VIEW;
+import static com.android.SdkConstants.VIEW_TAG;
 
 import com.android.annotations.NonNull;
 import com.android.tools.lint.detector.api.Category;
@@ -90,6 +92,25 @@ public class InefficientWeightDetector extends LayoutDetector {
             InefficientWeightDetector.class,
             Scope.RESOURCE_FILE_SCOPE);
 
+    /** Using 0dp on the wrong dimension */
+    public static final Issue WRONG_0DP = Issue.create(
+            "Suspicious0dp", //$NON-NLS-1$
+            "Looks for 0dp as the width in a vertical LinearLayout or as the height in a " +
+            "horizontal",
+
+            "Using 0dp as the width in a horizontal LinearLayout with weights is a useful " +
+            "trick to ensure that only the weights (and not the intrinsic sizes) are used " +
+            "when sizing the children.\n" +
+            "\n" +
+            "However, if you use 0dp for the opposite dimension, the view will be invisible. " +
+            "This can happen if you change the orientation of a layout without also flipping " +
+            "the 0dp dimension in all the children.",
+            Category.CORRECTNESS,
+            6,
+            Severity.ERROR,
+            InefficientWeightDetector.class,
+            Scope.RESOURCE_FILE_SCOPE);
+
     /**
      * Map from element to whether that element has a non-zero linear layout
      * weight or has an ancestor which does
@@ -184,5 +205,78 @@ public class InefficientWeightDetector extends LayoutDetector {
 
             }
         }
+
+        if (context.isEnabled(WRONG_0DP)) {
+            checkWrong0Dp(context, element, children);
+        }
+    }
+
+    private void checkWrong0Dp(XmlContext context, Element element, List<Element> children) {
+        boolean isVertical = false;
+        String orientation = element.getAttributeNS(ANDROID_URI, ATTR_ORIENTATION);
+        if (VALUE_VERTICAL.equals(orientation)) {
+            isVertical = true;
+        }
+
+        for (Element child : children) {
+            String tagName = child.getTagName();
+            if (tagName.equals(VIEW)) {
+                // Might just used for spacing
+                return;
+            }
+            if (tagName.indexOf('.') != -1 || tagName.equals(VIEW_TAG)) {
+                // Custom views might perform their own dynamic sizing or ignore the layout
+                // attributes all together
+                return;
+            }
+
+            boolean hasWeight = child.hasAttributeNS(ANDROID_URI, ATTR_LAYOUT_WEIGHT);
+
+            Attr widthNode = child.getAttributeNodeNS(ANDROID_URI, ATTR_LAYOUT_WIDTH);
+            Attr heightNode = child.getAttributeNodeNS(ANDROID_URI, ATTR_LAYOUT_HEIGHT);
+
+            boolean noWidth = false;
+            boolean noHeight = false;
+            if (widthNode != null && widthNode.getValue().startsWith("0")) { //$NON-NLS-1$
+                noWidth = true;
+            }
+            if (heightNode != null && heightNode.getValue().startsWith("0")) { //$NON-NLS-1$
+                noHeight = true;
+            } else if (!noWidth) {
+                return;
+            }
+
+            // If you're specifying 0dp for both the width and height you are probably
+            // trying to hide it deliberately
+            if (noWidth && noHeight) {
+                return;
+            }
+            assert noWidth || noHeight;
+
+            if (noWidth) {
+                assert widthNode != null;
+                if (!hasWeight) {
+                    context.report(WRONG_0DP, widthNode, context.getLocation(widthNode),
+                        "Suspicious size: this will make the view invisible, should be " +
+                        "used with layout_weight", null);
+                } else if (isVertical) {
+                    context.report(WRONG_0DP, widthNode, context.getLocation(widthNode),
+                        "Suspicious size: this will make the view invisible, probably " +
+                        "intended for layout_height", null);
+                }
+            } else {
+                assert noHeight;
+                assert heightNode != null;
+                if (!hasWeight) {
+                    context.report(WRONG_0DP, widthNode, context.getLocation(heightNode),
+                        "Suspicious size: this will make the view invisible, should be " +
+                        "used with layout_weight", null);
+                } else if (!isVertical) {
+                    context.report(WRONG_0DP, widthNode, context.getLocation(heightNode),
+                        "Suspicious size: this will make the view invisible, probably " +
+                        "intended for layout_width", null);
+                }
+            }
+        }
     }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java
index f7d4a44..ad0da87 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java
@@ -91,4 +91,28 @@ public class InefficientWeightDetectorTest extends AbstractCheckTest {
 
                 lintFiles("res/layout/nested_weights2.xml"));
     }
+
+    public void testWrong0Dp() throws Exception {
+        assertEquals(
+            "res/layout/wrong0dp.xml:19: Error: Suspicious size: this will make the view invisible, should be used with layout_weight [Suspicious0dp]\n" +
+            "            android:layout_width=\"0dp\"\n" +
+            "            ~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/wrong0dp.xml:25: Error: Suspicious size: this will make the view invisible, should be used with layout_weight [Suspicious0dp]\n" +
+            "            android:layout_height=\"0dp\"\n" +
+            "            ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/wrong0dp.xml:34: Error: Suspicious size: this will make the view invisible, probably intended for layout_height [Suspicious0dp]\n" +
+            "            android:layout_width=\"0dp\"\n" +
+            "            ~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/wrong0dp.xml:67: Error: Suspicious size: this will make the view invisible, probably intended for layout_width [Suspicious0dp]\n" +
+            "            android:layout_height=\"0dp\"\n" +
+            "            ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/wrong0dp.xml:90: Error: Suspicious size: this will make the view invisible, probably intended for layout_width [Suspicious0dp]\n" +
+            "            android:layout_height=\"0dp\"\n" +
+            "            ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "5 errors, 0 warnings\n",
+
+            lintFiles("res/layout/wrong0dp.xml"));
+    }
+
+
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml
new file mode 100644
index 0000000..136329f
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml
@@ -0,0 +1,103 @@
+<?xml version="1.0" encoding="utf-8"?>
+<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:orientation="vertical"
+    tools:ignore="HardcodedText" >
+
+    <!-- Vertical Layout -->
+
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:orientation="vertical" >
+
+        <!-- No weight: Always an error -->
+
+        <Button
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:text="Button" />
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="0dp"
+            android:text="Button" />
+
+        <!--
+             0dp not along the orientation axis is wrong;
+             here layout_height is okay, layout_width is not
+        -->
+
+        <Button
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:layout_weight="1.0"
+            android:text="Button" />
+
+        <!-- OK -->
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="0dp"
+            android:layout_weight="1.0"
+            android:text="Button" />
+    </LinearLayout>
+
+    <!-- Horizontal Layout -->
+
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:orientation="horizontal" >
+
+        <!-- OK -->
+
+        <Button
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:layout_weight="1.0"
+            android:text="Button" />
+
+        <!-- Not OK -->
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="0dp"
+            android:layout_weight="1.0"
+            android:text="Button" />
+    </LinearLayout>
+
+    <!-- No orientation specified, so horizontal -->
+
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" >
+
+        <!-- OK -->
+
+        <Button
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:layout_weight="1.0"
+            android:text="Button" />
+
+        <!-- Not OK -->
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="0dp"
+            android:layout_weight="1.0"
+            android:text="Button" />
+
+        <!-- Check suppressed -->
+
+        <Button
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:text="Button"
+            tools:ignore="Suspicious0dp" />
+    </LinearLayout>
+
+</FrameLayout>