Represent Java annotations as interfaces so they can be extended, and disallow various misuses of them (#16260)

Inspired by the work of hrhino in Scala 2
(scala/scala#6869).

Author: smarter

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

@@ -61,7 +61,6 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
   @threadUnsafe lazy val AnnotationRetentionSourceAttr: TermSymbol = requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("SOURCE")
   @threadUnsafe lazy val AnnotationRetentionClassAttr: TermSymbol = requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("CLASS")
   @threadUnsafe lazy val AnnotationRetentionRuntimeAttr: TermSymbol = requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("RUNTIME")
-  @threadUnsafe lazy val JavaAnnotationClass: ClassSymbol = requiredClass("java.lang.annotation.Annotation")
 
   val bCodeAsmCommon: BCodeAsmCommon[int.type] = new BCodeAsmCommon(int)
 
@@ -415,7 +414,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
             arrAnnotV.visitEnd()
           }          // for the lazy val in ScalaSigBytes to be GC'ed, the invoker of emitAnnotations() should hold the ScalaSigBytes in a method-local var that doesn't escape.
   */
-        case t @ Apply(constr, args) if t.tpe.derivesFrom(JavaAnnotationClass) =>
+        case t @ Apply(constr, args) if t.tpe.classSymbol.is(JavaAnnotation) =>
           val typ = t.tpe.classSymbol.denot.info
           val assocs = assocsFromApply(t)
           val desc = innerClasesStore.typeDescriptor(typ) // the class descriptor of the nested annotation class

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -969,7 +969,6 @@ class Definitions {
 
   // Annotation base classes
   @tu lazy val AnnotationClass: ClassSymbol = requiredClass("scala.annotation.Annotation")
-  @tu lazy val ClassfileAnnotationClass: ClassSymbol = requiredClass("scala.annotation.ClassfileAnnotation")
   @tu lazy val StaticAnnotationClass: ClassSymbol = requiredClass("scala.annotation.StaticAnnotation")
   @tu lazy val RefiningAnnotationClass: ClassSymbol = requiredClass("scala.annotation.RefiningAnnotation")
 

compiler/src/dotty/tools/dotc/core/Flags.scala

@@ -350,8 +350,8 @@ object Flags {
   /** Symbol is a method which should be marked ACC_SYNCHRONIZED */
   val (_, Synchronized @ _, _) = newFlags(36, "<synchronized>")
 
-  /** Symbol is a Java-style varargs method */
-  val (_, JavaVarargs @ _, _) = newFlags(37, "<varargs>")
+  /** Symbol is a Java-style varargs method / a Java annotation */
+  val (_, JavaVarargs @ _, JavaAnnotation @ _) = newFlags(37, "<varargs>", "<java-annotation>")
 
   /** Symbol is a Java default method */
   val (_, DefaultMethod @ _, _) = newFlags(38, "<defaultmethod>")
@@ -477,7 +477,7 @@ object Flags {
    */
   val AfterLoadFlags: FlagSet = commonFlags(
     FromStartFlags, AccessFlags, Final, AccessorOrSealed,
-    Abstract, LazyOrTrait, SelfName, JavaDefined, Transparent)
+    Abstract, LazyOrTrait, SelfName, JavaDefined, JavaAnnotation, Transparent)
 
   /** A value that's unstable unless complemented with a Stable flag */
   val UnstableValueFlags: FlagSet = Mutable | Method

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -243,7 +243,6 @@ object StdNames {
     final val ToString: N             = "ToString"
     final val Xor: N                  = "^"
 
-    final val ClassfileAnnotation: N = "ClassfileAnnotation"
     final val ClassManifest: N       = "ClassManifest"
     final val Enum: N                = "Enum"
     final val Group: N               = "Group"

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -808,7 +808,7 @@ object SymDenotations {
 
     /** Is this a Scala or Java annotation ? */
     def isAnnotation(using Context): Boolean =
-      isClass && derivesFrom(defn.AnnotationClass)
+      isClass && (derivesFrom(defn.AnnotationClass) || is(JavaAnnotation))
 
     /** Is this symbol a class that extends `java.io.Serializable` ? */
     def isSerializable(using Context): Boolean =

compiler/src/dotty/tools/dotc/core/classfile/ClassfileConstants.scala

@@ -346,25 +346,24 @@ object ClassfileConstants {
       case JAVA_ACC_ENUM       => Enum
       case JAVA_ACC_ABSTRACT   => if (isClass) Abstract else Deferred
       case JAVA_ACC_INTERFACE  => PureInterfaceCreationFlags | JavaDefined
+      case JAVA_ACC_ANNOTATION => JavaAnnotation
       case _                   => EmptyFlags
     }
 
     private def addFlag(base: FlagSet, jflag: Int): FlagSet =
       if (jflag == 0) base else base | translateFlag(jflag)
 
     private def translateFlags(jflags: Int, baseFlags: FlagSet): FlagSet = {
-      val nflags =
-        if ((jflags & JAVA_ACC_ANNOTATION) == 0) jflags
-        else jflags & ~(JAVA_ACC_ABSTRACT | JAVA_ACC_INTERFACE) // annotations are neither abstract nor interfaces
       var res: FlagSet = baseFlags | JavaDefined
-      res = addFlag(res, nflags & JAVA_ACC_PRIVATE)
-      res = addFlag(res, nflags & JAVA_ACC_PROTECTED)
-      res = addFlag(res, nflags & JAVA_ACC_FINAL)
-      res = addFlag(res, nflags & JAVA_ACC_SYNTHETIC)
-      res = addFlag(res, nflags & JAVA_ACC_STATIC)
-      res = addFlag(res, nflags & JAVA_ACC_ENUM)
-      res = addFlag(res, nflags & JAVA_ACC_ABSTRACT)
-      res = addFlag(res, nflags & JAVA_ACC_INTERFACE)
+      res = addFlag(res, jflags & JAVA_ACC_PRIVATE)
+      res = addFlag(res, jflags & JAVA_ACC_PROTECTED)
+      res = addFlag(res, jflags & JAVA_ACC_FINAL)
+      res = addFlag(res, jflags & JAVA_ACC_SYNTHETIC)
+      res = addFlag(res, jflags & JAVA_ACC_STATIC)
+      res = addFlag(res, jflags & JAVA_ACC_ENUM)
+      res = addFlag(res, jflags & JAVA_ACC_ABSTRACT)
+      res = addFlag(res, jflags & JAVA_ACC_INTERFACE)
+      res = addFlag(res, jflags & JAVA_ACC_ANNOTATION)
       res
     }
 

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -165,11 +165,7 @@ class ClassfileParser(
      *  Updates the read pointer of 'in'. */
     def parseParents: List[Type] = {
       val superType =
-        if (isAnnotation) {
-          in.nextChar
-          defn.AnnotationClass.typeRef
-        }
-        else if (classRoot.symbol == defn.ComparableClass ||
+        if (classRoot.symbol == defn.ComparableClass ||
                  classRoot.symbol == defn.JavaCloneableClass ||
                  classRoot.symbol == defn.JavaSerializableClass) {
           // Treat these interfaces as universal traits
@@ -186,7 +182,6 @@ class ClassfileParser(
         // Consequently, no best implicit for the "Integral" evidence parameter of "range"
         // is found. Previously, this worked because of weak conformance, which has been dropped.
 
-      if (isAnnotation) ifaces = defn.ClassfileAnnotationClass.typeRef :: ifaces
       superType :: ifaces
     }
 
@@ -845,7 +840,7 @@ class ClassfileParser(
 
   class AnnotConstructorCompleter(classInfo: TempClassInfoType) extends LazyType {
     def complete(denot: SymDenotation)(using Context): Unit = {
-      val attrs = classInfo.decls.toList.filter(sym => sym.isTerm && sym != denot.symbol)
+      val attrs = classInfo.decls.toList.filter(sym => sym.isTerm && sym != denot.symbol && sym.name != nme.CONSTRUCTOR)
       val paramNames = attrs.map(_.name.asTermName)
       val paramTypes = attrs.map(_.info.resultType)
       denot.info = MethodType(paramNames, paramTypes, classRoot.typeRef)

compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala

@@ -822,7 +822,7 @@ object JavaParsers {
       val iface = atSpan(start, nameOffset) {
         TypeDef(
           name,
-          makeTemplate(parents, body, tparams, false)).withMods(mods | Flags.Trait | Flags.JavaInterface | Flags.Abstract)
+          makeTemplate(parents, body, tparams, false)).withMods(mods | Flags.JavaInterface)
       }
       addCompanionObject(statics, iface)
     }
@@ -858,10 +858,9 @@ object JavaParsers {
       }
       (statics.toList, members.toList)
     }
-    def annotationParents: List[Select] = List(
-      scalaAnnotationDot(tpnme.Annotation),
-      Select(javaLangDot(nme.annotation), tpnme.Annotation),
-      scalaAnnotationDot(tpnme.ClassfileAnnotation)
+    def annotationParents: List[Tree] = List(
+      javaLangObject(),
+      Select(javaLangDot(nme.annotation), tpnme.Annotation)
     )
     def annotationDecl(start: Offset, mods: Modifiers): List[Tree] = {
       accept(AT)
@@ -877,7 +876,7 @@ object JavaParsers {
         List(constructorParams), TypeTree(), EmptyTree).withMods(Modifiers(Flags.JavaDefined))
       val templ = makeTemplate(annotationParents, constr :: body, List(), true)
       val annot = atSpan(start, nameOffset) {
-        TypeDef(name, templ).withMods(mods | Flags.Abstract)
+        TypeDef(name, templ).withMods(mods | Flags.JavaInterface | Flags.JavaAnnotation)
       }
       addCompanionObject(statics, annot)
     }

compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala

@@ -10,6 +10,7 @@ import Symbols.defn
 import Constants._
 import Types._
 import Decorators._
+import Flags._
 
 import scala.collection.mutable
 
@@ -33,7 +34,7 @@ class RepeatableAnnotations extends MiniPhase:
     val annsByType = stableGroupBy(annotations, _.symbol)
     annsByType.flatMap {
       case (_, a :: Nil) => a :: Nil
-      case (sym, anns) if sym.derivesFrom(defn.ClassfileAnnotationClass) =>
+      case (sym, anns) if sym.is(JavaDefined) =>
         sym.getAnnotation(defn.JavaRepeatableAnnot).flatMap(_.argumentConstant(0)) match
           case Some(Constant(containerTpe: Type)) =>
             val clashingAnns = annsByType.getOrElse(containerTpe.classSymbol, Nil)

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -547,7 +547,7 @@ trait Applications extends Compatibility {
 
     /** Is `sym` a constructor of a Java-defined annotation? */
     def isJavaAnnotConstr(sym: Symbol): Boolean =
-      sym.is(JavaDefined) && sym.isConstructor && sym.owner.derivesFrom(defn.AnnotationClass)
+      sym.is(JavaDefined) && sym.isConstructor && sym.owner.is(JavaAnnotation)
 
     /** Match re-ordered arguments against formal parameters
      *  @param n   The position of the first parameter in formals in `methType`.

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -1110,6 +1110,8 @@ trait Checking {
   def checkParentCall(call: Tree, caller: ClassSymbol)(using Context): Unit =
     if (!ctx.isAfterTyper) {
       val called = call.tpe.classSymbol
+      if (called.is(JavaAnnotation))
+        report.error(i"${called.name} must appear without any argument to be a valid class parent because it is a Java annotation", call.srcPos)
       if (caller.is(Trait))
         report.error(i"$caller may not call constructor of $called", call.srcPos)
       else if (called.is(Trait) && !caller.mixins.contains(called))
@@ -1263,6 +1265,23 @@ trait Checking {
     if !Inlines.inInlineMethod && !ctx.isInlineContext then
       report.error(em"$what can only be used in an inline method", pos)
 
+  /** Check that the class corresponding to this tree is either a Scala or Java annotation.
+   *
+   *  @return The original tree or an error tree in case `tree` isn't a valid
+   *          annotation or already an error tree.
+   */
+  def checkAnnotClass(tree: Tree)(using Context): Tree =
+    if tree.tpe.isError then
+      return tree
+    val cls = Annotations.annotClass(tree)
+    if cls.is(JavaDefined) then
+      if !cls.is(JavaAnnotation) then
+        errorTree(tree, em"$cls is not a valid Java annotation: it was not declared with `@interface`")
+      else tree
+    else if !cls.derivesFrom(defn.AnnotationClass) then
+      errorTree(tree, em"$cls is not a valid Scala annotation: it does not extend `scala.annotation.Annotation`")
+    else tree
+
   /** Check arguments of compiler-defined annotations */
   def checkAnnotArgs(tree: Tree)(using Context): tree.type =
     val cls = Annotations.annotClass(tree)

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -833,7 +833,7 @@ class Namer { typer: Typer =>
           if (cls eq sym)
             report.error("An annotation class cannot be annotated with iself", annotTree.srcPos)
           else {
-            val ann = Annotation.deferred(cls)(typedAheadAnnotation(annotTree)(using annotCtx))
+            val ann = Annotation.deferred(cls)(typedAheadExpr(annotTree)(using annotCtx))
             sym.addAnnotation(ann)
           }
         }
@@ -1618,9 +1618,6 @@ class Namer { typer: Typer =>
   def typedAheadExpr(tree: Tree, pt: Type = WildcardType)(using Context): tpd.Tree =
     typedAhead(tree, typer.typedExpr(_, pt))
 
-  def typedAheadAnnotation(tree: Tree)(using Context): tpd.Tree =
-    typedAheadExpr(tree, defn.AnnotationClass.typeRef)
-
   def typedAheadAnnotationClass(tree: Tree)(using Context): Symbol = tree match {
     case Apply(fn, _) => typedAheadAnnotationClass(fn)
     case TypeApply(fn, _) => typedAheadAnnotationClass(fn)

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -2259,7 +2259,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   }
 
   def typedAnnotation(annot: untpd.Tree)(using Context): Tree =
-    checkAnnotArgs(typed(annot, defn.AnnotationClass.typeRef))
+    checkAnnotClass(checkAnnotArgs(typed(annot)))
 
   def registerNowarn(tree: Tree, mdef: untpd.Tree)(using Context): Unit =
     val annot = Annotations.Annotation(tree)
@@ -2595,6 +2595,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
    */
   def ensureConstrCall(cls: ClassSymbol, parent: Tree, psym: Symbol)(using Context): Tree =
     if parent.isType && !cls.is(Trait) && !cls.is(JavaDefined) && psym.isClass
+        // Annotations are represented as traits with constructors, but should
+        // never be called as such outside of annotation trees.
+        && !psym.is(JavaAnnotation)
         && (!psym.is(Trait)
             || psym.primaryConstructor.info.takesParams && !cls.superClass.isSubClass(psym))
     then typed(untpd.New(untpd.TypedSplice(parent), Nil))
@@ -2669,7 +2672,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   end typedPackageDef
 
   def typedAnnotated(tree: untpd.Annotated, pt: Type)(using Context): Tree = {
-    val annot1 = typedExpr(tree.annot, defn.AnnotationClass.typeRef)
+    val annot1 = checkAnnotClass(typedExpr(tree.annot))
     val annotCls = Annotations.annotClass(annot1)
     if annotCls == defn.NowarnAnnot then
       registerNowarn(annot1, tree)

compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala

@@ -2765,6 +2765,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
       def Invisible: Flags = dotc.core.Flags.Invisible
       def JavaDefined: Flags = dotc.core.Flags.JavaDefined
       def JavaStatic: Flags = dotc.core.Flags.JavaStatic
+      def JavaAnnotation: Flags = dotc.core.Flags.JavaAnnotation
       def Lazy: Flags = dotc.core.Flags.Lazy
       def Local: Flags = dotc.core.Flags.Local
       def Macro: Flags = dotc.core.Flags.Macro

library/src/scala/quoted/Quotes.scala

@@ -4278,6 +4278,9 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching =>
       /** Is implemented as a Java static */
       def JavaStatic: Flags
 
+      /** Is this an annotation defined in Java */
+      @experimental def JavaAnnotation: Flags
+
       /** Is this symbol `lazy` */
       def Lazy: Flags
 

tests/pending/neg/i5690.scala renamed to tests/neg/i5690.scala

@@ -0,0 +1,3 @@
+public @interface Ann_1 {
+  int value();
+}

tests/neg/java-ann-extends-separate/Ann_1.java

@@ -0,0 +1,2 @@
+def test(x: Ann_1) =
+  val y: scala.annotation.Annotation = x // error

tests/neg/java-ann-extends-separate/Test_2.scala

@@ -0,0 +1,3 @@
+public @interface Ann {
+  int value();
+}

tests/neg/java-ann-extends/Ann.java

@@ -0,0 +1,2 @@
+def test(x: Ann) =
+  val y: scala.annotation.Annotation = x // error

tests/neg/java-ann-extends/Test.scala

@@ -0,0 +1,3 @@
+public @interface Ann {
+  int value();
+}

tests/neg/java-ann-super-class/Ann.java

@@ -0,0 +1,9 @@
+class Bar extends Ann(1) { // error
+  def value = 1
+  def annotationType = classOf[Ann]
+}
+
+def test =
+  // Typer errors
+  new Ann // error
+  new Ann(1) {} // error

tests/neg/java-ann-super-class/Test.scala

@@ -0,0 +1,3 @@
+public @interface Ann {
+  int value();
+}

tests/neg/java-ann-super-class2/Ann.java

@@ -0,0 +1,3 @@
+def test =
+  // Posttyper errors
+  new Ann(1) // error

tests/neg/java-ann-super-class2/Test.scala

@@ -0,0 +1,3 @@
+public @interface Ann {
+  int value();
+}

tests/neg/java-ann-super-class3/Ann.java

@@ -0,0 +1,3 @@
+def test =
+  // Refchecks error
+  new Ann {} // error

tests/neg/java-ann-super-class3/Test.scala

@@ -0,0 +1 @@
+interface FakeAnn_1 extends java.lang.annotation.Annotation { }

tests/neg/java-fake-ann-separate/FakeAnn_1.java

@@ -0,0 +1,3 @@
+@FakeAnn_1 def test = // error
+  (1: @FakeAnn_1) // error
+

tests/neg/java-fake-ann-separate/Test_2.scala

@@ -0,0 +1 @@
+interface FakeAnn extends java.lang.annotation.Annotation { }

tests/neg/java-fake-ann/FakeAnn.java

@@ -0,0 +1,2 @@
+@FakeAnn def test = // error
+  (1: @FakeAnn) // error

tests/neg/java-fake-ann/Test.scala

@@ -6,11 +6,11 @@ import repeatable._
 @FirstLevel_0(Array()) // error
 trait U
 
-@FirstLevel_0(Array(Plain_0(4), Plain_0(5)))
-@FirstLevel_0(Array(Plain_0(6), Plain_0(7)))
+@FirstLevel_0(Array(new Plain_0(4), new Plain_0(5)))
+@FirstLevel_0(Array(new Plain_0(6), new Plain_0(7)))
 @SecondLevel_0(Array()) // error
 trait T
 
 @SecondLevel_0(Array())
 @SecondLevel_0(Array()) // error
-trait S
+trait S

tests/neg/repeatable/Test_1.scala

@@ -62,6 +62,7 @@ val experimentalDefinitionInLibrary = Set(
    //// New APIs: Quotes
   // Can be stabilized in 3.3.0 (unsure) or later
   "scala.quoted.Quotes.reflectModule.CompilationInfoModule.XmacroSettings",
+  "scala.quoted.Quotes.reflectModule.FlagsModule.JavaAnnotation",
   // Cant be stabilized yet.
   // Need newClass variant that can add constructor parameters.
   // Need experimental annotation macros to check that design works.