handle annotations in classfile parser

fixes sbt#630

internal/zinc-apiinfo/src/main/scala/xsbt/api/APIUtil.scala

@@ -35,6 +35,10 @@ object APIUtil {
     check.hasMacro
   }
 
+  def isAnnotationDefinition(c: ClassLike): Boolean =
+    c.structure.parents.flatMap(Discovery.simpleName)
+      .contains("java.lang.annotation.Annotation")
+
   private[this] class HasMacro extends Visit {
     var hasMacro = false
 

internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/ClassFile.scala

@@ -95,6 +95,8 @@ private[sbt] final case class AttributeInfo(name: Option[String], value: Array[B
   def isNamed(s: String) = name.exists(s == _)
   def isSignature = isNamed("Signature")
   def isSourceFile = isNamed("SourceFile")
+  def isRuntimeVisibleAnnotations = isNamed("RuntimeVisibleAnnotations")
+  def isRuntimeInvisibleAnnotations = isNamed("RuntimeInvisibleAnnotations")
 }
 private[sbt] object Constants {
   final val ACC_STATIC = 0x0008

internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala

@@ -29,6 +29,9 @@ import sbt.util.Logger
 // (BSD license at time of initial port; MIT license as of 2022)
 //
 // Note that unlike the rest of sbt, some things might be null.
+//
+// For debugging this, it's useful to uncomment this in JavaCompilerForUnitTesting:
+// logger.setLevel(sbt.util.Level.Debug)
 
 import Constants._
 
@@ -66,6 +69,7 @@ private[sbt] object Parser {
       val accessFlags: Int = in.readUnsignedShort()
 
       val className = getClassConstantName(in.readUnsignedShort())
+      log.debug(s"[zinc] classfile.Parser parsing $className")
       val superClassName = getClassConstantName(in.readUnsignedShort())
       val interfaceNames =
         array(in.readUnsignedShort())(getClassConstantName(in.readUnsignedShort()))
@@ -110,7 +114,8 @@ private[sbt] object Parser {
         AttributeInfo(name, value)
       }
 
-      def types = (classConstantReferences ++ fieldTypes ++ methodTypes).toSet
+      def types =
+        (classConstantReferences ++ fieldTypes ++ methodTypes ++ annotationsReferences).toSet
 
       private def getTypes(fieldsOrMethods: Array[FieldOrMethodInfo]) =
         fieldsOrMethods.flatMap { fieldOrMethod =>
@@ -120,6 +125,49 @@ private[sbt] object Parser {
       private def fieldTypes = getTypes(fields)
       private def methodTypes = getTypes(methods)
 
+      private def annotationsReferences: List[String] = {
+        val allAttributes =
+          attributes ++
+            fields.flatMap(_.attributes) ++
+            methods.flatMap(_.attributes)
+        allAttributes
+          .filter(x => x.isRuntimeVisibleAnnotations || x.isRuntimeInvisibleAnnotations)
+          .flatMap { attr =>
+            val in = new DataInputStream(new java.io.ByteArrayInputStream(attr.value))
+            val numAnnotations = in.readUnsignedShort()
+            val result = collection.mutable.ListBuffer[String]()
+            def parseElementValue(): Unit = { // JVMS 4.7.16.1
+              val c = in.readUnsignedByte().toChar
+              c match {
+                case 'e' =>
+                  result += slashesToDots(toUTF8(in.readUnsignedShort()))
+                  val _ = in.readUnsignedShort()
+                case 'c' =>
+                  result += slashesToDots(toUTF8(in.readUnsignedShort()))
+                case '@' =>
+                  parseAnnotation()
+                case '[' =>
+                  for (_ <- 0 until in.readUnsignedShort())
+                    parseElementValue()
+                case _ =>
+                // ignore. robustness is paramount
+              }
+            }
+            def parseAnnotation(): Unit = { // JVMS 4.7.16
+              result += slashesToDots(toUTF8(in.readUnsignedShort()))
+              for (_ <- 0 until in.readUnsignedShort()) {
+                in.readUnsignedShort() // skip element name index
+                parseElementValue()
+              }
+            }
+            for (_ <- 0 until numAnnotations)
+              parseAnnotation()
+            // the type names in the constant pool have the form e.g. `Ljava/lang/Object;`;
+            // we've already used `slashesToDots`, but we still need to drop the `L` and the semicolon
+            result.map(name => name.slice(1, name.length - 1))
+          }.toList
+      }
+
       private def classConstantReferences =
         constants.flatMap { constant =>
           constant.tag match {
@@ -200,11 +248,15 @@ private[sbt] object Parser {
   }
 
   private def toInt(v: Byte) = if (v < 0) v + 256 else v.toInt
-  def entryIndex(a: AttributeInfo) = {
-    require(a.value.length == 2, s"Expected two bytes for unsigned value; got: ${a.value.length}")
-    val Array(v0, v1) = a.value
-    toInt(v0) * 256 + toInt(v1)
-  }
+  private def u2(highByte: Byte, lowByte: Byte): Int =
+    toInt(highByte) * 256 + toInt(lowByte)
+  def entryIndex(a: AttributeInfo) =
+    a.value match {
+      case Array(v0, v1) =>
+        u2(v0, v1)
+      case _ =>
+        sys.error(s"Expected two bytes for unsigned value; got: ${a.value.length}")
+    }
 
   private def slashesToDots(s: String) = s.replace('/', '.')
 

internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/AnalyzeSpecification.scala

@@ -46,4 +46,142 @@ class AnalyzeSpecification extends UnitSpec {
     assert(deps.memberRef("D") === Set.empty)
   }
 
+  "Analyze" should "process runtime-visible annotations" in {
+    val srcTest =
+      """|import java.lang.annotation.Retention;
+         |import java.lang.annotation.RetentionPolicy;
+         |@Retention(RetentionPolicy.RUNTIME)
+         |public @interface Test { }
+         |""".stripMargin
+    val srcFoo =
+      """|@Test
+         |public class Foo {
+         |  public static void main(String[] args){
+         |    System.out.println(Foo.class.getAnnotations().length);
+         |  }
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test.java" -> srcTest,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test"))
+  }
+
+  "Analyze" should "process annotation with array argument" in {
+    val srcTest =
+      """|import java.lang.annotation.Retention;
+         |import java.lang.annotation.RetentionPolicy;
+         |import java.lang.annotation.ElementType;
+         |@Retention(RetentionPolicy.RUNTIME)
+         |public @interface Test {
+         |  ElementType[] value();
+         |}
+         |""".stripMargin
+    val srcFoo =
+      """|import java.lang.annotation.ElementType;
+         |@Test(ElementType.TYPE)
+         |public class Foo {
+         |  public static void main(String[] args){
+         |    System.out.println(Foo.class.getAnnotations().length);
+         |  }
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test.java" -> srcTest,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test"))
+  }
+
+  "Analyze" should "detect annotation in array argument to annotation" in {
+    val srcTest1 =
+      """|import java.lang.annotation.Retention;
+         |import java.lang.annotation.RetentionPolicy;
+         |@Retention(RetentionPolicy.RUNTIME)
+         |public @interface Test1 { }
+         |""".stripMargin
+    val srcTest2 =
+      """|import java.lang.annotation.Retention;
+         |import java.lang.annotation.RetentionPolicy;
+         |@Retention(RetentionPolicy.RUNTIME)
+         |public @interface Test2 {
+         |  Test1[] value();
+         |}
+         |""".stripMargin
+    val srcFoo =
+      """|import java.lang.annotation.ElementType;
+         |@Test2(@Test1)
+         |public class Foo {
+         |  public static void main(String[] args){
+         |    System.out.println(Foo.class.getAnnotations().length);
+         |  }
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test1.java" -> srcTest1,
+      "Test2.java" -> srcTest2,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test1"))
+    assert(deps.memberRef("Foo").contains("Test2"))
+  }
+
+  "Analyze" should "process runtime-invisible annotations" in {
+    val srcTest =
+      """|public @interface Test { }
+         |""".stripMargin
+    val srcFoo =
+      """|@Test
+         |public class Foo {
+         |  public static void main(String[] args){
+         |    System.out.println(Foo.class.getAnnotations().length);
+         |  }
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test.java" -> srcTest,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test"))
+  }
+
+  "Analyze" should "detect annotation on field" in {
+    val srcTest =
+      """|import java.lang.annotation.Target;
+         |import java.lang.annotation.ElementType;
+         |@Target(ElementType.FIELD)
+         |public @interface Test { }
+         |""".stripMargin
+    val srcFoo =
+      """|public class Foo {
+         |  @Test int foo;
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test.java" -> srcTest,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test"))
+  }
+
+  "Analyze" should "detect annotation on method" in {
+    val srcTest =
+      """|import java.lang.annotation.Target;
+         |import java.lang.annotation.ElementType;
+         |@Target(ElementType.METHOD)
+         |public @interface Test { }
+         |""".stripMargin
+    val srcFoo =
+      """|public class Foo {
+         |  @Test int foo() { return 0; }
+         |}
+         |""".stripMargin
+    val deps = JavaCompilerForUnitTesting.extractDependenciesFromSrcs(
+      "Test.java" -> srcTest,
+      "Foo.java" -> srcFoo,
+    )
+    assert(deps.memberRef("Foo").contains("Test"))
+  }
+
 }

internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/ParserSpecification.scala

@@ -34,6 +34,7 @@ object ParserSpecification extends Properties("Parser") {
       classOf[java.util.AbstractMap.SimpleEntry[String, String]],
       classOf[String],
       classOf[Thread],
-      classOf[Properties]
+      classOf[Properties],
+      classOf[java.lang.annotation.Retention]
     )
 }

internal/zinc-core/src/main/scala/sbt/internal/inc/Changes.scala

@@ -49,9 +49,10 @@ final class APIChanges(val apiChanges: Iterable[APIChange]) {
 sealed abstract class APIChange(val modifiedClass: String) extends XAPIChange {
   override def getModifiedClass: String = modifiedClass
   override def getModifiedNames: java.util.Set[XUsedName] = this match {
-    case _: APIChangeDueToMacroDefinition => java.util.Collections.emptySet[XUsedName]
-    case _: TraitPrivateMembersModified   => java.util.Collections.emptySet[XUsedName]
-    case NamesChange(_, modifiedNames)    => modifiedNames.names.map(x => x: XUsedName).asJava
+    case _: APIChangeDueToMacroDefinition      => java.util.Collections.emptySet[XUsedName]
+    case _: APIChangeDueToAnnotationDefinition => java.util.Collections.emptySet[XUsedName]
+    case _: TraitPrivateMembersModified        => java.util.Collections.emptySet[XUsedName]
+    case NamesChange(_, modifiedNames)         => modifiedNames.names.map(x => x: XUsedName).asJava
   }
 }
 
@@ -62,6 +63,13 @@ sealed abstract class APIChange(val modifiedClass: String) extends XAPIChange {
  */
 final case class APIChangeDueToMacroDefinition(modified0: String) extends APIChange(modified0)
 
+/**
+ * If we recompile an annotation definition in a Java source file then we always assume
+ * that its api has changed. We err on the side of caution because e.g. the annotation's
+ * Retention might have changed (sbt/zinc#630).
+ */
+final case class APIChangeDueToAnnotationDefinition(modified0: String) extends APIChange(modified0)
+
 /**
  * An APIChange that carries information about modified names.
  *

internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala

@@ -652,6 +652,8 @@ private final class AnalysisCallback(
   private[this] val binaryClassName = new TrieMap[VirtualFile, String]
   // source files containing a macro def.
   private[this] val macroClasses = ConcurrentHashMap.newKeySet[String]()
+  // source files containing a Java annotation definition
+  private[this] val annotationClasses = ConcurrentHashMap.newKeySet[String]()
 
   // Results of invalidation calculations (including whether to continue cycles) - the analysis at this point is
   // not useful and so isn't included.
@@ -839,6 +841,9 @@ private final class AnalysisCallback(
     val className = classApi.name
     if (APIUtil.isScalaSourceName(sourceFile.id) && APIUtil.hasMacro(classApi))
       macroClasses.add(className)
+    // sbt/zinc#630
+    if (!APIUtil.isScalaSourceName(sourceFile.id) && APIUtil.isAnnotationDefinition(classApi))
+      annotationClasses.add(className)
     val shouldMinimize = !Incremental.apiDebug(options)
     val savedClassApi = if (shouldMinimize) APIUtil.minimize(classApi) else classApi
     val apiHash: HashAPI.Hash = HashAPI(classApi)

internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala

@@ -312,7 +312,14 @@ private[inc] abstract class IncrementalCommon(
         val hasMacro = a.hasMacro || b.hasMacro
         if (hasMacro && IncOptions.getRecompileOnMacroDef(options)) {
           Some(APIChangeDueToMacroDefinition(className))
-        } else findAPIChange(className, a, b)
+        } else if (
+          APIUtil.isAnnotationDefinition(a.api().classApi()) ||
+          APIUtil.isAnnotationDefinition(b.api().classApi())
+        ) {
+          Some(APIChangeDueToAnnotationDefinition(className))
+        } else {
+          findAPIChange(className, a, b)
+        }
       }
     }
     val apiChanges = recompiledClasses.flatMap(name => classDiff(name, oldAPI(name), newAPI(name)))
@@ -589,6 +596,8 @@ private[inc] abstract class IncrementalCommon(
       apiChanges foreach {
         case APIChangeDueToMacroDefinition(src) =>
           wrappedLog.debug(s"Detected API change because $src contains a macro definition.")
+        case APIChangeDueToAnnotationDefinition(src) =>
+          wrappedLog.debug(s"Detected API change because $src contains an annotation definition.")
         case TraitPrivateMembersModified(modifiedClass) =>
           wrappedLog.debug(s"Detect change in private members of trait ${modifiedClass}.")
         case apiChange: NamesChange =>

internal/zinc-core/src/main/scala/sbt/internal/inc/InvalidationProfiler.scala

@@ -123,6 +123,11 @@ class ZincInvalidationProfiler extends InvalidationProfiler with XInvalidationPr
             .setModifiedClass(memo(change.modifiedClass))
             .setReason("API change due to macro definition.")
             .build
+        case change: APIChangeDueToAnnotationDefinition =>
+          Zprof.ApiChange.newBuilder
+            .setModifiedClass(memo(change.modifiedClass))
+            .setReason("API change due to annotation definition.")
+            .build
         case change: TraitPrivateMembersModified =>
           Zprof.ApiChange.newBuilder
             .setModifiedClass(memo(change.modifiedClass))

internal/zinc-core/src/main/scala/sbt/internal/inc/MemberRefInvalidator.scala

@@ -65,6 +65,8 @@ private[inc] class MemberRefInvalidator(log: Logger, logRecompileOnMacro: Boolea
     case _: TraitPrivateMembersModified => NoInvalidation
     case _: APIChangeDueToMacroDefinition =>
       new InvalidateUnconditionally(memberRef)
+    case _: APIChangeDueToAnnotationDefinition =>
+      new InvalidateUnconditionally(memberRef)
     case NamesChange(_, modifiedNames) if modifiedNames.in(UseScope.Implicit).nonEmpty =>
       new InvalidateUnconditionally(memberRef)
     case NamesChange(_, modifiedNames) =>
@@ -76,6 +78,8 @@ private[inc] class MemberRefInvalidator(log: Logger, logRecompileOnMacro: Boolea
       s"The private signature of trait ${modifiedClass} changed."
     case APIChangeDueToMacroDefinition(modifiedSrcFile) =>
       s"The $modifiedSrcFile source file declares a macro."
+    case APIChangeDueToAnnotationDefinition(modifiedSrcFile) =>
+      s"The $modifiedSrcFile source file declares an annotation."
     case NamesChange(modifiedClass, modifiedNames) =>
       modifiedNames.in(UseScope.Implicit) match {
         case changedImplicits if changedImplicits.isEmpty =>

zinc/src/sbt-test/source-dependencies/annotations-in-java-sources-a/Foo.java

@@ -0,0 +1,3 @@
+package test;
+@Test
+public class Foo { }

zinc/src/sbt-test/source-dependencies/annotations-in-java-sources-a/Test.java

@@ -0,0 +1,3 @@
+package test;
+public @interface Test {
+}

zinc/src/sbt-test/source-dependencies/annotations-in-java-sources-a/changes/Test1.java

@@ -0,0 +1,4 @@
+package test;
+public @interface Test {
+  int x() default 0;
+}

zinc/src/sbt-test/source-dependencies/annotations-in-java-sources-a/incOptions.properties

@@ -0,0 +1,2 @@
+# turn off pipelining because compileAllJava over-compiles
+pipelining = false