From 659ed8f892ce38201f6eee16253565b26a5484c8 Mon Sep 17 00:00:00 2001 From: Tor Norbye Date: Wed, 26 Sep 2012 14:26:03 -0700 Subject: 37861: Lint should warn when using mm or in units Change-Id: I220112559c1a7f30c505551b41004560b6e89891 --- .../tools/lint/checks/BuiltinIssueRegistry.java | 3 +- .../android/tools/lint/checks/PxUsageDetector.java | 50 ++++++++++++++++++++-- .../tools/lint/checks/PxUsageDetectorTest.java | 34 +++++++++------ .../checks/data/res/layout/now_playing_after.xml | 4 +- .../tools/lint/checks/data/res/values/pxsp.xml | 13 ++++-- 5 files changed, 83 insertions(+), 21 deletions(-) (limited to 'lint') 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 sIssues; static { - final int initialCapacity = 111; + final int initialCapacity = 112; List issues = new ArrayList(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" + + " 50mm\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" + + " 50dp\n" + + " ^\n" + + "res/values/pxsp.xml:12: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" + + " 50dip \n" + + " ^\n" + "res/values/pxsp.xml:9: Warning: Avoid using \"px\" as units; use \"dp\" instead [PxUsage]\n" + " 50px\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" + " 50px\n" + " ^\n" + - "res/values/pxsp.xml:6: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" + - " 50dp\n" + - " ^\n" + - "res/values/pxsp.xml:12: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" + - " 50dip \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" /> 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 @@ - - - @@ -19,4 +19,11 @@ 50dip + + -- cgit v1.1