diff options
author | Tor Norbye <tnorbye@google.com> | 2012-12-26 18:59:39 -0800 |
---|---|---|
committer | Tor Norbye <tnorbye@google.com> | 2013-01-02 13:08:46 -0800 |
commit | 2d59e6a8e56b928ba22cb4e0315dabbaf8c53fad (patch) | |
tree | 4ace57ece47f668e6d3307f24bc98a294b06bddd /lint/cli | |
parent | 0083b32f502408b4f3ad5d0be35ac5cf7d5234f7 (diff) | |
download | sdk-2d59e6a8e56b928ba22cb4e0315dabbaf8c53fad.zip sdk-2d59e6a8e56b928ba22cb4e0315dabbaf8c53fad.tar.gz sdk-2d59e6a8e56b928ba22cb4e0315dabbaf8c53fad.tar.bz2 |
41753: Lint API Check: Consult source files for constants
Lint currently checks the .class files for field references and method
references to APIs that require a version higher than the
minSdkVersion in the manifest.
However, constants (such as final static integers) will be copied into
the .class file, so there is no reference to the original API and its
API version, which means lint can't flag these.
In some cases, such as referencing LayoutParams.MATCH_PARENT, that's
no problem; the constant value works on older versions, and there is
no problem, since the value rather than the reference is
used. However, in other cases this may lead to runtime crashes.
This CL updates lint to look at the source files and flag field
references. It excludes some constants that are known to be okay
(such as referencing say android.os.Build.VERSION_CODES.JELLY_BEAN_MR1
(which is typically done precisely to conditionally branch based on
the current device's version). It also ignores usages as case
constants or in conditional if checks. We may need to do additional
tweaks to this in the future, since unlike method and field references
there are probably many cases where referencing these constants are
fine on older devices, but there are also cases where it's
dangerous. For now, we're erring on the side of warning the developer
rather than being certain that it's not a problem.
Change-Id: Ieba4c15d7fdda02756b7b7f5a1b79f7698c1915b
Diffstat (limited to 'lint/cli')
5 files changed, 216 insertions, 10 deletions
diff --git a/lint/cli/src/test/java/com/android/tools/lint/MainTest.java b/lint/cli/src/test/java/com/android/tools/lint/MainTest.java index ba31ed8..5a48a47 100644 --- a/lint/cli/src/test/java/com/android/tools/lint/MainTest.java +++ b/lint/cli/src/test/java/com/android/tools/lint/MainTest.java @@ -142,6 +142,14 @@ public class MainTest extends AbstractCheckTest { "Similarly, you can use tools:targetApi=\"11\" in an XML file to indicate that\n" + "the element will only be inflated in an adequate context.\n" + "\n" + + "Lint will also flag certain constants, such as static final integers, which\n" + + "were introduced in later versions. These will actually be copied into the\n" + + "class files rather than being referenced, which means that the value is\n" + + "available even when running on older devices. In some cases that's fine, and\n" + + "in other cases it can result in a runtime crash or incorrect behavior. It\n" + + "depends on the context, so consider the code carefully and device whether it's\n" + + "safe and can be suppressed or whether the code needs to be guarded.\n" + + "\n" + "\n", // Expected error diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/ApiDetectorTest.java b/lint/cli/src/test/java/com/android/tools/lint/checks/ApiDetectorTest.java index e42280f..1bcbfaa 100644 --- a/lint/cli/src/test/java/com/android/tools/lint/checks/ApiDetectorTest.java +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/ApiDetectorTest.java @@ -138,14 +138,14 @@ public class ApiDetectorTest extends AbstractCheckTest { " ~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:33: Error: Field requires API level 11 (current min is 1): dalvik.bytecode.OpcodeInfo#MAXIMUM_VALUE [NewApi]\n" + " int field = OpcodeInfo.MAXIMUM_VALUE; // API 11\n" + - " ~~~~~~~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:38: Error: Field requires API level 14 (current min is 1): android.app.ApplicationErrorReport#batteryInfo [NewApi]\n" + " BatteryInfo batteryInfo = getReport().batteryInfo;\n" + " ~~~~~~~~~~~\n" + // Note: the above error range is wrong; should be pointing to the second "src/foo/bar/ApiCallTest.java:41: Error: Field requires API level 11 (current min is 1): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + " Mode mode = PorterDuff.Mode.OVERLAY; // API 11\n" + - " ~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~\n" + "7 errors, 0 warnings\n", lintProject( @@ -172,13 +172,13 @@ public class ApiDetectorTest extends AbstractCheckTest { " ~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:33: Error: Field requires API level 11 (current min is 2): dalvik.bytecode.OpcodeInfo#MAXIMUM_VALUE [NewApi]\n" + " int field = OpcodeInfo.MAXIMUM_VALUE; // API 11\n" + - " ~~~~~~~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:38: Error: Field requires API level 14 (current min is 2): android.app.ApplicationErrorReport#batteryInfo [NewApi]\n" + " BatteryInfo batteryInfo = getReport().batteryInfo;\n" + " ~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:41: Error: Field requires API level 11 (current min is 2): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + " Mode mode = PorterDuff.Mode.OVERLAY; // API 11\n" + - " ~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~\n" + "7 errors, 0 warnings\n", lintProject( @@ -202,13 +202,13 @@ public class ApiDetectorTest extends AbstractCheckTest { " ~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:33: Error: Field requires API level 11 (current min is 4): dalvik.bytecode.OpcodeInfo#MAXIMUM_VALUE [NewApi]\n" + " int field = OpcodeInfo.MAXIMUM_VALUE; // API 11\n" + - " ~~~~~~~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:38: Error: Field requires API level 14 (current min is 4): android.app.ApplicationErrorReport#batteryInfo [NewApi]\n" + " BatteryInfo batteryInfo = getReport().batteryInfo;\n" + " ~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:41: Error: Field requires API level 11 (current min is 4): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + " Mode mode = PorterDuff.Mode.OVERLAY; // API 11\n" + - " ~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~\n" + "6 errors, 0 warnings\n", lintProject( @@ -229,13 +229,13 @@ public class ApiDetectorTest extends AbstractCheckTest { " ~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:33: Error: Field requires API level 11 (current min is 10): dalvik.bytecode.OpcodeInfo#MAXIMUM_VALUE [NewApi]\n" + " int field = OpcodeInfo.MAXIMUM_VALUE; // API 11\n" + - " ~~~~~~~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:38: Error: Field requires API level 14 (current min is 10): android.app.ApplicationErrorReport#batteryInfo [NewApi]\n" + " BatteryInfo batteryInfo = getReport().batteryInfo;\n" + " ~~~~~~~~~~~\n" + "src/foo/bar/ApiCallTest.java:41: Error: Field requires API level 11 (current min is 10): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + " Mode mode = PorterDuff.Mode.OVERLAY; // API 11\n" + - " ~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~\n" + "5 errors, 0 warnings\n", lintProject( @@ -366,13 +366,13 @@ public class ApiDetectorTest extends AbstractCheckTest { " ~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/SuppressTest1.java:89: Error: Field requires API level 11 (current min is 1): dalvik.bytecode.OpcodeInfo#MAXIMUM_VALUE [NewApi]\n" + " int field = OpcodeInfo.MAXIMUM_VALUE; // API 11\n" + - " ~~~~~~~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + "src/foo/bar/SuppressTest1.java:94: Error: Field requires API level 14 (current min is 1): android.app.ApplicationErrorReport#batteryInfo [NewApi]\n" + " BatteryInfo batteryInfo = getReport().batteryInfo;\n" + " ~~~~~~~~~~~\n" + "src/foo/bar/SuppressTest1.java:97: Error: Field requires API level 11 (current min is 1): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + " Mode mode = PorterDuff.Mode.OVERLAY; // API 11\n" + - " ~~~~~~~\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~\n" + // Note: These annotations are within the methods, not ON the methods, so they have // no effect (because they don't end up in the bytecode) @@ -716,4 +716,74 @@ public class ApiDetectorTest extends AbstractCheckTest { "apicheck/ApiCallTest12.class.data=>bin/classes/test/pkg/ApiCallTest12.class" )); } + + public void testJavaConstants() throws Exception { + assertEquals("" + + "src/test/pkg/ApiSourceCheck.java:5: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + "import static android.view.View.MEASURED_STATE_MASK;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:30: Error: Field requires API level 11 (current min is 1): android.widget.ZoomControls#MEASURED_STATE_MASK [NewApi]\n" + + " int x = MEASURED_STATE_MASK;\n" + + " ~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:33: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " int y = android.view.View.MEASURED_STATE_MASK;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:36: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " int z = View.MEASURED_STATE_MASK;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:37: Error: Field requires API level 14 (current min is 1): android.view.View#FIND_VIEWS_WITH_TEXT [NewApi]\n" + + " int find2 = View.FIND_VIEWS_WITH_TEXT; // requires API 14\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:40: Error: Field requires API level 12 (current min is 1): android.app.ActivityManager#MOVE_TASK_NO_USER_ACTION [NewApi]\n" + + " int w = ActivityManager.MOVE_TASK_NO_USER_ACTION;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:41: Error: Field requires API level 14 (current min is 1): android.widget.ZoomButton#FIND_VIEWS_WITH_CONTENT_DESCRIPTION [NewApi]\n" + + " int find1 = ZoomButton.FIND_VIEWS_WITH_CONTENT_DESCRIPTION; // requires\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:44: Error: Field requires API level 9 (current min is 1): android.widget.ZoomControls#OVER_SCROLL_ALWAYS [NewApi]\n" + + " int overScroll = OVER_SCROLL_ALWAYS; // requires API 9\n" + + " ~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:47: Error: Field requires API level 16 (current min is 1): android.widget.ZoomControls#IMPORTANT_FOR_ACCESSIBILITY_AUTO [NewApi]\n" + + " int auto = IMPORTANT_FOR_ACCESSIBILITY_AUTO; // requires API 16\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:51: Error: Field requires API level 14 (current min is 1): android.widget.ZoomButton#ROTATION_X [NewApi]\n" + + " Object rotationX = ZoomButton.ROTATION_X; // Requires API 14\n" + + " ~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:54: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " return (child.getMeasuredWidth() & View.MEASURED_STATE_MASK)\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:55: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_HEIGHT_STATE_SHIFT [NewApi]\n" + + " | ((child.getMeasuredHeight() >> View.MEASURED_HEIGHT_STATE_SHIFT) & (View.MEASURED_STATE_MASK >> View.MEASURED_HEIGHT_STATE_SHIFT));\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:55: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_HEIGHT_STATE_SHIFT [NewApi]\n" + + " | ((child.getMeasuredHeight() >> View.MEASURED_HEIGHT_STATE_SHIFT) & (View.MEASURED_STATE_MASK >> View.MEASURED_HEIGHT_STATE_SHIFT));\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:55: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " | ((child.getMeasuredHeight() >> View.MEASURED_HEIGHT_STATE_SHIFT) & (View.MEASURED_STATE_MASK >> View.MEASURED_HEIGHT_STATE_SHIFT));\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:90: Error: Field requires API level 8 (current min is 1): android.R.id#custom [NewApi]\n" + + " int custom = android.R.id.custom; // API 8\n" + + " ~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:94: Error: Field requires API level 13 (current min is 1): android.Manifest.permission#SET_POINTER_SPEED [NewApi]\n" + + " String setPointerSpeed = permission.SET_POINTER_SPEED;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:95: Error: Field requires API level 13 (current min is 1): android.Manifest.permission#SET_POINTER_SPEED [NewApi]\n" + + " String setPointerSpeed2 = Manifest.permission.SET_POINTER_SPEED;\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:120: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " int y = View.MEASURED_STATE_MASK; // Not OK\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/ApiSourceCheck.java:121: Error: Field requires API level 11 (current min is 1): android.view.View#MEASURED_STATE_MASK [NewApi]\n" + + " testBenignUsages(View.MEASURED_STATE_MASK); // Not OK\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "19 errors, 0 warnings\n", + + lintProject( + "apicheck/classpath=>.classpath", + "apicheck/minsdk1.xml=>AndroidManifest.xml", + "project.properties1=>project.properties", + "apicheck/ApiSourceCheck.java.txt=>src/test/pkg/ApiSourceCheck.java", + "apicheck/ApiSourceCheck.class.data=>bin/classes/test/pkg/ApiSourceCheck.class" + )); + } } diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.class.data b/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.class.data Binary files differnew file mode 100644 index 0000000..726b8a5 --- /dev/null +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.class.data diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.java.txt b/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.java.txt new file mode 100644 index 0000000..429b388 --- /dev/null +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/apicheck/ApiSourceCheck.java.txt @@ -0,0 +1,123 @@ +package test.pkg; + +import android.util.Property; +import android.view.View; +import static android.view.View.MEASURED_STATE_MASK; +import static android.os.Build.VERSION_CODES.JELLY_BEAN_MR1; +import android.view.*; +import android.annotation.*; +import android.app.*; +import android.widget.*; +import static android.widget.ZoomControls.*; +import android.Manifest.permission; +import android.Manifest; + +/** Various tests for source-level checks */ +final class ApiSourceCheck extends LinearLayout { + public ApiSourceCheck(android.content.Context context) { + super(context); + } + + /** + * Return only the state bits of {@link #getMeasuredWidthAndState()} and + * {@link #getMeasuredHeightAndState()}, combined into one integer. The + * width component is in the regular bits {@link #MEASURED_STATE_MASK} and + * the height component is at the shifted bits + * {@link #MEASURED_HEIGHT_STATE_SHIFT}>>{@link #MEASURED_STATE_MASK}. + */ + public static int m1(View child) { + // from static import of field + int x = MEASURED_STATE_MASK; + + // fully qualified name field access + int y = android.view.View.MEASURED_STATE_MASK; + + // from explicitly imported class + int z = View.MEASURED_STATE_MASK; + int find2 = View.FIND_VIEWS_WITH_TEXT; // requires API 14 + + // from wildcard import of package + int w = ActivityManager.MOVE_TASK_NO_USER_ACTION; + int find1 = ZoomButton.FIND_VIEWS_WITH_CONTENT_DESCRIPTION; // requires + // API 14 + // from static wildcard import + int overScroll = OVER_SCROLL_ALWAYS; // requires API 9 + + // Inherited field from ancestor class (View) + int auto = IMPORTANT_FOR_ACCESSIBILITY_AUTO; // requires API 16 + + // object field reference: ensure that we don't get two errors + // (one from source scan, the other from class scan) + Object rotationX = ZoomButton.ROTATION_X; // Requires API 14 + + // different type of expression than variable declaration + return (child.getMeasuredWidth() & View.MEASURED_STATE_MASK) + | ((child.getMeasuredHeight() >> View.MEASURED_HEIGHT_STATE_SHIFT) & (View.MEASURED_STATE_MASK >> View.MEASURED_HEIGHT_STATE_SHIFT)); + } + + @SuppressLint("NewApi") + private void testSuppress1() { + // Checks suppress on surrounding method + int w = ActivityManager.MOVE_TASK_NO_USER_ACTION; + } + + private void testSuppress2() { + // Checks suppress on surrounding declaration statement + @SuppressLint("NewApi") + int w, z = ActivityManager.MOVE_TASK_NO_USER_ACTION; + } + + @TargetApi(17) + private void testTargetApi1() { + // Checks @TargetApi on surrounding method + int w, z = ActivityManager.MOVE_TASK_NO_USER_ACTION; + } + + @TargetApi(android.os.Build.VERSION_CODES.JELLY_BEAN_MR1) + private void testTargetApi2() { + // Checks @TargetApi with codename + int w, z = ActivityManager.MOVE_TASK_NO_USER_ACTION; + } + + @TargetApi(JELLY_BEAN_MR1) + private void testTargetApi3() { + // Checks @TargetApi with codename + int w, z = ActivityManager.MOVE_TASK_NO_USER_ACTION; + } + + private void checkOtherFields() { + // Look at fields that aren't capitalized + int custom = android.R.id.custom; // API 8 + } + + private void innerclass() { + String setPointerSpeed = permission.SET_POINTER_SPEED; + String setPointerSpeed2 = Manifest.permission.SET_POINTER_SPEED; + } + + private void test() { + // Make sure that local variable references which look like fields, + // even imported ones, aren't taken as invalid references + int OVER_SCROLL_ALWAYS = 1, IMPORTANT_FOR_ACCESSIBILITY_AUTO = 2; + int x = OVER_SCROLL_ALWAYS; + int y = IMPORTANT_FOR_ACCESSIBILITY_AUTO; + findViewById(IMPORTANT_FOR_ACCESSIBILITY_AUTO); // yes, nonsensical + } + + private void testBenignUsages(int x) { + // Certain types of usages (such as switch/case constants) are okay + switch (x) { + case View.MEASURED_STATE_MASK: { // OK + break; + } + } + if (x == View.MEASURED_STATE_MASK) { // OK + } + if (false || x == View.MEASURED_STATE_MASK) { // OK + } + if (x >= View.MEASURED_STATE_MASK) { // OK + } + int y = View.MEASURED_STATE_MASK; // Not OK + testBenignUsages(View.MEASURED_STATE_MASK); // Not OK + } +} diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt b/lint/cli/src/test/java/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt index 6fef833..45743ce 100644 --- a/lint/cli/src/test/java/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt @@ -28,4 +28,9 @@ public class WrongAnnotation { @SuppressLint("NewApi") int localvar = 5; } + + private static void test() { + @SuppressLint("NewApi") // Invalid + int a = View.MEASURED_STATE_MASK; + } } |