Find mismatches between EditText inputType and name

This adds a lint check which looks at the id of an EditText,
and if it looks like it has a specific purpose (e.g. password
or e-mail or number or phone etc) then it checks that the
corresponding inputType matches.

Change-Id: Ib40d3e47c88ea01ccbad61f3a24d9b9a4e79d5c6

4 files changed, 396 insertions, 5 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
index c95b12d..73d5bb6 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
@@ -51,7 +51,6 @@ import org.w3c.dom.NodeList;
 import java.io.File;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -150,7 +149,7 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             6,
             Severity.FATAL,
             ManifestOrderDetector.class,
-            EnumSet.of(Scope.MANIFEST)).setMoreInfo(
+            Scope.MANIFEST_SCOPE).setMoreInfo(
             "http://developer.android.com/guide/topics/manifest/manifest-intro.html"); //$NON-NLS-1$
 
     /** Missing a {@code <uses-sdk>} element */
@@ -167,7 +166,7 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             5,
             Severity.ERROR,
             ManifestOrderDetector.class,
-            EnumSet.of(Scope.MANIFEST));
+            Scope.MANIFEST_SCOPE);
 
     /** Not explicitly defining allowBackup */
     public static final Issue ALLOW_BACKUP = Issue.create(
@@ -200,7 +199,7 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             3,
             Severity.WARNING,
             ManifestOrderDetector.class,
-            EnumSet.of(Scope.MANIFEST)).setMoreInfo(
+            Scope.MANIFEST_SCOPE).setMoreInfo(
                     "http://developer.android.com/reference/android/R.attr.html#allowBackup");
 
     /** Constructs a new {@link ManifestOrderDetector} check */
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextFieldDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextFieldDetector.java
index 74efb32..376018b 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextFieldDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextFieldDetector.java
@@ -18,19 +18,27 @@ package com.android.tools.lint.checks;
 
 import static com.android.SdkConstants.ANDROID_URI;
 import static com.android.SdkConstants.ATTR_HINT;
+import static com.android.SdkConstants.ATTR_ID;
 import static com.android.SdkConstants.ATTR_INPUT_METHOD;
 import static com.android.SdkConstants.ATTR_INPUT_TYPE;
+import static com.android.SdkConstants.ATTR_PASSWORD;
+import static com.android.SdkConstants.ATTR_PHONE_NUMBER;
 import static com.android.SdkConstants.EDIT_TEXT;
+import static com.android.SdkConstants.ID_PREFIX;
+import static com.android.SdkConstants.NEW_ID_PREFIX;
 
 import com.android.annotations.NonNull;
+import com.android.annotations.VisibleForTesting;
 import com.android.tools.lint.detector.api.Category;
 import com.android.tools.lint.detector.api.Issue;
 import com.android.tools.lint.detector.api.LayoutDetector;
+import com.android.tools.lint.detector.api.Location;
 import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
 import com.android.tools.lint.detector.api.Speed;
 import com.android.tools.lint.detector.api.XmlContext;
 
+import org.w3c.dom.Attr;
 import org.w3c.dom.Element;
 
 import java.util.Collection;
@@ -44,12 +52,18 @@ public class TextFieldDetector extends LayoutDetector {
     public static final Issue ISSUE = Issue.create(
             "TextFields", //$NON-NLS-1$
             "Looks for text fields missing inputType or hint settings",
+
             "Providing an `inputType` attribute on a text field improves usability " +
             "because depending on the data to be input, optimized keyboards can be shown " +
             "to the user (such as just digits and parentheses for a phone number). Similarly," +
             "a hint attribute displays a hint to the user for what is expected in the " +
             "text field.\n" +
             "\n" +
+            "The lint detector also looks at the `id` of the view, and if the id offers a " +
+            "hint of the purpose of the field (for example, the `id` contains the phrase " +
+            "`phone` or `email`), then lint will also ensure that the `inputType` contains " +
+            "the corresponding type attributes.\n" +
+            "\n" +
             "If you really want to keep the text field generic, you can suppress this warning " +
             "by setting `inputType=\"text\"`.",
 
@@ -75,7 +89,8 @@ public class TextFieldDetector extends LayoutDetector {
 
     @Override
     public void visitElement(@NonNull XmlContext context, @NonNull Element element) {
-        if (!element.hasAttributeNS(ANDROID_URI, ATTR_INPUT_TYPE) &&
+        Attr inputTypeNode = element.getAttributeNodeNS(ANDROID_URI, ATTR_INPUT_TYPE);
+        if (inputTypeNode == null &&
                 !element.hasAttributeNS(ANDROID_URI, ATTR_HINT)) {
             // Also make sure the EditText does not set an inputMethod in which case
             // an inputType might be provided from the input.
@@ -86,5 +101,203 @@ public class TextFieldDetector extends LayoutDetector {
             context.report(ISSUE, element, context.getLocation(element),
                     "This text field does not specify an inputType or a hint", null);
         }
+
+        Attr idNode = element.getAttributeNodeNS(ANDROID_URI, ATTR_ID);
+        if (idNode == null) {
+            return;
+        }
+        String id = idNode.getValue();
+        if (id.isEmpty()) {
+            return;
+        }
+        if (id.startsWith("editText")) { //$NON-NLS-1$
+            // Just the default label
+            return;
+        }
+
+        String inputType = "";
+        if (inputTypeNode != null) {
+            inputType = inputTypeNode.getValue();
+        }
+
+        // TODO: See if the name is just the default names (button1, editText1 etc)
+        // and if so, do nothing
+        // TODO: Unit test this
+
+        if (containsWord(id, "phone", true, true)) {                 //$NON-NLS-1$
+            if (!inputType.contains("phone")                         //$NON-NLS-1$
+                    && element.getAttributeNodeNS(ANDROID_URI, ATTR_PHONE_NUMBER) == null) {
+                String message = String.format("The view name (%1$s) suggests this is a phone "
+                        + "number, but it does not include 'phone' in the inputType", id);
+                reportMismatch(context, idNode, inputTypeNode, message);
+            }
+            return;
+        }
+
+        if (containsWord(id, "width", false, true)
+                || containsWord(id, "height", false, true)
+                || containsWord(id, "size", false, true)
+                || containsWord(id, "length", false, true)
+                || containsWord(id, "weight", false, true)
+                || containsWord(id, "number", false, true)) {
+            if (!inputType.contains("number") && !inputType.contains("phone")) { //$NON-NLS-1$
+                String message = String.format("The view name (%1$s) suggests this is a number, "
+                        + "but it does not include a numeric inputType (such as 'numberSigned')",
+                        id);
+                reportMismatch(context, idNode, inputTypeNode, message);
+            }
+            return;
+        }
+
+        if (containsWord(id, "password", true, true)) {   //$NON-NLS-1$
+            if (!(inputType.contains("Password"))  //$NON-NLS-1$
+                && element.getAttributeNodeNS(ANDROID_URI, ATTR_PASSWORD) == null) {
+                String message = String.format("The view name (%1$s) suggests this is a password, "
+                        + "but it does not include 'textPassword' in the inputType", id);
+                reportMismatch(context, idNode, inputTypeNode, message);
+            }
+            return;
+        }
+
+        if (containsWord(id, "email", true, true)) {                   //$NON-NLS-1$
+            if (!inputType.contains("Email")) { //$NON-NLS-1$
+                String message = String.format("The view name (%1$s) suggests this is an e-mail "
+                        + "address, but it does not include 'textEmail' in the inputType", id);
+                reportMismatch(context, idNode, inputTypeNode, message);
+            }
+            return;
+        }
+
+        if (endsWith(id, "pin", false, true)) {    //$NON-NLS-1$
+            if (!(inputType.contains("numberPassword"))  //$NON-NLS-1$
+                && element.getAttributeNodeNS(ANDROID_URI, ATTR_PASSWORD) == null) {
+                String message = String.format("The view name (%1$s) suggests this is a password, "
+                        + "but it does not include 'numberPassword' in the inputType", id);
+                reportMismatch(context, idNode, inputTypeNode, message);
+            }
+            return;
+        }
+
+        if ((containsWord(id, "uri") || containsWord(id, "url"))
+                && !inputType.contains("textUri")) {
+            String message = String.format("The view name (%1$s) suggests this is a URI, "
+                    + "but it does not include 'textUri' in the inputType", id);
+            reportMismatch(context, idNode, inputTypeNode, message);
+        }
+
+        if ((containsWord(id, "date"))            //$NON-NLS-1$
+                && !inputType.contains("date")) { //$NON-NLS-1$
+            String message = String.format("The view name (%1$s) suggests this is a date, "
+                    + "but it does not include 'date' or 'datetime' in the inputType", id);
+            reportMismatch(context, idNode, inputTypeNode, message);
+        }
+    }
+
+    private void reportMismatch(XmlContext context, Attr idNode, Attr inputTypeNode,
+            String message) {
+        Location location;
+        if (inputTypeNode != null) {
+            location = context.getLocation(inputTypeNode);
+            Location secondary = context.getLocation(idNode);
+            secondary.setMessage("id defined here");
+            location.setSecondary(secondary);
+        } else {
+            location = context.getLocation(idNode);
+        }
+        context.report(ISSUE, idNode.getOwnerElement(), location, message, null);
+    }
+
+    /** Returns true if the given sentence contains a given word */
+    @VisibleForTesting
+    static boolean containsWord(String sentence, String word) {
+        return containsWord(sentence, word, false, false);
+    }
+
+    /**
+     * Returns true if the given sentence contains a given word
+     * @param sentence the full sentence to search within
+     * @param word the word to look for
+     * @param allowPrefix if true, allow a prefix match even if the next character
+     *    is in the same word (same case or not an underscore)
+     * @param allowSuffix if true, allow a suffix match even if the preceding character
+     *    is in the same word (same case or not an underscore)
+     * @return true if the word is contained in the sentence
+     */
+    @VisibleForTesting
+    static boolean containsWord(String sentence, String word, boolean allowPrefix,
+            boolean allowSuffix) {
+        return indexOfWord(sentence, word, allowPrefix, allowSuffix) != -1;
+    }
+
+    /** Returns true if the given sentence <b>ends</b> with a given word */
+    private static boolean endsWith(String sentence, String word, boolean allowPrefix,
+            boolean allowSuffix) {
+        int index = indexOfWord(sentence, word, allowPrefix, allowSuffix);
+
+        if (index != -1) {
+            return index == sentence.length() - word.length();
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns the index of the given word in the given sentence, if any. It will match
+     * across cases, and ignore words that seem to be just a substring in the middle
+     * of another word.
+     *
+     * @param sentence the full sentence to search within
+     * @param word the word to look for
+     * @param allowPrefix if true, allow a prefix match even if the next character
+     *    is in the same word (same case or not an underscore)
+     * @param allowSuffix if true, allow a suffix match even if the preceding character
+     *    is in the same word (same case or not an underscore)
+     * @return true if the word is contained in the sentence
+     */
+    private static int indexOfWord(String sentence, String word, boolean allowPrefix,
+            boolean allowSuffix) {
+        if (sentence.isEmpty()) {
+            return -1;
+        }
+        int wordLength = word.length();
+        if (wordLength > sentence.length()) {
+            return -1;
+        }
+
+        char firstUpper = Character.toUpperCase(word.charAt(0));
+        char firstLower = Character.toLowerCase(firstUpper);
+
+        int start = 0;
+        if (sentence.startsWith(NEW_ID_PREFIX)) {
+            start += NEW_ID_PREFIX.length();
+        } else if (sentence.startsWith(ID_PREFIX)) {
+            start += ID_PREFIX.length();
+        }
+
+        for (int i = start, n = sentence.length(), m = n - (wordLength - 1); i < m; i++) {
+            char c = sentence.charAt(i);
+            if (c == firstUpper || c == firstLower) {
+                if (sentence.regionMatches(true, i, word, 0, wordLength)) {
+                    if (i <= start && allowPrefix) {
+                        return i;
+                    }
+                    if (i == m - 1 && allowSuffix) {
+                        return i;
+                    }
+                    if (i <= start || (sentence.charAt(i - 1) == '_')
+                            || Character.isUpperCase(c)) {
+                        if (i == m - 1) {
+                            return i;
+                        }
+                        char after = sentence.charAt(i + wordLength);
+                        if (after == '_' || Character.isUpperCase(after)) {
+                            return i;
+                        }
+                    }
+                }
+            }
+        }
+
+        return -1;
     }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextFieldDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextFieldDetectorTest.java
index 85e4b8d..795bc85 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextFieldDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextFieldDetectorTest.java
@@ -34,4 +34,94 @@ public class TextFieldDetectorTest extends AbstractCheckTest {
             "",
             lintFiles("res/layout/note_edit.xml"));
     }
+
+    public void testTypeFromName() throws Exception {
+        assertEquals(
+            "res/layout/edit_type.xml:14: Warning: The view name (@+id/mypassword) suggests this is a password, but it does not include 'textPassword' in the inputType [TextFields]\n" +
+            "        android:inputType=\"text\" >\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:10: id defined here\n" +
+            "res/layout/edit_type.xml:45: Warning: The view name (@+id/password_length) suggests this is a number, but it does not include a numeric inputType (such as 'numberSigned') [TextFields]\n" +
+            "        android:inputType=\"text\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:41: id defined here\n" +
+            "res/layout/edit_type.xml:54: Warning: The view name (@+id/welcome_url) suggests this is a URI, but it does not include 'textUri' in the inputType [TextFields]\n" +
+            "        android:inputType=\"text\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:50: id defined here\n" +
+            "res/layout/edit_type.xml:63: Warning: The view name (@+id/start_date) suggests this is a date, but it does not include 'date' or 'datetime' in the inputType [TextFields]\n" +
+            "        android:inputType=\"text\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:59: id defined here\n" +
+            "res/layout/edit_type.xml:72: Warning: The view name (@+id/email_address) suggests this is an e-mail address, but it does not include 'textEmail' in the inputType [TextFields]\n" +
+            "        android:inputType=\"text\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:68: id defined here\n" +
+            "res/layout/edit_type.xml:81: Warning: The view name (@+id/login_pin) suggests this is a password, but it does not include 'numberPassword' in the inputType [TextFields]\n" +
+            "        android:inputType=\"textPassword\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "    res/layout/edit_type.xml:77: id defined here\n" +
+            "res/layout/edit_type.xml:83: Warning: This text field does not specify an inputType or a hint [TextFields]\n" +
+            "    <EditText\n" +
+            "    ^\n" +
+            "res/layout/edit_type.xml:84: Warning: The view name (@+id/number_of_items) suggests this is a number, but it does not include a numeric inputType (such as 'numberSigned') [TextFields]\n" +
+            "        android:id=\"@+id/number_of_items\"\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "0 errors, 8 warnings\n",
+
+            lintFiles("res/layout/edit_type.xml"));
+    }
+
+    public void testContainsWord() {
+        assertFalse(containsWord("", "foob"));
+        assertFalse(containsWord("foo", "foob"));
+
+        assertTrue(containsWord("foo", "foo"));
+        assertTrue(containsWord("Foo", "foo"));
+        assertTrue(containsWord("foo_bar", "foo"));
+        assertTrue(containsWord("bar_foo", "foo"));
+        assertTrue(containsWord("bar_Foo", "foo"));
+        assertTrue(containsWord("bar_foo_baz", "foo"));
+        assertTrue(containsWord("bar_Foo_baz", "foo"));
+        assertTrue(containsWord("barFooBaz", "foo"));
+        assertTrue(containsWord("barFOO_", "foo"));
+        assertTrue(containsWord("FooBaz", "foo"));
+        assertTrue(containsWord("BarFoo", "foo"));
+        assertFalse(containsWord("barfoo", "foo"));
+        assertTrue(containsWord("barfoo", "foo", false, true));
+        assertTrue(containsWord("foobar", "foo", true, false));
+        assertFalse(containsWord("foobar", "foo"));
+        assertFalse(containsWord("barfoobar", "foo"));
+
+        assertTrue(containsWord("phoneNumber", "phone"));
+        assertTrue(containsWord("phoneNumber", "number"));
+        assertTrue(containsWord("uri_prefix", "uri"));
+        assertTrue(containsWord("fooURI", "uri"));
+        assertTrue(containsWord("my_url", "url"));
+        assertTrue(containsWord("network_prefix_length", "length"));
+
+        assertFalse(containsWord("sizer", "size"));
+        assertFalse(containsWord("synthesize_to_filename", "size"));
+        assertFalse(containsWord("update_text", "date"));
+        assertFalse(containsWord("daten", "date"));
+
+        assertFalse(containsWord("phonenumber", "phone"));
+        assertFalse(containsWord("myphone", "phone"));
+        assertTrue(containsWord("phonenumber", "phone", true, true));
+        assertTrue(containsWord("myphone", "phone", true, true));
+        assertTrue(containsWord("phoneNumber", "phone"));
+
+        assertTrue(containsWord("phoneNumber", "phone"));
+        assertTrue(containsWord("@id/phoneNumber", "phone"));
+        assertTrue(containsWord("@+id/phoneNumber", "phone"));
+    }
+
+    private static boolean containsWord(String name, String word, boolean allowPrefix,
+            boolean allowSuffix) {
+        return TextFieldDetector.containsWord(name, word, allowPrefix, allowSuffix);
+    }
+
+    private static boolean containsWord(String name, String word) {
+        return TextFieldDetector.containsWord(name, word);
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_type.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_type.xml
new file mode 100644
index 0000000..63452e3
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_type.xml
@@ -0,0 +1,89 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:orientation="vertical" >
+
+    <!-- Wrong: doesn't specify textPassword -->
+
+    <EditText
+        android:id="@+id/mypassword"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text" >
+
+        <requestFocus />
+    </EditText>
+
+    <!-- OK, specifies textPassword: -->
+
+    <EditText
+        android:id="@+id/password1"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text|numberPassword" />
+
+    <!-- OK, specifies password: -->
+
+    <EditText
+        android:id="@+id/password2"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text"
+        android:password="true" />
+
+    <!-- Wrong, doesn't include number -->
+
+    <EditText
+        android:id="@+id/password_length"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text" />
+
+    <!-- Wrong, doesn't include URL -->
+
+    <EditText
+        android:id="@+id/welcome_url"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text" />
+
+    <!-- Wrong, doesn't include date -->
+
+    <EditText
+        android:id="@+id/start_date"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text" />
+
+    <!-- Wrong, doesn't include e-mail -->
+
+    <EditText
+        android:id="@+id/email_address"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="text" />
+
+    <!-- Wrong, uses wrong password type for PIN -->
+
+    <EditText
+        android:id="@+id/login_pin"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10"
+        android:inputType="textPassword" />
+
+    <EditText
+        android:id="@+id/number_of_items"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:ems="10" />
+
+</LinearLayout>