37861: Lint should warn when using mm or in units

Change-Id: I220112559c1a7f30c505551b41004560b6e89891

5 files changed, 83 insertions, 21 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 476ebf9..4b12a9e 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 = 111;
+        final int initialCapacity = 112;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -104,6 +104,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(ProguardDetector.SPLITCONFIG);
         issues.add(PxUsageDetector.PX_ISSUE);
         issues.add(PxUsageDetector.DP_ISSUE);
+        issues.add(PxUsageDetector.IN_MM_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 ef2768d..d1c5c63 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
@@ -22,6 +22,8 @@ import static com.android.SdkConstants.TAG_ITEM;
 import static com.android.SdkConstants.TAG_STYLE;
 import static com.android.SdkConstants.UNIT_DIP;
 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 com.android.annotations.NonNull;
@@ -48,7 +50,7 @@ import java.util.Collections;
  * Also look for non-"sp" text sizes.
  */
 public class PxUsageDetector extends LayoutDetector {
-    /** The main issue discovered by this detector */
+    /** Using px instead of dp */
     public static final Issue PX_ISSUE = Issue.create(
             "PxUsage", //$NON-NLS-1$
             "Looks for use of the \"px\" dimension",
@@ -69,7 +71,24 @@ public class PxUsageDetector extends LayoutDetector {
             Scope.RESOURCE_FILE_SCOPE).setMoreInfo(
             "http://developer.android.com/guide/practices/screens_support.html#screen-independence"); //$NON-NLS-1$
 
-    /** The main issue discovered by this detector */
+    /** Using mm/in instead of dp */
+    public static final Issue IN_MM_ISSUE = Issue.create(
+            "InOrMmUsage", //$NON-NLS-1$
+            "Looks for use of the \"mm\" or \"in\" dimensions",
+
+            "Avoid using `mm` (millimeters) or `in` (inches) as the unit for dimensions.\n" +
+            "\n" +
+            "While it should work in principle, unfortunately many devices do not report " +
+            "the correct true physical density, which means that the dimension calculations " +
+            "won't work correctly. You are better off using `dp` (and for font sizes, `sp`.)",
+
+            Category.CORRECTNESS,
+            4,
+            Severity.WARNING,
+            PxUsageDetector.class,
+            Scope.RESOURCE_FILE_SCOPE);
+
+    /** Using sp instead of dp */
     public static final Issue DP_ISSUE = Issue.create(
             "SpUsage", //$NON-NLS-1$
             "Looks for uses of \"dp\" instead of \"sp\" dimensions for text sizes",
@@ -86,7 +105,7 @@ public class PxUsageDetector extends LayoutDetector {
             "font size settings are not respected, so consider adjusting the layout itself " +
             "to be more flexible.",
             Category.CORRECTNESS,
-            2,
+            3,
             Severity.WARNING,
             PxUsageDetector.class,
             Scope.RESOURCE_FILE_SCOPE).setMoreInfo(
@@ -134,6 +153,19 @@ public class PxUsageDetector extends LayoutDetector {
                 context.report(PX_ISSUE, attribute, context.getLocation(attribute),
                     "Avoid using \"px\" as units; use \"dp\" instead", null);
             }
+        } else if (value.endsWith(UNIT_MM) && value.matches("\\d+mm") //$NON-NLS-1$
+                       || value.endsWith(UNIT_IN) && value.matches("\\d+in")) { //$NON-NLS-1$
+            if (value.charAt(0) == '0') {
+                // 0mm == 0in == 0dp
+                return;
+            }
+            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),
+                    null);
+            }
         } else if (ATTR_TEXT_SIZE.equals(attribute.getLocalName())
                 && (value.endsWith(UNIT_DP) || value.endsWith(UNIT_DIP))
                 && (value.matches("\\d+di?p"))) {
@@ -183,6 +215,18 @@ public class PxUsageDetector extends LayoutDetector {
                                 "Avoid using \"px\" as units; use \"dp\" instead", null);
                         }
                     }
+                } else if (c == 'm' && text.charAt(j - 1) == 'm' ||
+                            c == 'n' && text.charAt(j - 1) == 'i') {
+                    text = text.trim();
+                    String unit = text.substring(text.length() - 2);
+                    if (text.matches("\\d+" + unit)) { //$NON-NLS-1$
+                        if (context.isEnabled(IN_MM_ISSUE)) {
+                            context.report(IN_MM_ISSUE, item, context.getLocation(textNode),
+                                String.format("Avoid using \"%1$s\" as units "
+                                        + "(it does not work accurately on all devices); "
+                                        + "use \"dp\" instead", unit), null);
+                        }
+                    }
                 } else if (c == 'p' && (text.charAt(j - 1) == 'd'
                         || text.charAt(j - 1) == 'i')) { // ends with dp or di
                     text = text.trim();
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 77ebd70..9b74676 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
@@ -27,11 +27,16 @@ public class PxUsageDetectorTest extends AbstractCheckTest {
 
     public void testPx() throws Exception {
         assertEquals(
+            "res/layout/now_playing_after.xml:49: Warning: Avoid using \"mm\" as units (it does not work accurately on all devices); use \"dp\" instead [InOrMmUsage]\n" +
+            "        android:layout_width=\"100mm\"\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "res/layout/now_playing_after.xml:50: Warning: Avoid using \"in\" as units (it does not work accurately on all devices); use \"dp\" instead [InOrMmUsage]\n" +
+            "        android:layout_height=\"120in\"\n" +
+            "        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
             "res/layout/now_playing_after.xml:41: Warning: Avoid using \"px\" as units; use \"dp\" instead [PxUsage]\n" +
             "        android:layout_width=\"1px\"\n" +
             "        ~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
-            "0 errors, 1 warnings\n" +
-            "",
+            "0 errors, 3 warnings\n",
             lintFiles("res/layout/now_playing_after.xml"));
     }
 
@@ -43,13 +48,25 @@ 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" +
-            "",
+            "0 errors, 2 warnings\n",
+
             lintFiles("res/layout/textsize.xml"));
     }
 
     public void testStyles() throws Exception {
         assertEquals(
+            "res/values/pxsp.xml:23: Warning: Avoid using \"mm\" as units (it does not work accurately on all devices); use \"dp\" instead [InOrMmUsage]\n" +
+            "        <item name=\"android:textSize\">50mm</item>\n" +
+            "                                      ^\n" +
+            "res/values/pxsp.xml:25: Warning: Avoid using \"in\" as units (it does not work accurately on all devices); use \"dp\" instead [InOrMmUsage]\n" +
+            "            50in \n" +
+            "            ^\n" +
+            "res/values/pxsp.xml:6: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
+            "        <item name=\"android:textSize\">50dp</item>\n" +
+            "                                      ^\n" +
+            "res/values/pxsp.xml:12: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
+            "        <item name=\"android:textSize\"> 50dip </item>\n" +
+            "                                       ^\n" +
             "res/values/pxsp.xml:9: Warning: Avoid using \"px\" as units; use \"dp\" instead [PxUsage]\n" +
             "        <item name=\"android:textSize\">50px</item>\n" +
             "                                      ^\n" +
@@ -59,14 +76,7 @@ public class PxUsageDetectorTest extends AbstractCheckTest {
             "res/values/pxsp.xml:18: Warning: Avoid using \"px\" as units; use \"dp\" instead [PxUsage]\n" +
             "        <item name=\"android:paddingTop\">50px</item>\n" +
             "                                        ^\n" +
-            "res/values/pxsp.xml:6: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
-            "        <item name=\"android:textSize\">50dp</item>\n" +
-            "                                      ^\n" +
-            "res/values/pxsp.xml:12: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
-            "        <item name=\"android:textSize\"> 50dip </item>\n" +
-            "                                       ^\n" +
-            "0 errors, 5 warnings\n" +
-            "",
+            "0 errors, 7 warnings\n",
 
             lintFiles("res/values/pxsp.xml"));
     }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/now_playing_after.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/now_playing_after.xml
index 367a044..64f681c 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/now_playing_after.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/now_playing_after.xml
@@ -46,8 +46,8 @@
         android:id="@+id/now_playing_more"
         android:src="@drawable/ic_now_playing_logo"
         android:padding="12dip"
-        android:layout_width="wrap_content"
-        android:layout_height="fill_parent"
+        android:layout_width="100mm"
+        android:layout_height="120in"
         android:onClick="onNowPlayingLogoClick"
         android:scaleType="center" />
 </LinearLayout>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/pxsp.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/pxsp.xml
index 9f334a0..91bf06b 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/pxsp.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/pxsp.xml
@@ -2,13 +2,13 @@
     <style name="Style1">
         <item name="android:textSize">50sp</item>
     </style>
-    <style name="Style1">
+    <style name="Style2">
         <item name="android:textSize">50dp</item>
     </style>
-    <style name="Style1">
+    <style name="Style3">
         <item name="android:textSize">50px</item>
     </style>
-    <style name="Style1">
+    <style name="Style4">
         <item name="android:textSize"> 50dip </item>
     </style>
  
@@ -19,4 +19,11 @@
         <item name="android:paddingBottom">50dip</item>
     </style>
 
+    <style name="Style6">
+        <item name="android:textSize">50mm</item>
+        <item name="android:textSize">
+            50in
+        </item>
+    </style>
+
 </resources>