diff options
6 files changed, 281 insertions, 14 deletions
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java index a0d988f..4c5f298 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java @@ -568,6 +568,18 @@ public class ClassContext extends Context { } /** + * Converts from a VM owner name (such as foo/bar/Foo$Baz) to a + * fully qualified class name (such as foo.bar.Foo.Baz). + * + * @param owner the owner name to convert + * @return the corresponding fully qualified class name + */ + @NonNull + public static String getFqcn(@NonNull String owner) { + return owner.replace('/', '.').replace('$','.'); + } + + /** * Computes a user-readable type signature from the given class owner, name * and description. For example, for owner="foo/bar/Foo$Baz", name="foo", * description="(I)V", it returns "void foo.bar.Foo.Bar#foo(int)". @@ -587,7 +599,7 @@ public class ClassContext extends Context { } if (owner != null) { - sb.append(owner.replace('/', '.').replace('$','.')); + sb.append(getFqcn(owner)); } if (name != null) { sb.append('#'); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java index 1643e58..a434359 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java @@ -22,6 +22,7 @@ import static com.android.tools.lint.detector.api.LintConstants.ATTR_CLASS; import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME; import static com.android.tools.lint.detector.api.LintConstants.TARGET_API; import static com.android.tools.lint.detector.api.LintConstants.VIEW_TAG; +import static com.android.tools.lint.detector.api.LintUtils.getNextInstruction; import static com.android.tools.lint.detector.api.Location.SearchDirection.BACKWARD; import static com.android.tools.lint.detector.api.Location.SearchDirection.FORWARD; import static com.android.tools.lint.detector.api.Location.SearchDirection.NEAREST; @@ -49,8 +50,10 @@ import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.FieldInsnNode; import org.objectweb.asm.tree.InsnList; +import org.objectweb.asm.tree.IntInsnNode; import org.objectweb.asm.tree.LdcInsnNode; import org.objectweb.asm.tree.LocalVariableNode; +import org.objectweb.asm.tree.LookupSwitchInsnNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; import org.w3c.dom.Attr; @@ -68,7 +71,6 @@ import java.util.List; */ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassScanner { private static final boolean AOSP_BUILD = System.getenv("ANDROID_BUILD_TOP") != null; //$NON-NLS-1$ - private static final String TARGET_API_VMSIG = '/' + TARGET_API + ';'; /** Accessing an unsupported API */ public static final Issue UNSUPPORTED = Issue.create("NewApi", //$NON-NLS-1$ @@ -95,6 +97,10 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc .addAnalysisScope(Scope.RESOURCE_FILE_SCOPE) .addAnalysisScope(Scope.CLASS_FILE_SCOPE); + private static final String TARGET_API_VMSIG = '/' + TARGET_API + ';'; + private static final String SWITCH_TABLE_PREFIX = "$SWITCH_TABLE$"; //$NON-NLS-1$ + private static final String ORDINAL_METHOD = "ordinal"; //$NON-NLS-1$ + private ApiLookup mApiDatabase; private int mMinApi = -1; @@ -251,7 +257,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc // ---- Implements ClassScanner ---- - @SuppressWarnings("rawtypes") + @SuppressWarnings("rawtypes") // ASM API @Override public void checkClass(final @NonNull ClassContext context, @NonNull ClassNode classNode) { if (mApiDatabase == null) { @@ -262,6 +268,9 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc return; } + // Requires util package (add prebuilts/tools/common/asm-tools/asm-debug-all-4.0.jar) + //classNode.accept(new TraceClassVisitor(new PrintWriter(System.out))); + int classMinSdk = getClassMinSdk(context, classNode); if (classMinSdk == -1) { classMinSdk = getMinSdk(context); @@ -289,7 +298,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc String className = desc.substring(1, desc.length() - 1); int api = mApiDatabase.getClassVersion(className); if (api > minSdk) { - String fqcn = className.replace('/', '.').replace('$', '.'); + String fqcn = ClassContext.getFqcn(className); String message = String.format( "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); @@ -313,7 +322,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc String type = signature.substring(args + 2, signature.length() - 1); int api = mApiDatabase.getClassVersion(type); if (api > minSdk) { - String fqcn = type.replace('/', '.').replace('$', '.'); + String fqcn = ClassContext.getFqcn(type); String message = String.format( "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); @@ -343,15 +352,37 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc while (owner != null) { int api = mApiDatabase.getCallVersion(owner, name, desc); if (api > minSdk) { + if (method.name.startsWith(SWITCH_TABLE_PREFIX)) { + // We're in a compiler-generated method to generate an + // array indexed by enum ordinal values to enum values. The enum + // itself must be requiring a higher API number than is + // currently used, but the call site for the switch statement + // will also be referencing it, so no need to report these + // calls. + break; + } String fqcn; if (CONSTRUCTOR_NAME.equals(name)) { - fqcn = "new " + owner.replace('/', '.'); //$NON-NLS-1$ + fqcn = "new " + ClassContext.getFqcn(owner); //$NON-NLS-1$ } else { - fqcn = owner.replace('/', '.') + '#' + name; + fqcn = ClassContext.getFqcn(owner) + '#' + name; } String message = String.format( "Call requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); + + if (name.equals(ORDINAL_METHOD) + && instruction.getNext() != null + && instruction.getNext().getNext() != null + && instruction.getNext().getOpcode() == Opcodes.IALOAD + && instruction.getNext().getNext().getOpcode() + == Opcodes.TABLESWITCH) { + message = String.format( + "Enum for switch requires API level %1$d " + + "(current min is %2$d): %3$s", + api, minSdk, ClassContext.getFqcn(owner)); + } + report(context, message, node, method, name, null, SearchHints.create(FORWARD).matchJavaSymbol()); } @@ -373,7 +404,12 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc String owner = node.owner; int api = mApiDatabase.getFieldVersion(owner, name); if (api > minSdk) { - String fqcn = owner.replace('/', '.') + '#' + name; + if (method.name.startsWith(SWITCH_TABLE_PREFIX)) { + checkSwitchBlock(context, classNode, node, method, name, owner, + api, minSdk); + continue; + } + String fqcn = ClassContext.getFqcn(owner) + '#' + name; String message = String.format( "Field requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); @@ -388,7 +424,7 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc int api = mApiDatabase.getClassVersion(className); if (api > minSdk) { - String fqcn = className.replace('/', '.'); + String fqcn = ClassContext.getFqcn(className); String message = String.format( "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); @@ -402,6 +438,129 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc } } + @SuppressWarnings("rawtypes") // ASM API + private void checkSwitchBlock(ClassContext context, ClassNode classNode, FieldInsnNode field, + MethodNode method, String name, String owner, int api, int minSdk) { + // Switch statements on enums are tricky. The compiler will generate a method + // which returns an array of the enum constants, indexed by their ordinal() values. + // However, we only want to complain if the code is actually referencing one of + // the non-available enum fields. + // + // For the android.graphics.PorterDuff.Mode enum for example, the first few items + // in the array are populated like this: + // + // L0 + // ALOAD 0 + // GETSTATIC android/graphics/PorterDuff$Mode.ADD : Landroid/graphics/PorterDuff$Mode; + // INVOKEVIRTUAL android/graphics/PorterDuff$Mode.ordinal ()I + // ICONST_1 + // IASTORE + // L1 + // GOTO L3 + // L2 + // FRAME FULL [[I] [java/lang/NoSuchFieldError] + // POP + // L3 + // FRAME SAME + // ALOAD 0 + // GETSTATIC android/graphics/PorterDuff$Mode.CLEAR : Landroid/graphics/PorterDuff$Mode; + // INVOKEVIRTUAL android/graphics/PorterDuff$Mode.ordinal ()I + // ICONST_2 + // IASTORE + // ... + // So if we for example find that the "ADD" field isn't accessible, since it requires + // API 11, we need to + // (1) First find out what its ordinal number is. We can look at the following + // instructions to discover this; it's the "ICONST_1" and "IASTORE" instructions. + // (After ICONST_5 it moves on to BIPUSH 6, BIPUSH 7, etc.) + // (2) Find the corresponding *usage* of this switch method. For the above enum, + // the switch ordinal lookup method will be called + // "$SWITCH_TABLE$android$graphics$PorterDuff$Mode" with desc "()[I". + // This means we will be looking for an invocation in some other method which looks + // like this: + // INVOKESTATIC (current class).$SWITCH_TABLE$android$graphics$PorterDuff$Mode ()[I + // (obviously, it can be invoked more than once) + // Note that it can be used more than once in this class and all sites should be + // checked! + // (3) Look up the corresponding table switch, which should look something like this: + // INVOKESTATIC (current class).$SWITCH_TABLE$android$graphics$PorterDuff$Mode ()[I + // ALOAD 0 + // INVOKEVIRTUAL android/graphics/PorterDuff$Mode.ordinal ()I + // IALOAD + // LOOKUPSWITCH + // 2: L1 + // 11: L2 + // default: L3 + // Here we need to see if the LOOKUPSWITCH instruction is referencing our target + // case. Above we were looking for the "ADD" case which had ordinal 1. Since this + // isn't explicitly referenced, we can ignore this field reference. + AbstractInsnNode next = field.getNext(); + if (next == null || next.getOpcode() != Opcodes.INVOKEVIRTUAL) { + return; + } + next = next.getNext(); + if (next == null) { + return; + } + int ordinal = -1; + switch (next.getOpcode()) { + case Opcodes.ICONST_0: ordinal = 0; break; + case Opcodes.ICONST_1: ordinal = 1; break; + case Opcodes.ICONST_2: ordinal = 2; break; + case Opcodes.ICONST_3: ordinal = 3; break; + case Opcodes.ICONST_4: ordinal = 4; break; + case Opcodes.ICONST_5: ordinal = 5; break; + case Opcodes.BIPUSH: { + IntInsnNode iin = (IntInsnNode) next; + ordinal = iin.operand; + break; + } + default: + return; + } + + // Find usages of this call site + List methodList = classNode.methods; + for (Object m : methodList) { + InsnList nodes = ((MethodNode) m).instructions; + for (int i = 0, n = nodes.size(); i < n; i++) { + AbstractInsnNode instruction = nodes.get(i); + if (instruction.getOpcode() != Opcodes.INVOKESTATIC){ + continue; + } + MethodInsnNode node = (MethodInsnNode) instruction; + if (node.name.equals(method.name) + && node.desc.equals(method.desc) + && node.owner.equals(classNode.name)) { + // Find lookup switch + AbstractInsnNode target = getNextInstruction(node); + while (target != null) { + if (target.getOpcode() == Opcodes.LOOKUPSWITCH) { + LookupSwitchInsnNode lookup = (LookupSwitchInsnNode) target; + @SuppressWarnings("unchecked") // ASM API + List<Integer> keys = lookup.keys; + if (keys != null && keys.contains(ordinal)) { + String fqcn = ClassContext.getFqcn(owner) + '#' + name; + String message = String.format( + "Enum value requires API level %1$d " + + "(current min is %2$d): %3$s", + api, minSdk, fqcn); + report(context, message, lookup, (MethodNode) m, name, null, + SearchHints.create(FORWARD).matchJavaSymbol()); + + // Break out of the inner target search only; the switch + // statement could be used in other places in this class as + // well and we want to report all problematic usages. + break; + } + } + target = getNextInstruction(target); + } + } + } + } + } + /** * Return the {@code @TargeTApi} level to use for the given {@code classNode}; * this will be the {@code @TargetApi} annotation on the class, or any outer diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java index f455f84..9dc5f3a 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java @@ -140,7 +140,7 @@ public class ApiDetectorTest extends AbstractCheckTest { "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" + - "src/foo/bar/ApiCallTest.java:41: Error: Field requires API level 11 (current min is 1): android.graphics.PorterDuff$Mode#OVERLAY [NewApi]\n" + + "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" + "src/foo/bar/ApiCallTest.java:45: Error: Class requires API level 14 (current min is 1): android.widget.GridLayout [NewApi]\n" + @@ -189,7 +189,7 @@ public class ApiDetectorTest extends AbstractCheckTest { "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" + + "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" + "src/foo/bar/ApiCallTest.java:45: Error: Class requires API level 14 (current min is 2): android.widget.GridLayout [NewApi]\n" + @@ -235,7 +235,7 @@ public class ApiDetectorTest extends AbstractCheckTest { "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" + + "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" + "src/foo/bar/ApiCallTest.java:45: Error: Class requires API level 14 (current min is 4): android.widget.GridLayout [NewApi]\n" + @@ -272,7 +272,7 @@ public class ApiDetectorTest extends AbstractCheckTest { "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" + + "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" + "src/foo/bar/ApiCallTest.java:45: Error: Class requires API level 14 (current min is 10): android.widget.GridLayout [NewApi]\n" + @@ -424,7 +424,7 @@ public class ApiDetectorTest extends AbstractCheckTest { "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" + + "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" + @@ -560,4 +560,29 @@ public class ApiDetectorTest extends AbstractCheckTest { "apicheck/ApiCallTest7.class.data=>bin/classes/test/pkg/ApiCallTest7.class" )); } + + public void testEnums() throws Exception { + // See http://code.google.com/p/android/issues/detail?id=36951 + assertEquals( + "src/test/pkg/TestEnum.java:26: Error: Enum value requires API level 11 (current min is 4): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + + " case OVERLAY: {\n" + + " ~~~~~~~\n" + + "src/test/pkg/TestEnum.java:37: Error: Enum value requires API level 11 (current min is 4): android.graphics.PorterDuff.Mode#OVERLAY [NewApi]\n" + + " case OVERLAY: {\n" + + " ~~~~~~~\n" + + "src/test/pkg/TestEnum.java:61: Error: Class requires API level 11 (current min is 4): android.renderscript.Element.DataType [NewApi]\n" + + " switch (type) {\n" + + " ^\n" + + "src/test/pkg/TestEnum.java:61: Error: Enum for switch requires API level 11 (current min is 4): android.renderscript.Element.DataType [NewApi]\n" + + " switch (type) {\n" + + " ^\n" + + "4 errors, 0 warnings\n", + + lintProject( + "apicheck/classpath=>.classpath", + "apicheck/minsdk4.xml=>AndroidManifest.xml", + "apicheck/TestEnum.java.txt=>src/test/pkg/TestEnum.java", + "apicheck/TestEnum.class.data=>bin/classes/test/pkg/TestEnum.class" + )); + } } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.class.data Binary files differnew file mode 100644 index 0000000..fa676b9 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.class.data diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.java.txt new file mode 100644 index 0000000..2d68b28 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/TestEnum.java.txt @@ -0,0 +1,67 @@ +package test.pkg; + +import android.annotation.SuppressLint; +import android.graphics.Bitmap.CompressFormat; +import android.graphics.PorterDuff; + +@SuppressWarnings("incomplete-switch") +public class TestEnum { + public static void test1(final CompressFormat format) { + switch (format) { + case JPEG: { + System.out.println("jpeg"); + break; + } + default: { + System.out.println("Default"); + } + } + } + + public static void test2(final PorterDuff.Mode mode) { + switch (mode) { + case CLEAR: { + System.out.println("clear"); + } + case OVERLAY: { + System.out.println("add"); + break; + } + } + + // Second usage: should also complain here + switch (mode) { + case CLEAR: { + System.out.println("clear"); + } + case OVERLAY: { + System.out.println("add"); + break; + } + } + } + + @SuppressLint("NewApi") + public static void test3(PorterDuff.Mode mode) { + // Third usage: no complaint because it's suppressed + switch (mode) { + case CLEAR: { + System.out.println("clear"); + } + case OVERLAY: { + System.out.println("add"); + break; + } + } + } + + public static void test4(final android.renderscript.Element.DataType type) { + // Switch usage where the whole underlying enum requires a higher API level: + // test customized error message + switch (type) { + case RS_FONT: { + System.out.println("font"); + } + } + } +} diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ClassContextTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ClassContextTest.java index 1f9bd2e..71c3486 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ClassContextTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/detector/api/ClassContextTest.java @@ -33,4 +33,8 @@ public class ClassContextTest extends TestCase { assertEquals("foo/bar/Foo$Bar", ClassContext.getInternalName("foo.bar.Foo.Bar")); } + + public void testGetFqcn() { + assertEquals("foo.bar.Foo.Bar", ClassContext.getFqcn("foo/bar/Foo$Bar")); + } } |