17782530: Annotations using retention policy source remain in .class

Change-Id: Ia62e0d6ef5024515567be6564197e723f0013ee0

tools/rmtypedefs/.idea/copyright/profiles_settings.xml

@@ -1,5 +1,3 @@
 <component name="CopyrightManager">
-  <settings default="">
+  <settings default="" />
-    <module2copyright />
-  </settings>
 </component>

tools/rmtypedefs/.idea/misc.xml

@@ -3,7 +3,7 @@
   <component name="EntryPointsManager">
     <entry_points version="2.0" />
   </component>
-  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="1.6" project-jdk-type="JavaSDK">
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="OpenJDK 1.7" project-jdk-type="JavaSDK">
     <output url="file://$PROJECT_DIR$/out" />
   </component>
 </project>

tools/rmtypedefs/rmtypedefs.iml

@@ -31,6 +31,26 @@
         </SOURCES>
       </library>
     </orderEntry>
+    <orderEntry type="module-library">
+      <library>
+        <CLASSES>
+          <root url="jar://$MODULE_DIR$/../../../prebuilts/tools/common/m2/repository/junit/junit/3.8.1/junit-3.8.1.jar!/" />
+        </CLASSES>
+        <JAVADOC />
+        <SOURCES>
+          <root url="jar://$MODULE_DIR$/../../../prebuilts/tools/common/m2/repository/junit/junit/3.8.1/junit-3.8.1-sources.jar!/" />
+        </SOURCES>
+      </library>
+    </orderEntry>
+    <orderEntry type="module-library">
+      <library>
+        <CLASSES>
+          <root url="jar://$MODULE_DIR$/../../../prebuilts/tools/common/m2/repository/org/eclipse/jdt/core/compiler/ecj/4.2.2/ecj-4.2.2.jar!/" />
+        </CLASSES>
+        <JAVADOC />
+        <SOURCES />
+      </library>
+    </orderEntry>
   </component>
 </module>
 

tools/rmtypedefs/src/com/android/tools/rmtypedefs/RmTypeDefs.java

@@ -16,17 +16,21 @@
 
 package com.android.tools.rmtypedefs;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import com.google.common.io.Files;
 
 import org.objectweb.asm.AnnotationVisitor;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 import static org.objectweb.asm.Opcodes.ASM4;
 
@@ -50,6 +54,10 @@ public class RmTypeDefs {
     private boolean mHaveError;
     private boolean mDryRun;
 
+    private Set<String> mAnnotationNames = Sets.newHashSet();
+    private List<File> mAnnotationClassFiles = Lists.newArrayList();
+    private Set<File> mAnnotationOuterClassFiles = Sets.newHashSet();
+
     public static void main(String[] args) {
         new RmTypeDefs().run(args);
     }
@@ -93,19 +101,30 @@ public class RmTypeDefs {
             System.out.println("Deleting @IntDef and @StringDef annotation class files");
         }
 
+        // Record typedef annotation names and files
         for (File dir : dirs) {
-            find(dir);
+            checkFile(dir);
         }
 
+        // Rewrite the .class files for any classes that *contain* typedefs as innerclasses
+        rewriteOuterClasses();
+
+        // Removes the actual .class files for the typedef annotations
+        deleteAnnotationClasses();
+
         System.exit(mHaveError ? -1 : 0);
     }
 
-    private void find(File file) {
+    /**
+     * Visits the given directory tree recursively and calls {@link #checkClass(java.io.File)}
+     * for any .class files encountered
+     */
+    private void checkFile(File file) {
         if (file.isDirectory()) {
             File[] files = file.listFiles();
             if (files != null) {
                 for (File f : files) {
-                    find(f);
+                    checkFile(f);
                 }
             }
         } else if (file.isFile()) {
@@ -119,11 +138,15 @@ public class RmTypeDefs {
         }
     }
 
+    /**
+     * Checks the given .class file to see if it's a typedef annotation, and if so
+     * records that fact by calling {@link #addTypeDef(String, java.io.File)}
+     */
     private void checkClass(File file) {
         try {
             byte[] bytes = Files.toByteArray(file);
             ClassReader classReader = new ClassReader(bytes);
-            classReader.accept(new MyVisitor(file), 0);
+            classReader.accept(new TypeDefVisitor(file), 0);
         } catch (IOException e) {
             System.err.println("Could not read " + file + ": " + e.getLocalizedMessage());
             System.exit(1);
@@ -142,9 +165,100 @@ public class RmTypeDefs {
         out.println("  -q,--quiet                 quiet");
         out.println("  -v,--verbose               verbose");
         out.println("  -n,--dry-run               dry-run only, leaves files alone");
+        out.println("  --verify                   run extra diagnostics to verify file integrity");
     }
 
-    private class MyVisitor extends ClassVisitor {
+    /**
+     * Records the given class name (internal name) and class file path as corresponding to a
+     * typedef annotation
+     * */
+    private void addTypeDef(String name, File file) {
+        mAnnotationClassFiles.add(file);
+        mAnnotationNames.add(name);
+
+        String fileName = file.getName();
+        int index = fileName.lastIndexOf('$');
+        if (index != -1) {
+            File parentFile = file.getParentFile();
+            assert parentFile != null : file;
+            File container = new File(parentFile, fileName.substring(0, index) + ".class");
+            if (container.exists()) {
+                mAnnotationOuterClassFiles.add(file);
+            } else {
+                System.err.println("Warning: Could not find outer class " + container
+                        + " for typedef " + file);
+                mHaveError = true;
+            }
+        }
+    }
+
+    /**
+     * Rewrites the outer classes containing the typedefs such that they no longer refer to
+     * the (now removed) typedef annotation inner classes
+     */
+    private void rewriteOuterClasses() {
+        for (File file : mAnnotationOuterClassFiles) {
+            byte[] bytes;
+            try {
+                bytes = Files.toByteArray(file);
+            } catch (IOException e) {
+                System.err.println("Could not read " + file + ": " + e.getLocalizedMessage());
+                mHaveError = true;
+                continue;
+            }
+
+            ClassWriter classWriter = new ClassWriter(ASM4);
+            ClassVisitor classVisitor = new ClassVisitor(ASM4, classWriter) {
+                @Override
+                public void visitInnerClass(String name, String outerName, String innerName,
+                        int access) {
+                    if (!mAnnotationNames.contains(name)) {
+                        super.visitInnerClass(name, outerName, innerName, access);
+                    }
+                }
+            };
+            ClassReader reader = new ClassReader(bytes);
+            reader.accept(classVisitor, 0);
+            byte[] rewritten = classWriter.toByteArray();
+            try {
+                Files.write(rewritten, file);
+            } catch (IOException e) {
+                System.err.println("Could not write " + file + ": " + e.getLocalizedMessage());
+                mHaveError = true;
+                //noinspection UnnecessaryContinue
+                continue;
+            }
+        }
+    }
+
+    /**
+     * Performs the actual deletion (or display, if in dry-run mode) of the typedef annotation
+     * files
+     */
+    private void deleteAnnotationClasses() {
+        for (File mFile : mAnnotationClassFiles) {
+            if (mVerbose) {
+                if (mDryRun) {
+                    System.out.println("Would delete " + mFile);
+                } else {
+                    System.out.println("Deleting " + mFile);
+                }
+            }
+            if (!mDryRun) {
+                boolean deleted = mFile.delete();
+                if (!deleted) {
+                    System.err.println("Could not delete " + mFile);
+                    mHaveError = true;
+                }
+            }
+        }
+    }
+
+    /**
+     * Visitor which visits .class files and checks whether each class is a typedef annotation
+     * (and if so, calls {@link #addTypeDef(String, java.io.File)}
+     */
+    private class TypeDefVisitor extends ClassVisitor {
 
         /** Class file name */
         private File mFile;
@@ -161,7 +275,7 @@ public class RmTypeDefs {
         /** Does the annotation have source retention? Only applies if {@link #mAnnotation} */
         private boolean mSourceRetention;
 
-        public MyVisitor(File file) {
+        public TypeDefVisitor(File file) {
             super(ASM4);
             mFile = file;
         }
@@ -203,20 +317,8 @@ public class RmTypeDefs {
                             + "with @Retention(RetentionPolicy.SOURCE)");
                     mHaveError = true;
                 }
-                if (mVerbose) {
+
-                    if (mDryRun) {
+                addTypeDef(mName, mFile);
-                        System.out.println("Would delete " + mFile);
-                    } else {
-                        System.out.println("Deleting " + mFile);
-                    }
-                }
-                if (!mDryRun) {
-                    boolean deleted = mFile.delete();
-                    if (!deleted) {
-                        System.err.println("Could not delete " + mFile);
-                        mHaveError = true;
-                    }
-                }
             }
         }
     }

tools/rmtypedefs/test/com/android/tools/rmtypedefs/RmTypeDefsTest.java

@@ -0,0 +1,254 @@
+package com.android.tools.rmtypedefs;
+
+import com.google.common.base.Charsets;
+import com.google.common.collect.Lists;
+import com.google.common.io.Files;
+
+import junit.framework.TestCase;
+
+import org.eclipse.jdt.core.compiler.batch.BatchCompiler;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.security.Permission;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+@SuppressWarnings("SpellCheckingInspection")
+public class RmTypeDefsTest extends TestCase {
+    public void test() throws IOException {
+        // Creates a test class containing various typedefs, as well as the @IntDef annotation
+        // itself (to make the test case independent of the SDK), and compiles this using
+        // ECJ. It then runs the RmTypeDefs tool on the resulting output directory, and
+        // finally verifies that the tool exits with a 0 exit code.
+
+        File dir = Files.createTempDir();
+        String testClass = ""
+            + "package test.pkg;\n"
+            + "\n"
+            + "import android.annotation.IntDef;\n"
+            + "\n"
+            + "import java.lang.annotation.Retention;\n"
+            + "import java.lang.annotation.RetentionPolicy;\n"
+            + "\n"
+            + "@SuppressWarnings({\"UnusedDeclaration\",\"JavaDoc\"})\n"
+            + "public class TestClass {\n"
+            + "    /** @hide */\n"
+            + "    @Retention(RetentionPolicy.SOURCE)\n"
+            + "    @IntDef(flag = true,\n"
+            + "            value = {\n"
+            + "                    DISPLAY_USE_LOGO,\n"
+            + "                    DISPLAY_SHOW_HOME,\n"
+            + "                    DISPLAY_HOME_AS_UP,\n"
+            + "                    DISPLAY_SHOW_TITLE,\n"
+            + "                    DISPLAY_SHOW_CUSTOM,\n"
+            + "                    DISPLAY_TITLE_MULTIPLE_LINES\n"
+            + "            })\n"
+            + "    public @interface DisplayOptions {}\n"
+            + "\n"
+            + "    public static final int DISPLAY_USE_LOGO = 0x1;\n"
+            + "    public static final int DISPLAY_SHOW_HOME = 0x2;\n"
+            + "    public static final int DISPLAY_HOME_AS_UP = 0x4;\n"
+            + "    public static final int DISPLAY_SHOW_TITLE = 0x8;\n"
+            + "    public static final int DISPLAY_SHOW_CUSTOM = 0x10;\n"
+            + "    public static final int DISPLAY_TITLE_MULTIPLE_LINES = 0x20;\n"
+            + "\n"
+            + "    public void setDisplayOptions(@DisplayOptions int options) {\n"
+            + "        System.out.println(\"setDisplayOptions \" + options);\n"
+            + "    }\n"
+            + "    public void setDisplayOptions(@DisplayOptions int options, @DisplayOptions int mask) {\n"
+            + "        System.out.println(\"setDisplayOptions \" + options + \", mask=\" + mask);\n"
+            + "    }\n"
+            + "\n"
+            + "    public static class StaticInnerClass {\n"
+            + "        int mViewFlags = 0;\n"
+            + "        static final int VISIBILITY_MASK = 0x0000000C;\n"
+            + "\n"
+            + "        /** @hide */\n"
+            + "        @IntDef({VISIBLE, INVISIBLE, GONE})\n"
+            + "        @Retention(RetentionPolicy.SOURCE)\n"
+            + "        public @interface Visibility {}\n"
+            + "\n"
+            + "        public static final int VISIBLE = 0x00000000;\n"
+            + "        public static final int INVISIBLE = 0x00000004;\n"
+            + "        public static final int GONE = 0x00000008;\n"
+            + "\n"
+            + "        @Visibility\n"
+            + "        public int getVisibility() {\n"
+            + "            return mViewFlags & VISIBILITY_MASK;\n"
+            + "        }\n"
+            + "    }\n"
+            + "\n"
+            + "    public static class Inherits extends StaticInnerClass {\n"
+            + "        @Override\n"
+            + "        @Visibility\n"
+            + "        public int getVisibility() {\n"
+            + "            return 0;\n"
+            + "        }\n"
+            + "    }\n"
+            + "}\n";
+        String intdef = ""
+            + "package android.annotation;\n"
+            + "@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS)\n"
+            + "@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE})\n"
+            + "public @interface IntDef {\n"
+            + "    long[] value() default {};\n"
+            + "    boolean flag() default false;\n"
+            + "}";
+
+        File srcDir = new File(dir, "test" + File.separator + "pkg");
+        boolean mkdirs = srcDir.mkdirs();
+        assertTrue(mkdirs);
+        File srcFile1 = new File(srcDir, "TestClass.java");
+        Files.write(testClass, srcFile1, Charsets.UTF_8);
+
+        srcDir = new File(dir, "android" + File.separator + "annotation");
+        mkdirs = srcDir.mkdirs();
+        assertTrue(mkdirs);
+        File srcFile2 = new File(srcDir, "IntDef.java");
+        Files.write(intdef, srcFile2, Charsets.UTF_8);
+
+        boolean compileSuccessful = BatchCompiler.compile(srcFile1 + " " + srcFile2 +
+                        " -source 1.6 -target 1.6 -nowarn",
+                new PrintWriter(System.out),
+                new PrintWriter(System.err), null);
+        assertTrue(compileSuccessful);
+
+        assertEquals(""
+            + "testDir/\n"
+            + "    testDir/android/\n"
+            + "        testDir/android/annotation/\n"
+            + "            testDir/android/annotation/IntDef.class\n"
+            + "            testDir/android/annotation/IntDef.java\n"
+            + "    testDir/test/\n"
+            + "        testDir/test/pkg/\n"
+            + "            testDir/test/pkg/TestClass$DisplayOptions.class\n"
+            + "            testDir/test/pkg/TestClass$Inherits.class\n"
+            + "            testDir/test/pkg/TestClass$StaticInnerClass$Visibility.class\n"
+            + "            testDir/test/pkg/TestClass$StaticInnerClass.class\n"
+            + "            testDir/test/pkg/TestClass.class\n"
+            + "            testDir/test/pkg/TestClass.java\n",
+            getDirectoryContents(dir));
+
+        // Trap System.exit calls:
+        System.setSecurityManager(new SecurityManager() {
+            @Override
+            public void checkPermission(Permission perm) {
+            }
+            @Override
+            public void checkPermission(Permission perm, Object context) {
+            }
+            @Override
+            public void checkExit(int status) {
+                throw new ExitException(status);
+            }
+        });
+        try {
+            RmTypeDefs.main(new String[]{"--verbose", dir.getPath()});
+        } catch (ExitException e) {
+            assertEquals(0, e.getStatus());
+        }
+        System.setSecurityManager(null);
+
+        // TODO: check that the classes are identical
+        // BEFORE removal
+
+        assertEquals(""
+                + "testDir/\n"
+                + "    testDir/android/\n"
+                + "        testDir/android/annotation/\n"
+                + "            testDir/android/annotation/IntDef.class\n"
+                + "            testDir/android/annotation/IntDef.java\n"
+                + "    testDir/test/\n"
+                + "        testDir/test/pkg/\n"
+                + "            testDir/test/pkg/TestClass$Inherits.class\n"
+                + "            testDir/test/pkg/TestClass$StaticInnerClass.class\n"
+                + "            testDir/test/pkg/TestClass.class\n"
+                + "            testDir/test/pkg/TestClass.java\n",
+                getDirectoryContents(dir));
+
+
+        deleteDir(dir);
+    }
+
+    String getDirectoryContents(File root) {
+        StringBuilder sb = new StringBuilder();
+        list(sb, root, "", 0, "testDir");
+        return sb.toString();
+    }
+
+    private void list(StringBuilder sb, File file, String prefix, int depth, String rootName) {
+        for (int i = 0; i < depth; i++) {
+            sb.append("    ");
+        }
+
+        if (!prefix.isEmpty()) {
+            sb.append(prefix);
+        }
+        String fileName = file.getName();
+        if (depth == 0 && rootName != null) { // avoid temp-name
+            fileName = rootName;
+        }
+        sb.append(fileName);
+        if (file.isDirectory()) {
+            sb.append('/');
+            sb.append('\n');
+            File[] files = file.listFiles();
+            if (files != null) {
+                List<File> children = Lists.newArrayList();
+                Collections.addAll(children, files);
+                Collections.sort(children, new Comparator<File>() {
+                    @Override
+                    public int compare(File o1, File o2) {
+                        return o1.getName().compareTo(o2.getName());
+                    }
+                });
+                prefix = prefix + fileName + "/";
+                for (File child : children) {
+                    list(sb, child, prefix, depth + 1, rootName);
+                }
+            }
+        } else {
+            sb.append('\n');
+        }
+    }
+
+    /**
+     * Recursive delete directory. Mostly for fake SDKs.
+     *
+     * @param root directory to delete
+     */
+    @SuppressWarnings("ResultOfMethodCallIgnored")
+    private static void deleteDir(File root) {
+        if (root.exists()) {
+            File[] files = root.listFiles();
+            if (files != null) {
+                for (File file : files) {
+                    if (file.isDirectory()) {
+                        deleteDir(file);
+                    } else {
+                        file.delete();
+                    }
+                }
+            }
+            root.delete();
+        }
+    }
+
+    private static class ExitException extends SecurityException {
+        private static final long serialVersionUID = 1L;
+
+        private final int mStatus;
+
+        public ExitException(int status) {
+            super("Unit test");
+            mStatus = status;
+        }
+
+        public int getStatus() {
+            return mStatus;
+        }
+    }
+}