Merge "Add lint check for making dynamic text views selectable"

4 files changed, 58 insertions, 7 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 113de79..3a40115 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 = 125;
+        final int initialCapacity = 126;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -113,6 +113,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(PxUsageDetector.SMALL_SP_ISSUE);
         issues.add(TextFieldDetector.ISSUE);
         issues.add(TextViewDetector.ISSUE);
+        issues.add(TextViewDetector.SELECTABLE);
         issues.add(UnusedResourceDetector.ISSUE);
         issues.add(UnusedResourceDetector.ISSUE_IDS);
         issues.add(ExtraTextDetector.ISSUE);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextViewDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextViewDetector.java
index 1714198..dd98709 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextViewDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TextViewDetector.java
@@ -24,6 +24,7 @@ import static com.android.SdkConstants.ATTR_CURSOR_VISIBLE;
 import static com.android.SdkConstants.ATTR_DIGITS;
 import static com.android.SdkConstants.ATTR_EDITABLE;
 import static com.android.SdkConstants.ATTR_EDITOR_EXTRAS;
+import static com.android.SdkConstants.ATTR_ID;
 import static com.android.SdkConstants.ATTR_IME_ACTION_ID;
 import static com.android.SdkConstants.ATTR_IME_ACTION_LABEL;
 import static com.android.SdkConstants.ATTR_IME_OPTIONS;
@@ -33,6 +34,9 @@ import static com.android.SdkConstants.ATTR_NUMERIC;
 import static com.android.SdkConstants.ATTR_PASSWORD;
 import static com.android.SdkConstants.ATTR_PHONE_NUMBER;
 import static com.android.SdkConstants.ATTR_PRIVATE_IME_OPTIONS;
+import static com.android.SdkConstants.ATTR_TEXT;
+import static com.android.SdkConstants.ATTR_TEXT_IS_SELECTABLE;
+import static com.android.SdkConstants.ATTR_VISIBILITY;
 import static com.android.SdkConstants.BUTTON;
 import static com.android.SdkConstants.CHECKED_TEXT_VIEW;
 import static com.android.SdkConstants.CHECK_BOX;
@@ -87,6 +91,25 @@ public class TextViewDetector extends LayoutDetector {
             TextViewDetector.class,
             Scope.RESOURCE_FILE_SCOPE);
 
+    /** Text could be selectable */
+    public static final Issue SELECTABLE = Issue.create(
+            "SelectableText", //$NON-NLS-1$
+            "Looks for TextViews which should probably allow their text to be selected",
+
+            "If a `<TextView>` is used to display data, the user might want to copy that " +
+            "data and paste it elsewhere. To allow this, the `<TextView>` should specify " +
+            "`android:textIsSelectable=\"true\"`.\n" +
+            "\n" +
+            "This lint check looks for TextViews which are likely to be displaying data: " +
+            "views whose text is set dynamically. This value will be ignored on platforms " +
+            "older than API 11, so it is okay to set it regardless of your `minSdkVersion`.",
+
+            Category.USABILITY,
+            7,
+            Severity.WARNING,
+            TextViewDetector.class,
+            Scope.RESOURCE_FILE_SCOPE);
+
     /** Constructs a new {@link TextViewDetector} */
     public TextViewDetector() {
     }
@@ -111,6 +134,18 @@ public class TextViewDetector extends LayoutDetector {
 
     @Override
     public void visitElement(@NonNull XmlContext context, @NonNull Element element) {
+        if (element.getTagName().equals(TEXT_VIEW)) {
+            if (!element.hasAttributeNS(ANDROID_URI, ATTR_TEXT)
+                    && element.hasAttributeNS(ANDROID_URI, ATTR_ID)
+                    && !element.hasAttributeNS(ANDROID_URI, ATTR_TEXT_IS_SELECTABLE)
+                    && !element.hasAttributeNS(ANDROID_URI, ATTR_VISIBILITY)
+                    && context.getMainProject().getTargetSdk() >= 11) {
+                context.report(SELECTABLE, element, context.getLocation(element),
+                        "Consider making the text value selectable by specifying " +
+                        "android:textIsSelectable=\"true\"", null);
+            }
+        }
+
         NamedNodeMap attributes = element.getAttributes();
         for (int i = 0, n = attributes.getLength(); i < n; i++) {
             Attr attribute = (Attr) attributes.item(i);
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextViewDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextViewDetectorTest.java
index 8c63bd8..0ee144b 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextViewDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/TextViewDetectorTest.java
@@ -90,9 +90,13 @@ public class TextViewDetectorTest extends AbstractCheckTest {
             "res/layout/edit_textview.xml:62: Warning: Attribute android:cursorVisible should not be used with <ToggleButton>: intended for editable text widgets [TextViewEdits]\n" +
             "        android:cursorVisible=\"true\" />\n" +
             "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
-            "0 errors, 21 warnings\n" +
-            "",
+            "res/layout/edit_textview.xml:83: Warning: Consider making the text value selectable by specifying android:textIsSelectable=\"true\" [SelectableText]\n" +
+            "    <TextView\n" +
+            "    ^\n" +
+            "0 errors, 22 warnings\n",
 
-            lintFiles("res/layout/edit_textview.xml"));
+            lintFiles(
+                    "apicheck/minsdk14.xml=>AndroidManifest.xml", // API >= 11 for selectable issue
+                    "res/layout/edit_textview.xml"));
     }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_textview.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_textview.xml
index 39aa4b7..c0d8cda 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_textview.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/edit_textview.xml
@@ -7,7 +7,7 @@
     <!-- Various attributes that should be set on EditTexts, not TextViews -->
 
     <TextView
-        android:id="@+id/textview"
+        android:text="label"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:autoText="true"
@@ -65,7 +65,7 @@
     <!-- Ok #1 -->
 
     <TextView
-        android:id="@+id/okTextView1"
+        android:text="label"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:bufferType="spannable"
@@ -76,8 +76,19 @@
     <!-- Ok #2 -->
 
     <TextView
-        android:id="@+id/okTextView2"
+        android:text="label"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content" />
 
+    <TextView
+        android:id="@+id/dynamictext"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content" />
+
+    <TextView
+        android:id="@+id/dynamictext"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:textIsSelectable="true" />
+
 </LinearLayout>