Merge "Add lint warning for small text"

4 files changed, 85 insertions, 6 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 38dd432..113de79 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 = 124;
+        final int initialCapacity = 125;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -110,6 +110,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(PxUsageDetector.PX_ISSUE);
         issues.add(PxUsageDetector.DP_ISSUE);
         issues.add(PxUsageDetector.IN_MM_ISSUE);
+        issues.add(PxUsageDetector.SMALL_SP_ISSUE);
         issues.add(TextFieldDetector.ISSUE);
         issues.add(TextViewDetector.ISSUE);
         issues.add(UnusedResourceDetector.ISSUE);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/PxUsageDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/PxUsageDetector.java
index 63aac6c..dbe74c1 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/PxUsageDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/PxUsageDetector.java
@@ -16,6 +16,7 @@
 
 package com.android.tools.lint.checks;
 
+import static com.android.SdkConstants.ATTR_LAYOUT_HEIGHT;
 import static com.android.SdkConstants.ATTR_NAME;
 import static com.android.SdkConstants.ATTR_TEXT_SIZE;
 import static com.android.SdkConstants.TAG_ITEM;
@@ -25,6 +26,7 @@ import static com.android.SdkConstants.UNIT_DP;
 import static com.android.SdkConstants.UNIT_IN;
 import static com.android.SdkConstants.UNIT_MM;
 import static com.android.SdkConstants.UNIT_PX;
+import static com.android.SdkConstants.UNIT_SP;
 
 import com.android.annotations.NonNull;
 import com.android.annotations.Nullable;
@@ -111,6 +113,20 @@ public class PxUsageDetector extends LayoutDetector {
             Scope.RESOURCE_FILE_SCOPE).setMoreInfo(
             "http://developer.android.com/training/multiscreen/screendensities.html"); //$NON-NLS-1$
 
+    /** Using text sizes that are too small */
+    public static final Issue SMALL_SP_ISSUE = Issue.create(
+            "SmallSp", //$NON-NLS-1$
+            "Looks for text sizes that are too small",
+
+            "Avoid using sizes smaller than 12sp.",
+
+            Category.USABILITY,
+            4,
+            Severity.WARNING,
+            PxUsageDetector.class,
+            Scope.RESOURCE_FILE_SCOPE);
+
+
     /** Constructs a new {@link PxUsageDetector} */
     public PxUsageDetector() {
     }
@@ -161,14 +177,25 @@ public class PxUsageDetector extends LayoutDetector {
             }
             if (context.isEnabled(IN_MM_ISSUE)) {
                 String unit = value.substring(value.length() - 2);
-                context.report(IN_MM_ISSUE, attribute, context.getLocation(attribute), String.format(
-                    "Avoid using \"%1$s\" as units " +
-                    "(it does not work accurately on all devices); use \"dp\" instead", unit),
+                context.report(IN_MM_ISSUE, attribute, context.getLocation(attribute),
+                        String.format("Avoid using \"%1$s\" as units " +
+                                "(it does not work accurately on all devices); use \"dp\" instead",
+                                unit),
                     null);
             }
+        } else if (value.endsWith(UNIT_SP)
+                && (ATTR_TEXT_SIZE.equals(attribute.getLocalName())
+                        || ATTR_LAYOUT_HEIGHT.equals(attribute.getLocalName()))
+                && value.matches("\\d+sp")) { //$NON-NLS-1$
+            int size = getSize(value);
+            if (size > 0 && size < 12) {
+                context.report(SMALL_SP_ISSUE, attribute, context.getLocation(attribute),
+                        String.format("Avoid using sizes smaller than 12sp: %1$s", value),
+                        null);
+            }
         } else if (ATTR_TEXT_SIZE.equals(attribute.getLocalName())
                 && (value.endsWith(UNIT_DP) || value.endsWith(UNIT_DIP))
-                && (value.matches("\\d+di?p"))) {
+                && (value.matches("\\d+di?p"))) { //$NON-NLS-1$
             if (context.isEnabled(DP_ISSUE)) {
                 context.report(DP_ISSUE, attribute, context.getLocation(attribute),
                     "Should use \"sp\" instead of \"dp\" for text sizes", null);
@@ -176,6 +203,11 @@ public class PxUsageDetector extends LayoutDetector {
         }
     }
 
+    private static int getSize(String text) {
+        assert text.matches("\\d+sp") : text; //$NON-NLS-1$
+        return Integer.parseInt(text.substring(0, text.length() - 2));
+    }
+
     @Override
     public void visitElement(@NonNull XmlContext context, @NonNull Element element) {
         if (context.getResourceFolderType() != ResourceFolderType.VALUES) {
@@ -239,6 +271,23 @@ public class PxUsageDetector extends LayoutDetector {
                                 "Should use \"sp\" instead of \"dp\" for text sizes", null);
                         }
                     }
+                } else if (c == 'p' && text.charAt(j - 1) == 's') {
+                    String name = item.getAttribute(ATTR_NAME);
+                    if (ATTR_TEXT_SIZE.equals(name) || ATTR_LAYOUT_HEIGHT.equals(name)) {
+                        text = text.trim();
+                        String unit = text.substring(text.length() - 2);
+                        if (text.matches("\\d+" + unit)) { //$NON-NLS-1$
+                            if (context.isEnabled(SMALL_SP_ISSUE)) {
+                                int size = getSize(text);
+                                if (size > 0 && size < 12) {
+                                    context.report(SMALL_SP_ISSUE, item,
+                                        context.getLocation(textNode), String.format(
+                                                "Avoid using sizes smaller than 12sp: %1$s",
+                                                        text), null);
+                                }
+                            }
+                        }
+                    }
                 }
                 break;
             }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
index 7b5dd0c..68f218f 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
@@ -48,7 +48,13 @@ public class PxUsageDetectorTest extends AbstractCheckTest {
             "res/layout/textsize.xml:16: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
             "        android:textSize=\"14dip\" />\n" +
             "        ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
-            "0 errors, 2 warnings\n",
+            "res/layout/textsize.xml:33: Warning: Avoid using sizes smaller than 12sp: 11sp [SmallSp]\n" +
+            "        android:textSize=\"11sp\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/textsize.xml:37: Warning: Avoid using sizes smaller than 12sp: 6sp [SmallSp]\n" +
+            "        android:layout_height=\"6sp\" />\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "0 errors, 4 warnings\n",
 
             lintFiles("res/layout/textsize.xml"));
     }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/textsize.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/textsize.xml
index 8a480ef..610867e 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/textsize.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/textsize.xml
@@ -25,4 +25,27 @@
         android:layout_height="wrap_content"
         android:textSize="@android/dimen/mysizedp" />
 
+    <!-- Small -->
+
+    <TextView
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:textSize="11sp" />
+
+    <ImageView
+        android:layout_width="wrap_content"
+        android:layout_height="6sp" />
+
+    <!-- No warnings: wrong attribute, size == 0, etc -->
+
+    <TextView
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:textSize="0sp" />
+
+    <TextView
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:marginTop="5sp" />
+
 </LinearLayout>