Skip to content

Commit

Permalink
Java annotations are not classes.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hrhino committed Jul 2, 2018
1 parent 78c0fc9 commit a018b73
Show file tree
Hide file tree
Showing 19 changed files with 182 additions and 52 deletions.
34 changes: 14 additions & 20 deletions spec/11-annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,26 +149,20 @@ Java platform, the following annotations have a standard meaning.
Whenever the static type of an expression matches a specialized variant of
a definition, the compiler will instead use the specialized version.
See the [specialization sid](http://docs.scala-lang.org/sips/completed/scala-specialization.html) for more details of the implementation.


## User-defined Annotations

Other annotations may be interpreted by platform- or
application-dependent tools. Class `scala.Annotation` has two
sub-traits which are used to indicate how these annotations are
retained. Instances of an annotation class inheriting from trait
`scala.ClassfileAnnotation` will be stored in the generated class
files. Instances of an annotation class inheriting from trait
`scala.StaticAnnotation` will be visible to the Scala type-checker
in every compilation unit where the annotated symbol is accessed. An
annotation class can inherit from both `scala.ClassfileAnnotation`
and `scala.StaticAnnotation`. If an annotation class inherits from
neither `scala.ClassfileAnnotation` nor
`scala.StaticAnnotation`, its instances are visible only locally
during the compilation run that analyzes them.

Classes inheriting from `scala.ClassfileAnnotation` may be
subject to further restrictions in order to assure that they can be
mapped to the host environment. In particular, on both the Java and
the .NET platforms, such classes must be toplevel; i.e. they may not
be contained in another class or object. Additionally, on both
Java and .NET, all constructor arguments must be constant expressions.
Other annotations may be interpreted by platform- or application-dependent
tools. The class `scala.annotation.Annotation` is the base class for
user-defined annotations. It has two sub-traits:
- `scala.annotation.StaticAnnotation`: Instances of a subclass of this trait
will be stored in the generated class files, and therefore accessible to
runtime reflection and later compilation runs.
- `scala.annotation.ConstantAnnotation`: Instances of a subclass of this trait
may only have arguments which are
[constant expressions](06-expressions.html#constant-expressions), and will be
stored in the generated class file in the host platform's format.
- If an annotation class inherits from neither `scala.ClassfileAnnotation` nor
`scala.StaticAnnotation`, its instances are visible only locally during the
compilation run that analyzes them.
13 changes: 3 additions & 10 deletions src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,9 @@ abstract class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {

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

// scala/bug#9393: java annotations are interfaces, but the classfile / java source parsers make them look like classes.
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait || sym.hasJavaAnnotationFlag

val classParents = {
val parents = classSym.info.parents
// scala/bug#9393: the classfile / java source parsers add Annotation and StaticAnnotation to the
// parents of a java annotations. undo this for the backend (where we need classfile-level information).
if (classSym.hasJavaAnnotationFlag) parents.filterNot(c => c.typeSymbol == StaticAnnotationClass || c.typeSymbol == AnnotationClass)
else parents
}
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait

val classParents = classSym.info.parents

val minimizedParents = if (classSym.isJavaDefined) classParents else erasure.minimizeParents(classSym, classParents)
// We keep the superClass when computing minimizeParents to eliminate more interfaces.
Expand Down
11 changes: 5 additions & 6 deletions src/compiler/scala/tools/nsc/javac/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -770,11 +770,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
val idefs = members.toList ::: (sdefs flatMap forwarders)
(sdefs, idefs)
}
def annotationParents = List(
gen.scalaAnnotationDot(tpnme.Annotation),
Select(javaLangDot(nme.annotation), tpnme.Annotation),
gen.scalaAnnotationDot(tpnme.StaticAnnotation)
)
def annotationParents = Select(javaLangDot(nme.annotation), tpnme.Annotation) :: Nil
def annotationDecl(mods: Modifiers): List[Tree] = {
accept(AT)
accept(INTERFACE)
Expand All @@ -783,7 +779,10 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
val (statics, body) = typeBody(AT, name)
val templ = makeTemplate(annotationParents, body)
addCompanionObject(statics, atPos(pos) {
ClassDef(mods | Flags.JAVA_ANNOTATION, name, List(), templ)
import Flags._
ClassDef(
mods | JAVA_ANNOTATION | TRAIT | INTERFACE | ABSTRACT,
name, List(), templ)
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,9 @@ abstract class ClassfileParser {
val sflags = jflags.toScalaFlags // includes JAVA

def parseParents(): List[Type] = raiseLoaderLevel {
val superType = if (jflags.isAnnotation) { u2; AnnotationClass.tpe }
else pool.getSuperClass(u2).tpe_*
val superType = pool.getSuperClass(u2).tpe_*
val ifaceCount = u2
var ifaces = for (i <- List.range(0, ifaceCount)) yield pool.getSuperClass(u2).tpe_*
if (jflags.isAnnotation) ifaces ::= StaticAnnotationClass.tpe
val ifaces = for (i <- List.range(0, ifaceCount)) yield pool.getSuperClass(u2).tpe_*
superType :: ifaces
}

Expand Down Expand Up @@ -518,7 +516,7 @@ abstract class ClassfileParser {
val needsConstructor = (
!sawPrivateConstructor
&& !(instanceScope containsName nme.CONSTRUCTOR)
&& (sflags & INTERFACE) == 0
&& ((sflags & INTERFACE) == 0 || (sflags | JAVA_ANNOTATION) != 0)
)
if (needsConstructor)
instanceScope enter clazz.newClassConstructor(NoPosition)
Expand Down
10 changes: 9 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ trait Contexts { self: Analyzer =>
def inSecondTry_=(value: Boolean) = this(SecondTry) = value
def inReturnExpr = this(ReturnExpr)
def inTypeConstructorAllowed = this(TypeConstructorAllowed)
def inAnnotation = this(TypingAnnotation)

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

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

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

/** Are we currently typing the core or args of an annotation?
* When set, Java annotations may be instantiated directly.
*/
final val TypingAnnotation: ContextMode = 1 << 19

/** TODO: The "sticky modes" are EXPRmode, PATTERNmode, TYPEmode.
* To mimic the sticky mode behavior, when captain stickyfingers
* comes around we need to propagate those modes but forget the other
Expand All @@ -1672,7 +1679,8 @@ object ContextMode {
SecondTry -> "SecondTry",
TypeConstructorAllowed -> "TypeConstructorAllowed",
DiagUsedDefaults -> "DiagUsedDefaults",
SuppressDeadArgWarning -> "SuppressDeadArgWarning"
SuppressDeadArgWarning -> "SuppressDeadArgWarning",
TypingAnnotation -> "TypingAnnotation",
)
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,7 @@ trait Namers extends MethodSynthesis {
}
cda.companionModuleClassNamer = templateNamer
}

val classTp = ClassInfoType(parents, decls, clazz)
templateNamer.expandMacroAnnotations(templ.body)
pluginsTypeSig(classTp, templateNamer.typer, templ, WildcardType)
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3740,7 +3740,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
/**
* Convert an annotation constructor call into an AnnotationInfo.
*/
def typedAnnotation(ann: Tree, mode: Mode = EXPRmode): AnnotationInfo = {
def typedAnnotation(ann: Tree, mode: Mode = EXPRmode): AnnotationInfo = context.withinAnnotation {
var hasError: Boolean = false
val pending = ListBuffer[AbsTypeError]()
def ErroneousAnnotation = new ErroneousAnnotation().setOriginal(ann)
Expand Down Expand Up @@ -4601,7 +4601,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper

val tp = tpt1.tpe
val sym = tp.typeSymbol.initialize
if (sym.isAbstractType || sym.hasAbstractFlag)
if ((sym.isAbstractType || sym.hasAbstractFlag)
&& !(sym.isJavaAnnotation && context.inAnnotation))
IsAbstractError(tree, sym)
else if (isPrimitiveValueClass(sym)) {
NotAMemberError(tpt, TypeTree(tp), nme.CONSTRUCTOR)
Expand Down
2 changes: 1 addition & 1 deletion src/reflect/scala/reflect/internal/AnnotationInfos.scala
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable =>
/** Check whether the type or any of the arguments are erroneous */
def isErroneous = atp.isErroneous || args.exists(_.isErroneous)

def isStatic = symbol isNonBottomSubClass StaticAnnotationClass
final def isStatic = symbol.isStaticAnnotation

/** Check whether any of the arguments mention a symbol */
def refsSymbol(sym: Symbol) = hasArgWhich(_.symbol == sym)
Expand Down
4 changes: 2 additions & 2 deletions src/reflect/scala/reflect/internal/ClassfileConstants.scala
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ object ClassfileConstants {
case JAVA_ACC_FINAL => FINAL
case JAVA_ACC_SYNTHETIC => SYNTHETIC | ARTIFACT // maybe should be just artifact?
case JAVA_ACC_STATIC => STATIC
case JAVA_ACC_ABSTRACT => if (isAnnotation) 0L else if (isClass) ABSTRACT else DEFERRED
case JAVA_ACC_INTERFACE => if (isAnnotation) 0L else TRAIT | INTERFACE | ABSTRACT
case JAVA_ACC_ABSTRACT => if (isClass) ABSTRACT else DEFERRED
case JAVA_ACC_INTERFACE => TRAIT | INTERFACE | ABSTRACT
case JAVA_ACC_ENUM => JAVA_ENUM
case JAVA_ACC_ANNOTATION => JAVA_ANNOTATION
case _ => 0L
Expand Down
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/internal/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>

def isJavaEnum: Boolean = hasJavaEnumFlag
def isJavaAnnotation: Boolean = hasJavaAnnotationFlag
def isStaticAnnotation: Boolean =
hasJavaAnnotationFlag || isNonBottomSubClass(StaticAnnotationClass)

def newNestedSymbol(name: Name, pos: Position, newFlags: Long, isClass: Boolean): Symbol = name match {
case n: TermName => newTermSymbol(n, pos, newFlags)
Expand Down
4 changes: 1 addition & 3 deletions src/reflect/scala/reflect/runtime/JavaMirrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -760,9 +760,7 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
parentsLevel += 1
val jsuperclazz = jclazz.getGenericSuperclass
val ifaces = jclazz.getGenericInterfaces.toList map typeToScala
val isAnnotation = JavaAccFlags(jclazz).isAnnotation
if (isAnnotation) AnnotationClass.tpe :: StaticAnnotationClass.tpe :: ifaces
else if (jclazz.isInterface) ObjectTpe :: ifaces // interfaces have Object as superclass in the classfile (see jvm spec), but getGenericSuperclass seems to return null
if (jclazz.isInterface) ObjectTpe :: ifaces // interfaces have Object as superclass in the classfile (see jvm spec), but getGenericSuperclass seems to return null
else (if (jsuperclazz == null) AnyTpe else typeToScala(jsuperclazz)) :: ifaces
} finally {
parentsLevel -= 1
Expand Down
43 changes: 43 additions & 0 deletions test/files/neg/java-annotation-bad.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
Test_1.scala:12: error: Java annotation Ann_0 is abstract; cannot be instantiated
val a: Ann_0 = new Ann_0 // nok
^
Test_1.scala:13: error: Java annotation Ann_0 is abstract; cannot be instantiated
val b: Ann_0 = new Ann_0(Array()) // nok
^
Test_1.scala:14: error: Java annotation Ann_1 is abstract; cannot be instantiated
val c: Ann_1 = new Ann_1 // nok
^
Test_1.scala:15: error: Java annotation Ann_1 is abstract; cannot be instantiated
val d: Ann_1 = new Ann_1(Array()) // nok
^
Test_1.scala:18: error: type mismatch;
found : ann.Ann_0
required: scala.annotation.Annotation
val e: Annotation = a // nok
^
Test_1.scala:19: error: type mismatch;
found : ann.Ann_1
required: scala.annotation.Annotation
val f: Annotation = c // nok
^
Test_1.scala:20: error: type mismatch;
found : ann.Ann_0
required: scala.annotation.StaticAnnotation
val g: StaticAnnotation = a // nok
^
Test_1.scala:21: error: type mismatch;
found : ann.Ann_1
required: scala.annotation.StaticAnnotation
val h: StaticAnnotation = c // nok
^
Test_1.scala:22: error: type mismatch;
found : ann.Ann_0
required: scala.annotation.ConstantAnnotation
val i: ConstantAnnotation = a // nok
^
Test_1.scala:23: error: type mismatch;
found : ann.Ann_1
required: scala.annotation.ConstantAnnotation
val j: ConstantAnnotation = c // nok
^
10 errors found
7 changes: 7 additions & 0 deletions test/files/neg/java-annotation-bad/Ann_0.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package ann;

public @interface Ann_0 {
N[] value();

public @interface N {}
}
7 changes: 7 additions & 0 deletions test/files/neg/java-annotation-bad/Ann_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package ann;

public @interface Ann_1 {
N[] value();

public @interface N {}
}
30 changes: 30 additions & 0 deletions test/files/neg/java-annotation-bad/Test_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
object Test {
import ann._

// ok
@Ann_0(Array(new Ann_0.N, new Ann_0.N))
class A

// ok
@Ann_1(Array(new Ann_1.N, new Ann_1.N))
class B

val a: Ann_0 = new Ann_0 // nok
val b: Ann_0 = new Ann_0(Array()) // nok
val c: Ann_1 = new Ann_1 // nok
val d: Ann_1 = new Ann_1(Array()) // nok

import scala.annotation._, java.lang.{annotation => jla}
val e: Annotation = a // nok
val f: Annotation = c // nok
val g: StaticAnnotation = a // nok
val h: StaticAnnotation = c // nok
val i: ConstantAnnotation = a // nok
val j: ConstantAnnotation = c // nok
val k: jla.Annotation = a // ok
val l: jla.Annotation = c // ok

val m = new Ann_0 { val annotationType = classOf[Ann_0] } // ok
val n = new Ann_1 { val annotationType = classOf[Ann_1] } // ok

}
2 changes: 1 addition & 1 deletion test/files/neg/nested-annotation.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import annotation._

class ComplexAnnotation(val value: Annotation) extends ConstantAnnotation
class ComplexAnnotation(val value: Any) extends ConstantAnnotation

class A {
// It's hard to induce this error because @ComplexAnnotation(@inline) is a parse
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/t5699.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package <empty> {
object MyAnnotation extends {
def <init>()
};
class MyAnnotation extends scala.annotation.Annotation with _root_.java.lang.annotation.Annotation with scala.annotation.StaticAnnotation {
abstract trait MyAnnotation extends _root_.java.lang.annotation.Annotation {
def <init>();
def value(): String
}
Expand Down
24 changes: 24 additions & 0 deletions test/files/run/t9400.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

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

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)
}
25 changes: 25 additions & 0 deletions test/files/run/t9644.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import java.lang.annotation._

@Deprecated @Retention(RetentionPolicy.RUNTIME) class Foo

object Test extends App {
classOf[Foo].getAnnotation(classOf[Deprecated])

assert(classOf[Foo].getAnnotation(classOf[Retention]).value() == RetentionPolicy.RUNTIME)

import reflect.runtime.universe._

val List(d, r) = symbolOf[Foo].annotations

d.tree match {
case Apply(Select(New(tpt), _), Nil) =>
assert (tpt.tpe.typeSymbol == symbolOf[Deprecated], tpt.tpe.typeSymbol)
}

val RetentionPolicy_RUNTIME = symbolOf[RetentionPolicy].companion.info.decl(TermName("RUNTIME"))
r.tree match {
case Apply(Select(New(tpt), _), List(NamedArg(Ident(TermName("value")), Literal(Constant(RetentionPolicy_RUNTIME))))) =>
assert (tpt.tpe.typeSymbol == symbolOf[Retention], tpt.tpe.typeSymbol)
}

}

0 comments on commit a018b73

Please sign in to comment.