Add lint check ensuring that buttons in bars are borderless

Change-Id: Ie99e3e7b75f1ac8cf6447c70afc5901437e2d600

5 files changed, 188 insertions, 4 deletions

diff --git a/common/src/com/android/SdkConstants.java b/common/src/com/android/SdkConstants.java
index 04cd6f0..c0ed9c7 100644
--- a/common/src/com/android/SdkConstants.java
+++ b/common/src/com/android/SdkConstants.java
@@ -805,6 +805,8 @@ public final class SdkConstants {
     public static final String VALUE_TRUE = "true";                     //$NON-NLS-1$
     public static final String VALUE_EDITABLE = "editable";             //$NON-NLS-1$
     public static final String VALUE_AUTO_FIT = "auto_fit";             //$NON-NLS-1$
+    public static final String VALUE_SELECTABLE_ITEM_BACKGROUND =
+            "?android:attr/selectableItemBackground";                   //$NON-NLS-1$
 
 
     // Values: Resources
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 a475ac6..52c89d4 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 = 130;
+        final int initialCapacity = 131;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -159,6 +159,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(ButtonDetector.ORDER);
         issues.add(ButtonDetector.CASE);
         issues.add(ButtonDetector.BACKBUTTON);
+        issues.add(ButtonDetector.STYLE);
         issues.add(DetectMissingPrefix.MISSING_NAMESPACE);
         issues.add(OverdrawDetector.ISSUE);
         issues.add(StringFormatDetector.INVALID);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
index a06fc1f..d7aa4d4 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
@@ -18,6 +18,7 @@ package com.android.tools.lint.checks;
 
 import static com.android.SdkConstants.ANDROID_STRING_PREFIX;
 import static com.android.SdkConstants.ANDROID_URI;
+import static com.android.SdkConstants.ATTR_BACKGROUND;
 import static com.android.SdkConstants.ATTR_ID;
 import static com.android.SdkConstants.ATTR_LAYOUT_ALIGN_PARENT_LEFT;
 import static com.android.SdkConstants.ATTR_LAYOUT_ALIGN_PARENT_RIGHT;
@@ -25,6 +26,7 @@ import static com.android.SdkConstants.ATTR_LAYOUT_TO_LEFT_OF;
 import static com.android.SdkConstants.ATTR_LAYOUT_TO_RIGHT_OF;
 import static com.android.SdkConstants.ATTR_NAME;
 import static com.android.SdkConstants.ATTR_ORIENTATION;
+import static com.android.SdkConstants.ATTR_STYLE;
 import static com.android.SdkConstants.ATTR_TEXT;
 import static com.android.SdkConstants.BUTTON;
 import static com.android.SdkConstants.LINEAR_LAYOUT;
@@ -32,6 +34,7 @@ import static com.android.SdkConstants.RELATIVE_LAYOUT;
 import static com.android.SdkConstants.STRING_PREFIX;
 import static com.android.SdkConstants.TABLE_ROW;
 import static com.android.SdkConstants.TAG_STRING;
+import static com.android.SdkConstants.VALUE_SELECTABLE_ITEM_BACKGROUND;
 import static com.android.SdkConstants.VALUE_TRUE;
 import static com.android.SdkConstants.VALUE_VERTICAL;
 
@@ -117,6 +120,24 @@ public class ButtonDetector extends ResourceXmlDetector {
                 "http://developer.android.com/design/building-blocks/dialogs.html"); //$NON-NLS-1$
 
     /** The main issue discovered by this detector */
+    public static final Issue STYLE = Issue.create(
+            "ButtonStyle", //$NON-NLS-1$
+            "Ensures that buttons in button bars are borderless",
+
+            "Button bars typically use a borderless style for the buttons. Set the " +
+            "`style=\"?android:attr/buttonBarButtonStyle\"` attribute " +
+            "on each of the buttons, and set `style=\"?android:attr/buttonBarStyle\"` on " +
+            "the parent layout",
+
+            Category.USABILITY,
+            5,
+            Severity.WARNING,
+            ButtonDetector.class,
+            Scope.RESOURCE_FILE_SCOPE)
+            .setMoreInfo(
+                "http://developer.android.com/design/building-blocks/buttons.html"); //$NON-NLS-1$
+
+    /** The main issue discovered by this detector */
     public static final Issue BACKBUTTON = Issue.create(
             "BackButton", //$NON-NLS-1$
             "Looks for Back buttons, which are not common on the Android platform.",
@@ -284,8 +305,25 @@ public class ButtonDetector extends ResourceXmlDetector {
                 }
             }
         } else if (tagName.equals(BUTTON)) {
+            if (phase == 1) {
+                if (isInButtonBar(element)
+                        && !element.hasAttribute(ATTR_STYLE)
+                        && !VALUE_SELECTABLE_ITEM_BACKGROUND.equals(
+                                element.getAttributeNS(ANDROID_URI, ATTR_BACKGROUND))
+                        && (context.getProject().getMinSdk() >= 11
+                            || context.getFolderVersion() >= 11)
+                        && context.isEnabled(STYLE)
+                        && !parentDefinesSelectableItem(element)) {
+                    context.report(STYLE, element, context.getLocation(element),
+                            "Buttons in button bars should be borderless; use " +
+                            "style=\"?android:attr/buttonBarButtonStyle\" (and " +
+                            "?android:attr/buttonBarStyle on the parent)",
+                            null);
+                }
+            }
+
             String text = element.getAttributeNS(ANDROID_URI, ATTR_TEXT);
-            if (context.getDriver().getPhase() == 2) {
+            if (phase == 2) {
                 if (mApplicableResources.contains(text)) {
                     String key = text;
                     if (key.startsWith(STRING_PREFIX)) {
@@ -323,6 +361,20 @@ public class ButtonDetector extends ResourceXmlDetector {
         }
     }
 
+    private boolean parentDefinesSelectableItem(Element element) {
+        String background = element.getAttributeNS(ANDROID_URI, ATTR_BACKGROUND);
+        if (VALUE_SELECTABLE_ITEM_BACKGROUND.equals(background)) {
+            return true;
+        }
+
+        Node parent = element.getParentNode();
+        if (parent != null && parent.getNodeType() == Node.ELEMENT_NODE) {
+            return parentDefinesSelectableItem((Element) parent);
+        }
+
+        return false;
+    }
+
     /** Report the given OK button as being in the wrong position */
     private void reportOkPosition(XmlContext context, Element element) {
         report(context, element, false /*isCancel*/);
@@ -587,6 +639,48 @@ public class ButtonDetector extends ResourceXmlDetector {
         return isWrongPosition(element, false /*isCancel*/);
     }
 
+    private boolean isInButtonBar(Element element) {
+        assert element.getTagName().equals(BUTTON) : element.getTagName();
+        Node parentNode = element.getParentNode();
+        if (parentNode.getNodeType() != Node.ELEMENT_NODE) {
+            return false;
+        }
+        Element parent = (Element) parentNode;
+
+        String style = parent.getAttribute(ATTR_STYLE);
+        if (style != null && style.contains("buttonBarStyle")) { //$NON-NLS-1$
+            return true;
+        }
+
+        // Don't warn about single Cancel / OK buttons
+        if (LintUtils.getChildCount(parent) < 2) {
+            return false;
+        }
+
+        String layout = parent.getTagName();
+        if (layout.equals(LINEAR_LAYOUT) || layout.equals(TABLE_ROW)) {
+            String orientation = parent.getAttributeNS(ANDROID_URI, ATTR_ORIENTATION);
+            if (VALUE_VERTICAL.equals(orientation)) {
+                return false;
+            }
+        } else {
+            return false;
+        }
+
+        // Ensure that all the children are buttons
+        Node n = parent.getFirstChild();
+        while (n != null) {
+            if (n.getNodeType() == Node.ELEMENT_NODE) {
+                if (!BUTTON.equals(n.getNodeName())) {
+                    return false;
+                }
+            }
+            n = n.getNextSibling();
+        }
+
+        return true;
+    }
+
     /** Is the given button in the wrong position? */
     private boolean isWrongPosition(Element element, boolean isCancel) {
         Node parentNode = element.getParentNode();
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
index 58df83a..989f33c 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
@@ -325,4 +325,91 @@ public class ButtonDetectorTest extends AbstractCheckTest {
                 lintProject("res/layout/buttonbar.xml=>res/layout-de/buttonbar.xml",
                         "res/values/buttonbar-values.xml=>res/values-de/buttonbar-values.xml"));
     }
+
+    public void testButtonStyle() throws Exception {
+        sTestIssue = ButtonDetector.STYLE;
+        assertEquals(
+            "res/layout/buttonbar.xml:12: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:17: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:28: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:33: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:44: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:49: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:60: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:65: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:76: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:81: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:92: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:97: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:108: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:113: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:124: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:129: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:140: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:145: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:156: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "res/layout/buttonbar.xml:161: Warning: Buttons in button bars should be borderless; use style=\"?android:attr/buttonBarButtonStyle\" (and ?android:attr/buttonBarStyle on the parent) [ButtonStyle]\n" +
+            "        <Button\n" +
+            "        ^\n" +
+            "0 errors, 20 warnings\n",
+
+            lintProject(
+                    "apicheck/minsdk14.xml=>AndroidManifest.xml",
+                    "res/layout/buttonbar.xml",
+                    "res/layout/buttonbar2.xml",
+                    "res/layout/buttonbar3.xml",
+                    "res/values/buttonbar-values.xml"));
+    }
+
+    public void testButtonStyleOldMinSdk() throws Exception {
+        sTestIssue = ButtonDetector.STYLE;
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "apicheck/minsdk4.xml=>AndroidManifest.xml",
+                    "res/layout/buttonbar.xml",
+                    "res/layout/buttonbar2.xml",
+                    "res/layout/buttonbar3.xml",
+                    "res/values/buttonbar-values.xml"));
+    }
+
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar.xml
index 8e78aa0..d02d49b 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar.xml
@@ -170,12 +170,12 @@
         android:layout_height="wrap_content" >
 
         <Button
-            android:layout_width="wrap_content"
+            android:layout_width="wrap_content" android:background="?android:attr/selectableItemBackground"
             android:layout_height="wrap_content"
             android:text="@string/send" />
 
         <Button
-            android:layout_width="wrap_content"
+            android:layout_width="wrap_content" android:background="?android:attr/selectableItemBackground"
             android:layout_height="wrap_content"
             android:text="@string/cancel" />
     </LinearLayout>