Make sure that annotation extraction includes merges

Change-Id: I033c1da7d3beed67b0b1f61c809a411b4d7e792d
diff --git a/src/main/java/com/android/tools/metalava/AnnotationsMerger.kt b/src/main/java/com/android/tools/metalava/AnnotationsMerger.kt
index fce5a77..97b8cf3 100644
--- a/src/main/java/com/android/tools/metalava/AnnotationsMerger.kt
+++ b/src/main/java/com/android/tools/metalava/AnnotationsMerger.kt
@@ -55,6 +55,7 @@
 import com.android.tools.metalava.model.Item
 import com.android.tools.metalava.model.MethodItem
 import com.android.tools.metalava.model.psi.PsiAnnotationItem
+import com.android.tools.metalava.model.visitors.ApiVisitor
 import com.android.utils.XmlUtils
 import com.google.common.base.Charsets
 import com.google.common.io.ByteStreams
@@ -351,11 +352,6 @@
             }
 
             val annotationItem = createAnnotation(annotationElement) ?: continue
-
-            if (!AnnotationItem.isSignificantAnnotation(annotationItem.qualifiedName())) {
-                continue
-            }
-
             item.mutableModifiers().addAnnotation(annotationItem)
             count++
         }
@@ -422,10 +418,18 @@
 
                         // Attempt to sort in reflection order
                         if (!found && reflectionFields != null) {
+                            val filterEmit = ApiVisitor(codebase).filterEmit
+
                             // Attempt with reflection
                             var first = true
                             for (field in reflectionFields) {
                                 if (field.type == Integer.TYPE || field.type == Int::class.javaPrimitiveType) {
+                                    // Make sure this field is included in our API too
+                                    val fieldItem = codebase.findClass(clsName)?.findField(field.name)
+                                    if (fieldItem == null || !filterEmit.test(fieldItem)) {
+                                        continue
+                                    }
+
                                     if (first) {
                                         first = false
                                     } else {
diff --git a/src/main/java/com/android/tools/metalava/ExtractAnnotations.kt b/src/main/java/com/android/tools/metalava/ExtractAnnotations.kt
index 9661815..79a12f0 100644
--- a/src/main/java/com/android/tools/metalava/ExtractAnnotations.kt
+++ b/src/main/java/com/android/tools/metalava/ExtractAnnotations.kt
@@ -75,9 +75,9 @@
     private val annotationLookup = AnnotationLookup()
 
     private data class AnnotationHolder(
-        val annotationClass: ClassItem,
+        val annotationClass: ClassItem?,
         val annotationItem: AnnotationItem,
-        val uAnnotation: UAnnotation
+        val uAnnotation: UAnnotation?
     )
 
     private val classToAnnotationHolder = mutableMapOf<String, AnnotationHolder>()
@@ -175,6 +175,11 @@
                 qualifiedName.startsWith(ANDROID_ANNOTATION_PREFIX) ||
                 qualifiedName.startsWith(ANDROID_SUPPORT_ANNOTATION_PREFIX)
             ) {
+                if (annotation.isTypeDefAnnotation()) {
+                    // Imported typedef
+                    return AnnotationHolder(null, annotation, null)
+                }
+
                 continue
             }
 
@@ -358,7 +363,6 @@
             is ParameterItem -> {
                 return containingMethod().getExternalAnnotationSignature() + ' '.toString() + this.parameterIndex
             }
-
         }
 
         return null
@@ -369,10 +373,8 @@
         item: Item,
         annotationHolder: AnnotationHolder
     ) {
-        val uAnnotation = annotationHolder.uAnnotation
         val annotationItem = annotationHolder.annotationItem
         val qualifiedName = annotationItem.qualifiedName()
-        var attributes = uAnnotation.attributeValues
 
         writer.mark()
         writer.print("    <annotation name=\"")
@@ -381,92 +383,103 @@
         writer.print("\">")
         writer.println()
 
-        //noinspection PointlessBooleanExpression,ConstantConditions
-        if (attributes.size > 1 && sortAnnotations) {
-            // Ensure mutable
-            attributes = ArrayList(attributes)
+        val uAnnotation = annotationHolder.uAnnotation
+            ?: if (annotationItem is PsiAnnotationItem) {
+                // Imported annotation
+                JavaUAnnotation.wrap(annotationItem.psiAnnotation)
+            } else {
+                null
+            }
+        if (uAnnotation != null) {
+            var attributes = uAnnotation.attributeValues
 
-            // Ensure that the value attribute is written first
-            attributes.sortedWith(object : Comparator<UNamedExpression> {
-                private fun getName(pair: UNamedExpression): String {
-                    val name = pair.name
-                    return name ?: SdkConstants.ATTR_VALUE
-                }
+            // noinspection PointlessBooleanExpression,ConstantConditions
+            if (attributes.size > 1 && sortAnnotations) {
+                // Ensure mutable
+                attributes = ArrayList(attributes)
 
-                private fun rank(pair: UNamedExpression): Int {
-                    return if (SdkConstants.ATTR_VALUE == getName(pair)) -1 else 0
-                }
+                // Ensure that the value attribute is written first
+                attributes.sortedWith(object : Comparator<UNamedExpression> {
+                    private fun getName(pair: UNamedExpression): String {
+                        val name = pair.name
+                        return name ?: SdkConstants.ATTR_VALUE
+                    }
 
-                override fun compare(o1: UNamedExpression, o2: UNamedExpression): Int {
-                    val r1 = rank(o1)
-                    val r2 = rank(o2)
-                    val delta = r1 - r2
-                    return if (delta != 0) {
-                        delta
-                    } else getName(o1).compareTo(getName(o2))
-                }
-            })
-        }
+                    private fun rank(pair: UNamedExpression): Int {
+                        return if (SdkConstants.ATTR_VALUE == getName(pair)) -1 else 0
+                    }
 
-        if (attributes.size == 1 && Extractor.REQUIRES_PERMISSION.isPrefix(qualifiedName, true)) {
-            val expression = attributes[0].expression
-            if (expression is UAnnotation) {
-                // The external annotations format does not allow for nested/complex annotations.
-                // However, these special annotations (@RequiresPermission.Read,
-                // @RequiresPermission.Write, etc) are known to only be simple containers with a
-                // single permission child, so instead we "inline" the content:
-                //  @Read(@RequiresPermission(allOf={P1,P2},conditional=true)
-                //     =>
-                //      @RequiresPermission.Read(allOf({P1,P2},conditional=true)
-                // That's setting attributes that don't actually exist on the container permission,
-                // but we'll counteract that on the read-annotations side.
-                val annotation = expression as UAnnotation
-                attributes = annotation.attributeValues
-            } else if (expression is JavaUAnnotationCallExpression) {
-                val annotation = expression.uAnnotation
-                attributes = annotation.attributeValues
-            } else if (expression is UastEmptyExpression && attributes[0].psi is PsiNameValuePair) {
-                val memberValue = (attributes[0].psi as PsiNameValuePair).value
-                if (memberValue is PsiAnnotation) {
-                    val annotation = JavaUAnnotation.wrap(memberValue)
+                    override fun compare(o1: UNamedExpression, o2: UNamedExpression): Int {
+                        val r1 = rank(o1)
+                        val r2 = rank(o2)
+                        val delta = r1 - r2
+                        return if (delta != 0) {
+                            delta
+                        } else getName(o1).compareTo(getName(o2))
+                    }
+                })
+            }
+
+            if (attributes.size == 1 && Extractor.REQUIRES_PERMISSION.isPrefix(qualifiedName, true)) {
+                val expression = attributes[0].expression
+                if (expression is UAnnotation) {
+                    // The external annotations format does not allow for nested/complex annotations.
+                    // However, these special annotations (@RequiresPermission.Read,
+                    // @RequiresPermission.Write, etc) are known to only be simple containers with a
+                    // single permission child, so instead we "inline" the content:
+                    //  @Read(@RequiresPermission(allOf={P1,P2},conditional=true)
+                    //     =>
+                    //      @RequiresPermission.Read(allOf({P1,P2},conditional=true)
+                    // That's setting attributes that don't actually exist on the container permission,
+                    // but we'll counteract that on the read-annotations side.
+                    val annotation = expression as UAnnotation
                     attributes = annotation.attributeValues
+                } else if (expression is JavaUAnnotationCallExpression) {
+                    val annotation = expression.uAnnotation
+                    attributes = annotation.attributeValues
+                } else if (expression is UastEmptyExpression && attributes[0].psi is PsiNameValuePair) {
+                    val memberValue = (attributes[0].psi as PsiNameValuePair).value
+                    if (memberValue is PsiAnnotation) {
+                        val annotation = JavaUAnnotation.wrap(memberValue)
+                        attributes = annotation.attributeValues
+                    }
                 }
             }
-        }
 
-        val inlineConstants = isInlinedConstant(annotationItem)
-        var empty = true
-        for (pair in attributes) {
-            val expression = pair.expression
-            val value = attributeString(expression, inlineConstants) ?: continue
-            empty = false
-            var name = pair.name
-            if (name == null) {
-                name = SdkConstants.ATTR_VALUE // default name
+            val inlineConstants = isInlinedConstant(annotationItem)
+            var empty = true
+            for (pair in attributes) {
+                val expression = pair.expression
+                val value = attributeString(expression, inlineConstants) ?: continue
+                empty = false
+                var name = pair.name
+                if (name == null) {
+                    name = SdkConstants.ATTR_VALUE // default name
+                }
+
+                // Platform typedef annotations now declare a prefix attribute for
+                // documentation generation purposes; this should not be part of the
+                // extracted metadata.
+                if (("prefix" == name || "suffix" == name) && annotationItem.isTypeDefAnnotation()) {
+                    reporter.report(
+                        Errors.SUPERFLUOUS_PREFIX, item,
+                        "Superfluous $name attribute on typedef"
+                    )
+                    continue
+                }
+
+                writer.print("      <val name=\"")
+                writer.print(name)
+                writer.print("\" val=\"")
+                writer.print(escapeXml(value))
+                writer.println("\" />")
             }
 
-            // Platform typedef annotations now declare a prefix attribute for
-            // documentation generation purposes; this should not be part of the
-            // extracted metadata.
-            if (("prefix" == name || "suffix" == name) && annotationItem.isTypeDefAnnotation()) {
-                reporter.report(
-                    Errors.SUPERFLUOUS_PREFIX, item,
-                    "Superfluous $name attribute on typedef"
-                )
-                continue
+            if (empty) {
+                // All items were filtered out: don't write the annotation at all
+                writer.reset()
+                return
             }
-
-            writer.print("      <val name=\"")
-            writer.print(name)
-            writer.print("\" val=\"")
-            writer.print(escapeXml(value))
-            writer.println("\" />")
-        }
-
-        if (empty) {
-            // All items were filtered out: don't write the annotation at all
-            writer.reset()
-            return
         }
 
         writer.println("    </annotation>")
@@ -483,7 +496,9 @@
     }
 
     private fun appendExpression(
-        sb: StringBuilder, expression: UExpression, inlineConstants: Boolean
+        sb: StringBuilder,
+        expression: UExpression,
+        inlineConstants: Boolean
     ): Boolean {
         if (expression.isArrayInitializer()) {
             val call = expression as UCallExpression
@@ -550,7 +565,7 @@
                 }
                 return false
             } else {
-                warning("Unexpected reference to " + resolved!!)
+                warning("Unexpected reference to $expression")
                 return false
             }
         } else if (expression is ULiteralExpression) {
@@ -574,12 +589,7 @@
             }
         }
 
-        warning(
-            ("Unexpected annotation expression of type "
-                + expression.javaClass
-                + " and is "
-                + expression)
-        )
+        warning("Unexpected annotation expression of type ${expression.javaClass} and is $expression")
 
         return false
     }
diff --git a/src/main/java/com/android/tools/metalava/SignatureWriter.kt b/src/main/java/com/android/tools/metalava/SignatureWriter.kt
index dda47cd..7fca3d8 100644
--- a/src/main/java/com/android/tools/metalava/SignatureWriter.kt
+++ b/src/main/java/com/android/tools/metalava/SignatureWriter.kt
@@ -149,7 +149,8 @@
             includeDeprecated = true,
             includeAnnotations = compatibility.annotationsInSignatures,
             skipNullnessAnnotations = options.outputKotlinStyleNulls,
-            omitCommonPackages = options.omitCommonPackages
+            omitCommonPackages = options.omitCommonPackages,
+            onlyIncludeSignatureAnnotations = true
         )
     }
 
diff --git a/src/main/java/com/android/tools/metalava/StubWriter.kt b/src/main/java/com/android/tools/metalava/StubWriter.kt
index e30c0a7..caf91d8 100644
--- a/src/main/java/com/android/tools/metalava/StubWriter.kt
+++ b/src/main/java/com/android/tools/metalava/StubWriter.kt
@@ -140,6 +140,7 @@
                 // Some bug in UAST triggers duplicate nullability annotations
                 // here; make sure the are filtered out
                 filterDuplicates = true,
+                onlyIncludeSignatureAnnotations = true,
                 writer = writer
             )
             writer.println("package ${pkg.qualifiedName()};")
@@ -300,7 +301,8 @@
 
         ModifierList.write(
             writer, modifiers, item, removeAbstract = removeAbstract, removeFinal = removeFinal,
-            addPublic = addPublic, includeAnnotations = generateAnnotations
+            addPublic = addPublic, includeAnnotations = generateAnnotations,
+            onlyIncludeSignatureAnnotations = true
         )
     }
 
diff --git a/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt b/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt
index a0dee44..f87833f 100644
--- a/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt
+++ b/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt
@@ -54,7 +54,7 @@
 
     /** Whether this annotation is significant and should be included in signature files, stubs, etc */
     fun isSignificant(): Boolean {
-        return isSignificantAnnotation(qualifiedName() ?: return false)
+        return includeInSignatures(qualifiedName() ?: return false)
     }
 
     /** Attributes of the annotation (may be empty) */
@@ -78,12 +78,12 @@
     /** True if this annotation represents @IntDef, @LongDef or @StringDef */
     fun isTypeDefAnnotation(): Boolean {
         val name = qualifiedName() ?: return false
-        return (INT_DEF_ANNOTATION.isEquals(name)
-            || STRING_DEF_ANNOTATION.isEquals(name)
-            || LONG_DEF_ANNOTATION.isEquals(name)
-            || ANDROID_INT_DEF == name
-            || ANDROID_STRING_DEF == name
-            || ANDROID_LONG_DEF == name)
+        return (INT_DEF_ANNOTATION.isEquals(name) ||
+            STRING_DEF_ANNOTATION.isEquals(name) ||
+            LONG_DEF_ANNOTATION.isEquals(name) ||
+            ANDROID_INT_DEF == name ||
+            ANDROID_STRING_DEF == name ||
+            ANDROID_LONG_DEF == name)
     }
 
     /**
@@ -115,7 +115,7 @@
 
     companion object {
         /** Whether the given annotation name is "significant", e.g. should be included in signature files */
-        fun isSignificantAnnotation(qualifiedName: String?): Boolean {
+        fun includeInSignatures(qualifiedName: String?): Boolean {
             qualifiedName ?: return false
             if (qualifiedName.startsWith(ANDROID_SUPPORT_ANNOTATION_PREFIX) ||
                 qualifiedName.startsWith(ANDROIDX_ANNOTATION_PREFIX)
diff --git a/src/main/java/com/android/tools/metalava/model/ModifierList.kt b/src/main/java/com/android/tools/metalava/model/ModifierList.kt
index f5e1807..44bb063 100644
--- a/src/main/java/com/android/tools/metalava/model/ModifierList.kt
+++ b/src/main/java/com/android/tools/metalava/model/ModifierList.kt
@@ -183,7 +183,9 @@
             omitCommonPackages: Boolean = false,
             removeAbstract: Boolean = false,
             removeFinal: Boolean = false,
-            addPublic: Boolean = false
+            addPublic: Boolean = false,
+            onlyIncludeSignatureAnnotations: Boolean = true
+
         ) {
 
             val list = if (removeAbstract || removeFinal || addPublic) {
@@ -379,7 +381,8 @@
             omitCommonPackages: Boolean = false,
             separateLines: Boolean = false,
             filterDuplicates: Boolean = false,
-            writer: Writer
+            writer: Writer,
+            onlyIncludeSignatureAnnotations: Boolean = true
         ) {
             val annotations = list.annotations()
             if (annotations.isNotEmpty()) {
@@ -390,7 +393,7 @@
                         if (skipNullnessAnnotations) {
                             continue
                         }
-                    } else if (!annotation.isSignificant()) {
+                    } else if (onlyIncludeSignatureAnnotations && !annotation.isSignificant()) {
                         continue
                     }
 
diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiMethodItem.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiMethodItem.kt
index 813d675..a54dc24 100644
--- a/src/main/java/com/android/tools/metalava/model/psi/PsiMethodItem.kt
+++ b/src/main/java/com/android/tools/metalava/model/psi/PsiMethodItem.kt
@@ -249,7 +249,8 @@
         val modifierString = StringWriter()
         ModifierList.write(
             modifierString, method.modifiers, method, removeAbstract = false,
-            removeFinal = false, addPublic = true
+            removeFinal = false, addPublic = true,
+            onlyIncludeSignatureAnnotations = true
         )
         sb.append(modifierString.toString())
 
diff --git a/src/main/resources/version.properties b/src/main/resources/version.properties
index 8e037af..72e8ce9 100644
--- a/src/main/resources/version.properties
+++ b/src/main/resources/version.properties
@@ -1,4 +1,4 @@
 # Version definition
 # This file is read by gradle build scripts, but also packaged with metalava
 # as a resource for the Version classes to read.
-metalavaVersion=0.9.10
+metalavaVersion=0.9.11
diff --git a/src/test/java/com/android/tools/metalava/DriverTest.kt b/src/test/java/com/android/tools/metalava/DriverTest.kt
index b925335..da7a913 100644
--- a/src/test/java/com/android/tools/metalava/DriverTest.kt
+++ b/src/test/java/com/android/tools/metalava/DriverTest.kt
@@ -213,7 +213,7 @@
         /** Map from artifact id to artifact descriptor */
         artifacts: Map<String, String>? = null,
         /** Extract annotations and check that the given packages contain the given extracted XML files */
-        extractAnnotations: Map<String,String>? = null
+        extractAnnotations: Map<String, String>? = null
     ) {
         System.setProperty("METALAVA_TESTS_RUNNING", VALUE_TRUE)
 
@@ -678,8 +678,8 @@
         if (extractAnnotations != null && extractedAnnotationsZip != null) {
             assertTrue("Using --extract-annotations but $extractedAnnotationsZip was not created",
                 extractedAnnotationsZip.isFile)
-            for ((pkg,xml) in extractAnnotations) {
-                 assertPackageXml(pkg, extractedAnnotationsZip, xml)
+            for ((pkg, xml) in extractAnnotations) {
+                assertPackageXml(pkg, extractedAnnotationsZip, xml)
             }
         }
 
diff --git a/src/test/java/com/android/tools/metalava/ExtractAnnotationsTest.kt b/src/test/java/com/android/tools/metalava/ExtractAnnotationsTest.kt
index 9f0457c..91e1672 100644
--- a/src/test/java/com/android/tools/metalava/ExtractAnnotationsTest.kt
+++ b/src/test/java/com/android/tools/metalava/ExtractAnnotationsTest.kt
@@ -16,6 +16,7 @@
 
 package com.android.tools.metalava
 
+import org.junit.Ignore
 import org.junit.Test
 
 @SuppressWarnings("ALL") // Sample code
@@ -117,7 +118,7 @@
                     const val STYLE_NO_FRAME = 2L
                     const val STYLE_NO_INPUT = 3L
                     const val UNRELATED = 3L
-                    private val HIDDEN = 4
+                    private const val HIDDEN = 4
 
                     const val TYPE_1 = "type1"
                     const val TYPE_2 = "type2"
@@ -175,4 +176,46 @@
             )
         )
     }
+
+    @Ignore("Not working reliably")
+    @Test
+    fun `Include merged annotations in exported source annotations`() {
+        check(
+            compatibilityMode = false,
+            outputKotlinStyleNulls = false,
+            includeSystemApiAnnotations = false,
+            omitCommonPackages = false,
+            warnings = "error: Unexpected reference to Nonexistent.Field [AnnotationExtraction:146]",
+            sourceFiles = *arrayOf(
+                java(
+                    """
+                    package test.pkg;
+
+                    public class MyTest {
+                        public void test(int arg) { }
+                    }"""
+                )
+            ),
+            mergeAnnotations = """<?xml version="1.0" encoding="UTF-8"?>
+                <root>
+                  <item name="test.pkg.MyTest void test(int) 0">
+                    <annotation name="org.intellij.lang.annotations.MagicConstant">
+                      <val name="intValues" val="{java.util.Calendar.ERA, java.util.Calendar.YEAR, java.util.Calendar.MONTH, java.util.Calendar.WEEK_OF_YEAR, Nonexistent.Field}" />
+                    </annotation>
+                  </item>
+                </root>
+                """,
+            extractAnnotations = mapOf("test.pkg" to """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <root>
+                  <item name="test.pkg.MyTest void test(int) 0">
+                    <annotation name="androidx.annotation.IntDef">
+                      <val name="value" val="{java.util.Calendar.ERA, java.util.Calendar.YEAR, java.util.Calendar.MONTH, java.util.Calendar.WEEK_OF_YEAR}" />
+                    </annotation>
+                  </item>
+                </root>
+                """
+            )
+        )
+    }
 }
\ No newline at end of file