Skip to content

Commit a018b73

Browse files
committed
Java annotations are not classes.
Previously, `JavaParsers` and `ClassfileParser` would "fix-up" Java annotations to be non-abstract classes with `Annotation` and `StaticAnnotation` as parents. This caused all manner of strangeness and invalid classfiles (see linked tickets) and was surprising, to say the least. Now, the only special dispensation given to Java annotations is that they get a public, no-args class constructor, because that's what it takes to get `typedAnnotation` to be able to look up the type. This means that `new Foo`, where `Foo` is a Java annotation, still compiles (as it does right now, actually). Part of the reason for this commit is to provoke a discussion about how to fix that. Fixes scala/bug#8778. Fixes scala/bug#9400. Fixes scala/bug#9644.
1 parent 78c0fc9 commit a018b73

File tree

19 files changed

+182
-52
lines changed

19 files changed

+182
-52
lines changed

spec/11-annotations.md

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -149,26 +149,20 @@ Java platform, the following annotations have a standard meaning.
149149
Whenever the static type of an expression matches a specialized variant of
150150
a definition, the compiler will instead use the specialized version.
151151
See the [specialization sid](http://docs.scala-lang.org/sips/completed/scala-specialization.html) for more details of the implementation.
152+
152153

153154
## User-defined Annotations
154155

155-
Other annotations may be interpreted by platform- or
156-
application-dependent tools. Class `scala.Annotation` has two
157-
sub-traits which are used to indicate how these annotations are
158-
retained. Instances of an annotation class inheriting from trait
159-
`scala.ClassfileAnnotation` will be stored in the generated class
160-
files. Instances of an annotation class inheriting from trait
161-
`scala.StaticAnnotation` will be visible to the Scala type-checker
162-
in every compilation unit where the annotated symbol is accessed. An
163-
annotation class can inherit from both `scala.ClassfileAnnotation`
164-
and `scala.StaticAnnotation`. If an annotation class inherits from
165-
neither `scala.ClassfileAnnotation` nor
166-
`scala.StaticAnnotation`, its instances are visible only locally
167-
during the compilation run that analyzes them.
168-
169-
Classes inheriting from `scala.ClassfileAnnotation` may be
170-
subject to further restrictions in order to assure that they can be
171-
mapped to the host environment. In particular, on both the Java and
172-
the .NET platforms, such classes must be toplevel; i.e. they may not
173-
be contained in another class or object. Additionally, on both
174-
Java and .NET, all constructor arguments must be constant expressions.
156+
Other annotations may be interpreted by platform- or application-dependent
157+
tools. The class `scala.annotation.Annotation` is the base class for
158+
user-defined annotations. It has two sub-traits:
159+
- `scala.annotation.StaticAnnotation`: Instances of a subclass of this trait
160+
will be stored in the generated class files, and therefore accessible to
161+
runtime reflection and later compilation runs.
162+
- `scala.annotation.ConstantAnnotation`: Instances of a subclass of this trait
163+
may only have arguments which are
164+
[constant expressions](06-expressions.html#constant-expressions), and will be
165+
stored in the generated class file in the host platform's format.
166+
- If an annotation class inherits from neither `scala.ClassfileAnnotation` nor
167+
`scala.StaticAnnotation`, its instances are visible only locally during the
168+
compilation run that analyzes them.

src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,9 @@ abstract class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
207207

208208
def implementedInterfaces(classSym: Symbol): List[Symbol] = {
209209

210-
// scala/bug#9393: java annotations are interfaces, but the classfile / java source parsers make them look like classes.
211-
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait || sym.hasJavaAnnotationFlag
212-
213-
val classParents = {
214-
val parents = classSym.info.parents
215-
// scala/bug#9393: the classfile / java source parsers add Annotation and StaticAnnotation to the
216-
// parents of a java annotations. undo this for the backend (where we need classfile-level information).
217-
if (classSym.hasJavaAnnotationFlag) parents.filterNot(c => c.typeSymbol == StaticAnnotationClass || c.typeSymbol == AnnotationClass)
218-
else parents
219-
}
210+
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait
211+
212+
val classParents = classSym.info.parents
220213

221214
val minimizedParents = if (classSym.isJavaDefined) classParents else erasure.minimizeParents(classSym, classParents)
222215
// We keep the superClass when computing minimizeParents to eliminate more interfaces.

src/compiler/scala/tools/nsc/javac/JavaParsers.scala

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -770,11 +770,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
770770
val idefs = members.toList ::: (sdefs flatMap forwarders)
771771
(sdefs, idefs)
772772
}
773-
def annotationParents = List(
774-
gen.scalaAnnotationDot(tpnme.Annotation),
775-
Select(javaLangDot(nme.annotation), tpnme.Annotation),
776-
gen.scalaAnnotationDot(tpnme.StaticAnnotation)
777-
)
773+
def annotationParents = Select(javaLangDot(nme.annotation), tpnme.Annotation) :: Nil
778774
def annotationDecl(mods: Modifiers): List[Tree] = {
779775
accept(AT)
780776
accept(INTERFACE)
@@ -783,7 +779,10 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
783779
val (statics, body) = typeBody(AT, name)
784780
val templ = makeTemplate(annotationParents, body)
785781
addCompanionObject(statics, atPos(pos) {
786-
ClassDef(mods | Flags.JAVA_ANNOTATION, name, List(), templ)
782+
import Flags._
783+
ClassDef(
784+
mods | JAVA_ANNOTATION | TRAIT | INTERFACE | ABSTRACT,
785+
name, List(), templ)
787786
})
788787
}
789788

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -476,11 +476,9 @@ abstract class ClassfileParser {
476476
val sflags = jflags.toScalaFlags // includes JAVA
477477

478478
def parseParents(): List[Type] = raiseLoaderLevel {
479-
val superType = if (jflags.isAnnotation) { u2; AnnotationClass.tpe }
480-
else pool.getSuperClass(u2).tpe_*
479+
val superType = pool.getSuperClass(u2).tpe_*
481480
val ifaceCount = u2
482-
var ifaces = for (i <- List.range(0, ifaceCount)) yield pool.getSuperClass(u2).tpe_*
483-
if (jflags.isAnnotation) ifaces ::= StaticAnnotationClass.tpe
481+
val ifaces = for (i <- List.range(0, ifaceCount)) yield pool.getSuperClass(u2).tpe_*
484482
superType :: ifaces
485483
}
486484

@@ -518,7 +516,7 @@ abstract class ClassfileParser {
518516
val needsConstructor = (
519517
!sawPrivateConstructor
520518
&& !(instanceScope containsName nme.CONSTRUCTOR)
521-
&& (sflags & INTERFACE) == 0
519+
&& ((sflags & INTERFACE) == 0 || (sflags | JAVA_ANNOTATION) != 0)
522520
)
523521
if (needsConstructor)
524522
instanceScope enter clazz.newClassConstructor(NoPosition)

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ trait Contexts { self: Analyzer =>
270270
def inSecondTry_=(value: Boolean) = this(SecondTry) = value
271271
def inReturnExpr = this(ReturnExpr)
272272
def inTypeConstructorAllowed = this(TypeConstructorAllowed)
273+
def inAnnotation = this(TypingAnnotation)
273274

274275
def defaultModeForTyped: Mode = if (inTypeConstructorAllowed) Mode.NOmode else Mode.EXPRmode
275276

@@ -403,6 +404,7 @@ trait Contexts { self: Analyzer =>
403404
@inline final def withinSuperInit[T](op: => T): T = withMode(enabled = SuperInit)(op)
404405
@inline final def withinSecondTry[T](op: => T): T = withMode(enabled = SecondTry)(op)
405406
@inline final def withinPatAlternative[T](op: => T): T = withMode(enabled = PatternAlternative)(op)
407+
@inline final def withinAnnotation[T](op: => T): T = withMode(enabled = TypingAnnotation)(op)
406408

407409
@inline final def withSuppressDeadArgWarning[T](suppress: Boolean)(op: => T): T =
408410
if (suppress) withMode(enabled = SuppressDeadArgWarning)(op) else withMode(disabled = SuppressDeadArgWarning)(op)
@@ -1647,6 +1649,11 @@ object ContextMode {
16471649
/** Were default arguments used? */
16481650
final val DiagUsedDefaults: ContextMode = 1 << 18
16491651

1652+
/** Are we currently typing the core or args of an annotation?
1653+
* When set, Java annotations may be instantiated directly.
1654+
*/
1655+
final val TypingAnnotation: ContextMode = 1 << 19
1656+
16501657
/** TODO: The "sticky modes" are EXPRmode, PATTERNmode, TYPEmode.
16511658
* To mimic the sticky mode behavior, when captain stickyfingers
16521659
* comes around we need to propagate those modes but forget the other
@@ -1672,7 +1679,8 @@ object ContextMode {
16721679
SecondTry -> "SecondTry",
16731680
TypeConstructorAllowed -> "TypeConstructorAllowed",
16741681
DiagUsedDefaults -> "DiagUsedDefaults",
1675-
SuppressDeadArgWarning -> "SuppressDeadArgWarning"
1682+
SuppressDeadArgWarning -> "SuppressDeadArgWarning",
1683+
TypingAnnotation -> "TypingAnnotation",
16761684
)
16771685
}
16781686

src/compiler/scala/tools/nsc/typechecker/Namers.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,7 @@ trait Namers extends MethodSynthesis {
12021202
}
12031203
cda.companionModuleClassNamer = templateNamer
12041204
}
1205+
12051206
val classTp = ClassInfoType(parents, decls, clazz)
12061207
templateNamer.expandMacroAnnotations(templ.body)
12071208
pluginsTypeSig(classTp, templateNamer.typer, templ, WildcardType)

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3740,7 +3740,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
37403740
/**
37413741
* Convert an annotation constructor call into an AnnotationInfo.
37423742
*/
3743-
def typedAnnotation(ann: Tree, mode: Mode = EXPRmode): AnnotationInfo = {
3743+
def typedAnnotation(ann: Tree, mode: Mode = EXPRmode): AnnotationInfo = context.withinAnnotation {
37443744
var hasError: Boolean = false
37453745
val pending = ListBuffer[AbsTypeError]()
37463746
def ErroneousAnnotation = new ErroneousAnnotation().setOriginal(ann)
@@ -4601,7 +4601,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
46014601

46024602
val tp = tpt1.tpe
46034603
val sym = tp.typeSymbol.initialize
4604-
if (sym.isAbstractType || sym.hasAbstractFlag)
4604+
if ((sym.isAbstractType || sym.hasAbstractFlag)
4605+
&& !(sym.isJavaAnnotation && context.inAnnotation))
46054606
IsAbstractError(tree, sym)
46064607
else if (isPrimitiveValueClass(sym)) {
46074608
NotAMemberError(tpt, TypeTree(tp), nme.CONSTRUCTOR)

src/reflect/scala/reflect/internal/AnnotationInfos.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable =>
269269
/** Check whether the type or any of the arguments are erroneous */
270270
def isErroneous = atp.isErroneous || args.exists(_.isErroneous)
271271

272-
def isStatic = symbol isNonBottomSubClass StaticAnnotationClass
272+
final def isStatic = symbol.isStaticAnnotation
273273

274274
/** Check whether any of the arguments mention a symbol */
275275
def refsSymbol(sym: Symbol) = hasArgWhich(_.symbol == sym)

src/reflect/scala/reflect/internal/ClassfileConstants.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,8 @@ object ClassfileConstants {
342342
case JAVA_ACC_FINAL => FINAL
343343
case JAVA_ACC_SYNTHETIC => SYNTHETIC | ARTIFACT // maybe should be just artifact?
344344
case JAVA_ACC_STATIC => STATIC
345-
case JAVA_ACC_ABSTRACT => if (isAnnotation) 0L else if (isClass) ABSTRACT else DEFERRED
346-
case JAVA_ACC_INTERFACE => if (isAnnotation) 0L else TRAIT | INTERFACE | ABSTRACT
345+
case JAVA_ACC_ABSTRACT => if (isClass) ABSTRACT else DEFERRED
346+
case JAVA_ACC_INTERFACE => TRAIT | INTERFACE | ABSTRACT
347347
case JAVA_ACC_ENUM => JAVA_ENUM
348348
case JAVA_ACC_ANNOTATION => JAVA_ANNOTATION
349349
case _ => 0L

src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
118118

119119
def isJavaEnum: Boolean = hasJavaEnumFlag
120120
def isJavaAnnotation: Boolean = hasJavaAnnotationFlag
121+
def isStaticAnnotation: Boolean =
122+
hasJavaAnnotationFlag || isNonBottomSubClass(StaticAnnotationClass)
121123

122124
def newNestedSymbol(name: Name, pos: Position, newFlags: Long, isClass: Boolean): Symbol = name match {
123125
case n: TermName => newTermSymbol(n, pos, newFlags)

src/reflect/scala/reflect/runtime/JavaMirrors.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -760,9 +760,7 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
760760
parentsLevel += 1
761761
val jsuperclazz = jclazz.getGenericSuperclass
762762
val ifaces = jclazz.getGenericInterfaces.toList map typeToScala
763-
val isAnnotation = JavaAccFlags(jclazz).isAnnotation
764-
if (isAnnotation) AnnotationClass.tpe :: StaticAnnotationClass.tpe :: ifaces
765-
else if (jclazz.isInterface) ObjectTpe :: ifaces // interfaces have Object as superclass in the classfile (see jvm spec), but getGenericSuperclass seems to return null
763+
if (jclazz.isInterface) ObjectTpe :: ifaces // interfaces have Object as superclass in the classfile (see jvm spec), but getGenericSuperclass seems to return null
766764
else (if (jsuperclazz == null) AnyTpe else typeToScala(jsuperclazz)) :: ifaces
767765
} finally {
768766
parentsLevel -= 1
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
Test_1.scala:12: error: Java annotation Ann_0 is abstract; cannot be instantiated
2+
val a: Ann_0 = new Ann_0 // nok
3+
^
4+
Test_1.scala:13: error: Java annotation Ann_0 is abstract; cannot be instantiated
5+
val b: Ann_0 = new Ann_0(Array()) // nok
6+
^
7+
Test_1.scala:14: error: Java annotation Ann_1 is abstract; cannot be instantiated
8+
val c: Ann_1 = new Ann_1 // nok
9+
^
10+
Test_1.scala:15: error: Java annotation Ann_1 is abstract; cannot be instantiated
11+
val d: Ann_1 = new Ann_1(Array()) // nok
12+
^
13+
Test_1.scala:18: error: type mismatch;
14+
found : ann.Ann_0
15+
required: scala.annotation.Annotation
16+
val e: Annotation = a // nok
17+
^
18+
Test_1.scala:19: error: type mismatch;
19+
found : ann.Ann_1
20+
required: scala.annotation.Annotation
21+
val f: Annotation = c // nok
22+
^
23+
Test_1.scala:20: error: type mismatch;
24+
found : ann.Ann_0
25+
required: scala.annotation.StaticAnnotation
26+
val g: StaticAnnotation = a // nok
27+
^
28+
Test_1.scala:21: error: type mismatch;
29+
found : ann.Ann_1
30+
required: scala.annotation.StaticAnnotation
31+
val h: StaticAnnotation = c // nok
32+
^
33+
Test_1.scala:22: error: type mismatch;
34+
found : ann.Ann_0
35+
required: scala.annotation.ConstantAnnotation
36+
val i: ConstantAnnotation = a // nok
37+
^
38+
Test_1.scala:23: error: type mismatch;
39+
found : ann.Ann_1
40+
required: scala.annotation.ConstantAnnotation
41+
val j: ConstantAnnotation = c // nok
42+
^
43+
10 errors found
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package ann;
2+
3+
public @interface Ann_0 {
4+
N[] value();
5+
6+
public @interface N {}
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package ann;
2+
3+
public @interface Ann_1 {
4+
N[] value();
5+
6+
public @interface N {}
7+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
object Test {
2+
import ann._
3+
4+
// ok
5+
@Ann_0(Array(new Ann_0.N, new Ann_0.N))
6+
class A
7+
8+
// ok
9+
@Ann_1(Array(new Ann_1.N, new Ann_1.N))
10+
class B
11+
12+
val a: Ann_0 = new Ann_0 // nok
13+
val b: Ann_0 = new Ann_0(Array()) // nok
14+
val c: Ann_1 = new Ann_1 // nok
15+
val d: Ann_1 = new Ann_1(Array()) // nok
16+
17+
import scala.annotation._, java.lang.{annotation => jla}
18+
val e: Annotation = a // nok
19+
val f: Annotation = c // nok
20+
val g: StaticAnnotation = a // nok
21+
val h: StaticAnnotation = c // nok
22+
val i: ConstantAnnotation = a // nok
23+
val j: ConstantAnnotation = c // nok
24+
val k: jla.Annotation = a // ok
25+
val l: jla.Annotation = c // ok
26+
27+
val m = new Ann_0 { val annotationType = classOf[Ann_0] } // ok
28+
val n = new Ann_1 { val annotationType = classOf[Ann_1] } // ok
29+
30+
}

test/files/neg/nested-annotation.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import annotation._
22

3-
class ComplexAnnotation(val value: Annotation) extends ConstantAnnotation
3+
class ComplexAnnotation(val value: Any) extends ConstantAnnotation
44

55
class A {
66
// It's hard to induce this error because @ComplexAnnotation(@inline) is a parse

test/files/run/t5699.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package <empty> {
33
object MyAnnotation extends {
44
def <init>()
55
};
6-
class MyAnnotation extends scala.annotation.Annotation with _root_.java.lang.annotation.Annotation with scala.annotation.StaticAnnotation {
6+
abstract trait MyAnnotation extends _root_.java.lang.annotation.Annotation {
77
def <init>();
88
def value(): String
99
}

test/files/run/t9400.scala

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
class Deprecation extends Deprecated {
3+
final val annotationType = classOf[Deprecated]
4+
}
5+
6+
class Suppression extends SuppressWarnings {
7+
final val annotationType = classOf[SuppressWarnings]
8+
9+
def value = Array("unchecked")
10+
}
11+
12+
class Retention(runtime: Boolean) extends java.lang.annotation.Retention {
13+
final val annotationType = classOf[Retention]
14+
15+
def value =
16+
if (runtime) java.lang.annotation.RetentionPolicy.RUNTIME
17+
else java.lang.annotation.RetentionPolicy.SOURCE
18+
}
19+
20+
object Test extends App {
21+
new Deprecation
22+
new Suppression
23+
new Retention(true)
24+
}

test/files/run/t9644.scala

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import java.lang.annotation._
2+
3+
@Deprecated @Retention(RetentionPolicy.RUNTIME) class Foo
4+
5+
object Test extends App {
6+
classOf[Foo].getAnnotation(classOf[Deprecated])
7+
8+
assert(classOf[Foo].getAnnotation(classOf[Retention]).value() == RetentionPolicy.RUNTIME)
9+
10+
import reflect.runtime.universe._
11+
12+
val List(d, r) = symbolOf[Foo].annotations
13+
14+
d.tree match {
15+
case Apply(Select(New(tpt), _), Nil) =>
16+
assert (tpt.tpe.typeSymbol == symbolOf[Deprecated], tpt.tpe.typeSymbol)
17+
}
18+
19+
val RetentionPolicy_RUNTIME = symbolOf[RetentionPolicy].companion.info.decl(TermName("RUNTIME"))
20+
r.tree match {
21+
case Apply(Select(New(tpt), _), List(NamedArg(Ident(TermName("value")), Literal(Constant(RetentionPolicy_RUNTIME))))) =>
22+
assert (tpt.tpe.typeSymbol == symbolOf[Retention], tpt.tpe.typeSymbol)
23+
}
24+
25+
}

0 commit comments

Comments
 (0)