Skip to content

Commit

Permalink
Represent Java annotations as interfaces so they can be extended
Browse files Browse the repository at this point in the history
Previously we treated Java annotations as if they were classes, like Scala
annotations. For example, given

    @interface Ann { int foo(); }

we pretended it was defined as:

    abstract class Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

We take advantage of this to type annotation trees as if they were
new calls, for example `@Ann(1)` is typed as `new Ann(1)`.

Pretending that annotations are classes is fine most of the time and matches
what Scala 2.12 did, but it's problematic because the JVM treats annotations as
interfaces. In practice this was only an issue with code trying to extend
Java annotations, which would either be rejected at compile-time or miscompiled
before this commit.

This commit switches our representation of annotations to be trait-based
instead:

    trait Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

Classes are then free to extend annotations using the same pattern as in Scala 2.13:

    class Foo extends Ann {val annotationType = classOf[Retention]; def foo(): Int = 1}

Notice that we still pretend these traits have constructors, this lets us type
annotation trees in much the same way as before, and crucially it means that
macros that depended on the exact tree shape of annotation trees can continue to
work, as demonstrated by the annot-java-tree test extracted from wartremover.
To prevent miscompilation issues, we disallow passing arguments to the
annotation constructor in `extends` clause.

The treatment of default arguments to annotations stays unchanged from 85cd1cf.

Fixes scala#5690. Fixes scala#12840. Fixes scala#14199.
  • Loading branch information
smarter committed Oct 28, 2022
1 parent 9ff325c commit 09e39f9
Show file tree
Hide file tree
Showing 23 changed files with 163 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -353,18 +353,15 @@ object ClassfileConstants {
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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,7 @@ class ClassfileParser(
* Updates the read pointer of 'in'. */
def parseParents: List[Type] = {
val superType =
if (isAnnotation) {
in.nextChar
defn.ObjectType
}
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
Expand Down Expand Up @@ -844,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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -876,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)
}
addCompanionObject(statics, annot)
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,8 @@ trait Checking {
def checkParentCall(call: Tree, caller: ClassSymbol)(using Context): Unit =
if (!ctx.isAfterTyper) {
val called = call.tpe.classSymbol
if (called.derivesFrom(defn.JavaAnnotationClass))
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))
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.derivesFrom(defn.JavaAnnotationClass)
&& (!psym.is(Trait)
|| psym.primaryConstructor.info.takesParams && !cls.superClass.isSubClass(psym))
then typed(untpd.New(untpd.TypedSplice(parent), Nil))
Expand Down
File renamed without changes.
3 changes: 3 additions & 0 deletions tests/neg/java-ann-super-class/Ann.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Ann {
int value();
}
9 changes: 9 additions & 0 deletions tests/neg/java-ann-super-class/Test.scala
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/neg/java-ann-super-class2/Ann.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Ann {
int value();
}
3 changes: 3 additions & 0 deletions tests/neg/java-ann-super-class2/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def test =
// Posttyper errors
new Ann(1) // error
3 changes: 3 additions & 0 deletions tests/neg/java-ann-super-class3/Ann.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Ann {
int value();
}
3 changes: 3 additions & 0 deletions tests/neg/java-ann-super-class3/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def test =
// Refchecks error
new Ann {} // error
6 changes: 3 additions & 3 deletions tests/neg/repeatable/Test_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 0 additions & 8 deletions tests/pos/i5690.scala

This file was deleted.

6 changes: 3 additions & 3 deletions tests/pos/i7467.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ class DuplicateSymbolError_DirectSuperclass extends DefaultListCellRenderer() {
override def getListCellRendererComponent(list: JList[_ <: Object], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ???
}

class DuplicateSymbolError_IndirectInterface extends DefaultListCellRenderer() {
override def getListCellRendererComponent(list: JList[_], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ???
}
// class DuplicateSymbolError_IndirectInterface extends DefaultListCellRenderer() {
// override def getListCellRendererComponent(list: JList[_], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ???
// }
29 changes: 29 additions & 0 deletions tests/run-macros/annot-java-tree/AnnoMacro.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import scala.quoted.*

inline def checkSuppressWarnings[T]: Unit = ${ checkSuppressWarningsImpl[T] }

def checkSuppressWarningsImpl[T: Type](using Quotes): Expr[Unit] =
import quotes.reflect.*
val SuppressWarningsSymbol = TypeTree.of[SuppressWarnings].symbol
val sym = TypeRepr.of[T].typeSymbol
// Imitate what wartremover does, so we can avoid unintentionally breaking it:
// https://github.com/wartremover/wartremover/blob/fb18e6eafe9a47823e04960aaf4ec7a9293719ef/core/src/main/scala-3/org/wartremover/WartUniverse.scala#L63-L77
val actualArgs = sym
.getAnnotation(SuppressWarningsSymbol)
.collect {
case Apply(
Select(_, "<init>"),
Apply(Apply(_, Typed(Repeated(values, _), _) :: Nil), Apply(_, _ :: Nil) :: Nil) :: Nil
) =>
// "-Yexplicit-nulls"
// https://github.com/wartremover/wartremover/issues/660
values.collect { case Literal(StringConstant(str)) =>
str
}
}
.toList
.flatten
val expectedArgs = List("a", "b")
assert(actualArgs == expectedArgs,
s"Expected $expectedArgs arguments for SuppressWarnings annotation of $sym but got $actualArgs")
'{}
4 changes: 4 additions & 0 deletions tests/run-macros/annot-java-tree/S.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@SuppressWarnings(Array("a", "b")) class Foo

@main def Test =
checkSuppressWarnings[Foo]
4 changes: 4 additions & 0 deletions tests/run/java-ann-super-class-separate/Ann_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public @interface Ann_1 {
int bar() default 1;
int baz() default 2;
}
21 changes: 21 additions & 0 deletions tests/run/java-ann-super-class-separate/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// scalajs: --skip

class Foo extends Ann_1 {
override def bar = 3
override def baz = 4
def annotationType = classOf[Ann_1]
}

object Test {
def main(args: Array[String]): Unit = {
val x = new Foo
val y: Ann_1 = x
val z: Int @Ann_1(1) = 1
val zz: Int @Ann_1() = 1
// val x: scala.annotation.Annotation = new Ann {
// // val x: java.lang.annotation.Annotation = new Ann {
// def annotationType = classOf[Ann]
// }
// println(x)
}
}
4 changes: 4 additions & 0 deletions tests/run/java-ann-super-class/Ann.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public @interface Ann {
int bar() default 1;
int baz() default 2;
}
21 changes: 21 additions & 0 deletions tests/run/java-ann-super-class/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// scalajs: --skip

class Foo extends Ann {
override def bar = 3
override def baz = 4
def annotationType = classOf[Ann]
}

object Test {
def main(args: Array[String]): Unit = {
val x = new Foo
val y: Ann = x
val z: Int @Ann(1) = 1
val zz: Int @Ann() = 1
// val x: scala.annotation.Annotation = new Ann {
// // val x: java.lang.annotation.Annotation = new Ann {
// def annotationType = classOf[Ann]
// }
// println(x)
}
}
10 changes: 6 additions & 4 deletions tests/run/repeatable/Test_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@ import repeatable._
@Plain_0(3)
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)))
trait T

object Test:
def main(args: Array[String]) =
object Test {
def main(args: Array[String]) = {
val annValuesU = classOf[U].getAnnotation(classOf[FirstLevel_0]).value.toList.map(_.value).sorted
annValuesU.foreach(println)

println()

val annValuesT = classOf[T].getAnnotation(classOf[SecondLevel_0]).value.toList.map(_.value.toList.map(_.value).sorted).sorted
annValuesT.foreach(println)
}
}
28 changes: 28 additions & 0 deletions tests/run/t9400.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// scalajs: --skip

class Deprecation extends Deprecated {
final val annotationType = classOf[Deprecated]

def forRemoval(): Boolean = false
def since(): String = ""
}

class Suppression extends SuppressWarnings {
final val annotationType = classOf[SuppressWarnings]

def value = Array("unchecked")
}

class Retention(runtime: Boolean) extends java.lang.annotation.Retention {
final val annotationType = classOf[Retention]

def value =
if (runtime) java.lang.annotation.RetentionPolicy.RUNTIME
else java.lang.annotation.RetentionPolicy.SOURCE
}

object Test extends App {
new Deprecation
new Suppression
new Retention(true)
}

0 comments on commit 09e39f9

Please sign in to comment.