Check all annotations

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

@@ -108,52 +108,6 @@ object Annotations {
       go(metaSyms) || orNoneOf.nonEmpty && !go(orNoneOf)
     }
 
-    /** Checks if this annotation can be used as a type annotation. Reports one
-     *  or more errors if it cannot.
-     */
-    final def checkValidTypeAnnotation()(using Context): Unit =
-      def isTupleModule(sym: Symbol): Boolean =
-        ctx.definitions.isTupleClass(sym.companionClass)
-
-      def isFunctionAllowed(t: Tree): Boolean =
-        t match
-          case Select(qual, nme.apply) => qual.symbol == defn.ArrayModule || isTupleModule(qual.symbol)
-          case Apply(fun, _) => isFunctionAllowed(fun)
-          case TypeApply(fun, _) => isFunctionAllowed(fun)
-          case _ => false
-
-      def check(t: Tree): Boolean =
-        t match
-          case Literal(_) => true
-          case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(check)
-          case SeqLiteral(elems, _) => elems.forall(check)
-          case Typed(expr, _) => check(expr)
-          case NamedArg(_, arg) => check(arg)
-          case _ =>
-            t.tpe.stripped match
-              case _: SingletonType => true
-              // We need to handle type refs for these test cases:
-              // - tests/pos/dependent-annot.scala
-              // - tests/pos/i16208.scala
-              // - tests/run/java-ann-super-class
-              // - tests/run/java-ann-super-class-separate
-              // - tests/neg/i19470.scala (@retains)
-              // Why do we get type refs in these cases?
-              case _: TypeRef => true
-              case _: TypeParamRef => true
-              case tp => false
-
-      val uncheckedAnnots = Set[Symbol](defn.RetainsAnnot, defn.RetainsByNameAnnot)
-      if uncheckedAnnots(symbol) then return
-
-      for arg <- arguments if !check(arg)  do
-        report.error(
-          s"""Implementation restriction: not a valid type annotation argument.
-             |  Argument: $arg
-             |  Type: ${arg.tpe}""".stripMargin, arg.srcPos)
-
-      ()
-
     /** Operations for hash-consing, can be overridden */
     def hash: Int = System.identityHashCode(this)
     def eql(that: Annotation) = this eq that

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

@@ -5754,7 +5754,6 @@ object Types extends TypeUtils {
     def make(underlying: Type, annots: List[Annotation])(using Context): Type =
       annots.foldLeft(underlying)(apply(_, _))
     def apply(parent: Type, annot: Annotation)(using Context): AnnotatedType =
-      annot.checkValidTypeAnnotation()
       unique(CachedAnnotatedType(parent, annot))
   end AnnotatedType
 

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

@@ -68,6 +68,7 @@ class TreeChecker extends Phase with SymTransformer {
 
   def transformSym(symd: SymDenotation)(using Context): SymDenotation = {
     val sym = symd.symbol
+    Checking.checkWellFormedType(symd.info)
 
     if (sym.isClass && !sym.isAbsent()) {
       val validSuperclass = sym.isPrimitiveValueClass || defn.syntheticCoreClasses.contains(sym) ||
@@ -827,6 +828,8 @@ object TreeChecker {
             |${mismatch.message}${mismatch.explanation}
             |tree = $tree ${tree.className}""".stripMargin
       })
+      Checking.checkWellFormedType(tp1)
+      Checking.checkWellFormedType(tp2)
   }
 
   /** Tree checker that can be applied to a local tree. */

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

@@ -604,6 +604,17 @@ object Checking {
     checkNoConflict(Lazy, ParamAccessor, em"parameter may not be `lazy`")
   }
 
+  /** Check that the type `tp` is well-formed. Currently this only means
+   *  checking that annotated types have valid annotation arguments.
+   */
+  def checkWellFormedType(tp: Type)(using Context) =
+    tp.foreachPart:
+      case AnnotatedType(underlying, annot) =>
+        // Not checking `checkAnnotClass` because `Transform.toTypeTree` creates
+        // `Annotated` whose trees are not annotation instantiations.
+        checkAnnotArgs(annot.tree)
+      case _ => ()
+
   /** Check for illegal or redundant modifiers on modules. This is done separately
    *  from checkWellformed, since the original module modifiers don't surivive desugaring
    */
@@ -915,6 +926,69 @@ object Checking {
       case _ => annot
   end checkNamedArgumentForJavaAnnotation
 
+  /** Check arguments of annotations */
+  private def checkAnnotArgs(tree: Tree)(using Context): Tree =
+    val cls = Annotations.annotClass(tree)
+    tree match
+      case Apply(tycon, arg :: Nil) if cls == defn.TargetNameAnnot =>
+        arg match
+          case Literal(Constant("")) =>
+            report.error(em"target name cannot be empty", arg.srcPos)
+          case Literal(_) => // ok
+          case _ =>
+            report.error(em"@${cls.name} needs a string literal as argument", arg.srcPos)
+      case Apply(tycon, args) => args.foreach(checkAnnotArg)
+      case _ =>
+        // `FirstTransform.toTypeTree` creates `Annotated` nodes where the
+        // annotation tree is not an `Apply` node. For example, in
+        // tests/neg/i13044.scala, `FirstTransform.toTypeTree` creates two
+        // annotations with trees `t` and `ts`. In tests/run/t2755.scala,
+        // `FirstTransform.toTypeTree` creates an annotated types with tree `_`.
+        //
+        // report.error(em"unexpected annotation tree: $tree", tree.srcPos)
+        ()
+    tree
+
+  private def checkAnnotArg(tree: Tree)(using Context): Unit =
+    def isTupleModule(sym: Symbol): Boolean =
+      ctx.definitions.isTupleClass(sym.companionClass)
+
+    def isFunctionAllowed(t: Tree): Boolean =
+      t match
+        case Select(qual, nme.apply) => qual.symbol == defn.ArrayModule || isTupleModule(qual.symbol)
+        case Apply(fun, _) => isFunctionAllowed(fun)
+        case TypeApply(fun, _) => isFunctionAllowed(fun)
+        case _ => false
+
+    def valid(t: Tree): Boolean =
+      t match
+        case Literal(_) => true
+        case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(valid)
+        case SeqLiteral(elems, _) => elems.forall(valid)
+        case Typed(expr, _) => valid(expr)
+        case NamedArg(_, arg) => valid(arg)
+        case _ =>
+          t.tpe.stripped match
+            case _: SingletonType => true
+            // We need to handle type refs for these test cases:
+            // - tests/pos/dependent-annot.scala
+            // - tests/pos/i16208.scala
+            // - tests/run/java-ann-super-class
+            // - tests/run/java-ann-super-class-separate
+            // - tests/neg/i19470.scala (@retains)
+            // Why do we get type refs in these cases?
+            case _: TypeRef => true
+            case _: TypeParamRef => true
+            case tp => false
+
+    if !valid(tree) then
+      report.error(
+        s"""Implementation restriction: not a valid annotation argument.
+           |  Argument: $tree
+           |  Type: ${tree.tpe}""".stripMargin,
+        tree.srcPos
+      )
+
 }
 
 trait Checking {
@@ -1385,12 +1459,15 @@ trait Checking {
     if !Inlines.inInlineMethod && !ctx.isInlineContext then
       report.error(em"$what can only be used in an inline method", pos)
 
+  def checkAnnot(tree: Tree)(using Context): Tree =
+    Checking.checkAnnotArgs(checkAnnotClass(tree))
+
   /** 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 =
+  private def checkAnnotClass(tree: Tree)(using Context): Tree =
     if tree.tpe.isError then
       return tree
     val cls = Annotations.annotClass(tree)
@@ -1401,21 +1478,7 @@ trait Checking {
     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)
-    tree match
-      case Apply(tycon, arg :: Nil) if cls == defn.TargetNameAnnot =>
-        arg match
-          case Literal(Constant("")) =>
-            report.error(em"target name cannot be empty", arg.srcPos)
-          case Literal(_) => // ok
-          case _ =>
-            report.error(em"@${cls.name} needs a string literal as argument", arg.srcPos)
-      case _ =>
-    tree
-
+
   /** 1. Check that all case classes that extend `scala.reflect.Enum` are `enum` cases
    *  2. Check that parameterised `enum` cases do not extend java.lang.Enum.
    *  3. Check that only a static `enum` base class can extend java.lang.Enum.
@@ -1663,7 +1726,7 @@ trait NoChecking extends ReChecking {
   override def checkImplicitConversionDefOK(sym: Symbol)(using Context): Unit = ()
   override def checkImplicitConversionUseOK(tree: Tree, expected: Type)(using Context): Unit = ()
   override def checkFeasibleParent(tp: Type, pos: SrcPos, where: => String = "")(using Context): Type = tp
-  override def checkAnnotArgs(tree: Tree)(using Context): tree.type = tree
+  override def checkAnnot(tree: Tree)(using Context): tree.type = tree
   override def checkNoTargetNameConflict(stats: List[Tree])(using Context): Unit = ()
   override def checkParentCall(call: Tree, caller: ClassSymbol)(using Context): Unit = ()
   override def checkSimpleKinded(tpt: Tree)(using Context): Tree = tpt

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

@@ -2780,7 +2780,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   }
 
   def typedAnnotation(annot: untpd.Tree)(using Context): Tree =
-    checkAnnotClass(checkAnnotArgs(typed(annot)))
+    checkAnnot(typed(annot))
 
   def registerNowarn(tree: Tree, mdef: untpd.Tree)(using Context): Unit =
     val annot = Annotations.Annotation(tree)
@@ -3307,7 +3307,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   end typedPackageDef
 
   def typedAnnotated(tree: untpd.Annotated, pt: Type)(using Context): Tree = {
-    val annot1 = checkAnnotClass(typedExpr(tree.annot))
+    val annot1 = checkAnnot(typedExpr(tree.annot))
     val annotCls = Annotations.annotClass(annot1)
     if annotCls == defn.NowarnAnnot then
       registerNowarn(annot1, tree)

tests/neg/annot-invalid.scala

@@ -0,0 +1,17 @@
+class annot[T](arg: T) extends scala.annotation.Annotation
+
+def main =
+  val n: Int = 0
+  def f(x: Any): Unit = ()
+
+  val x1: Int @annot(n + 1) = 0 // error
+  val x2: Int @annot(f(2)) = 0 // error
+  val x3: Int @annot(throw new Error()) = 0 // error
+  val x4: Int @annot((x: Int) => x) = 0 // error
+
+  @annot(m1(2)) val y1: Int = 0 // error
+  @annot(throw new Error()) val y2: Int = 0 // error
+  @annot((x: Int) => x) val y3: Int = 0  // error
+  @annot(x + 1) val y4: Int = 0 // error
+
+  ()

tests/pos/annot-valid.scala

@@ -1,12 +1,27 @@
 class annot[T](arg: T) extends scala.annotation.Annotation
 
-type T1 = Int @annot(42)
-type T2 = Int @annot("hello")
-type T9 = Int @annot(classOf[Int])
-type T3 = Int @annot(Array(1,2))
-type T4 = Int @annot(Array(Array(1,2),Array(3,4)))
-type T5 = Int @annot((1,2))
-type T6 = Int @annot((1,2,3))
-type T7 = Int @annot(((1,2),3))
-type T8 = Int @annot(((1,2),(3,4)))
+def main =
+  val n: Int = ???
+  def f(x: Any): Unit = ()
 
+  val x1: Int @annot(42) = 0
+  val x2: Int @annot("hello") = 0
+  val x3: Int @annot(classOf[Int]) = 0
+  val x4: Int @annot(Array(1,2)) = 0
+  val x5: Int @annot(Array(Array(1,2),Array(3,4))) = 0
+  val x6: Int @annot((1,2)) = 0
+  val x7: Int @annot((1,2,3)) = 0
+  val x8: Int @annot(((1,2),3)) = 0
+  val x9: Int @annot(((1,2),(3,4))) = 0
+
+  @annot(42) val y1: Int = 0
+  @annot("hello") val y2: Int = 0
+  @annot(classOf[Int]) val y3: Int = 0
+  @annot(Array(1,2)) val y4: Int = 0
+  @annot(Array(Array(1,2),Array(3,4))) val y5: Int = 0
+  @annot((1,2)) val y6: Int = 0
+  @annot((1,2,3)) val y7: Int = 0
+  @annot(((1,2),3)) val y8: Int = 0
+  @annot(((1,2),(3,4))) val y9: Int = 0
+
+  ()