Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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>")
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

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.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
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Symbols.defn
import Constants._
import Types._
import Decorators._
import Flags._

import scala.collection.mutable

Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
19 changes: 19 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down 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.is(JavaAnnotation)
&& (!psym.is(Trait)
|| psym.primaryConstructor.info.takesParams && !cls.superClass.isSubClass(psym))
then typed(untpd.New(untpd.TypedSplice(parent), Nil))
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions library/src/scala/quoted/Quotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
File renamed without changes.
3 changes: 3 additions & 0 deletions tests/neg/java-ann-extends-separate/Ann_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Ann_1 {
int value();
}
2 changes: 2 additions & 0 deletions tests/neg/java-ann-extends-separate/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def test(x: Ann_1) =
val y: scala.annotation.Annotation = x // error
3 changes: 3 additions & 0 deletions tests/neg/java-ann-extends/Ann.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Ann {
int value();
}
2 changes: 2 additions & 0 deletions tests/neg/java-ann-extends/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def test(x: Ann) =
val y: scala.annotation.Annotation = x // error
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
1 change: 1 addition & 0 deletions tests/neg/java-fake-ann-separate/FakeAnn_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
interface FakeAnn_1 extends java.lang.annotation.Annotation { }
3 changes: 3 additions & 0 deletions tests/neg/java-fake-ann-separate/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@FakeAnn_1 def test = // error
(1: @FakeAnn_1) // error

1 change: 1 addition & 0 deletions tests/neg/java-fake-ann/FakeAnn.java
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
interface FakeAnn extends java.lang.annotation.Annotation { }
2 changes: 2 additions & 0 deletions tests/neg/java-fake-ann/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@FakeAnn def test = // error
(1: @FakeAnn) // 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.

Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading