Move checks to class Checking, handle boundType explicitly

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

@@ -69,9 +69,6 @@ class TreeChecker extends Phase with SymTransformer {
   def transformSym(symd: SymDenotation)(using Context): SymDenotation = {
     val sym = symd.symbol
 
-    if symd.isCompleted then
-      Checking.checkWellFormedType(symd.info)
-
     if (sym.isClass && !sym.isAbsent()) {
       val validSuperclass = sym.isPrimitiveValueClass || defn.syntheticCoreClasses.contains(sym) ||
         (sym eq defn.ObjectClass) || sym.isOneOf(NoSuperClassFlags) || (sym.asClass.superClass.exists) ||
@@ -830,8 +827,17 @@ object TreeChecker {
             |${mismatch.message}${mismatch.explanation}
             |tree = $tree ${tree.className}""".stripMargin
       })
-      Checking.checkWellFormedType(tp1)
-      Checking.checkWellFormedType(tp2)
+      checkWellFormedType(tp1)
+      checkWellFormedType(tp2)
+
+    /** Check that the type `tp` is well-formed. Currently this only means
+     *  checking that annotated types have valid annotation arguments.
+     */
+    private def checkWellFormedType(tp: Type)(using Context): Unit =
+      tp.foreachPart:
+        case AnnotatedType(underlying, annot) => checkAnnot(annot.tree)
+        case _ => ()
+
   }
 
   /** Tree checker that can be applied to a local tree. */

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

@@ -604,17 +604,6 @@ 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
    */
@@ -925,73 +914,6 @@ object Checking {
         annot
       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 _ =>
-        if cls.isRetainsLike then () // Do not check @retain annotations
-        else if cls == defn.ThrowsAnnot then
-          // Do not check @throws annotations.
-          // TODO(mbovel): in tests/run/t6380.scala, an annotation tree is
-          // `new throws[Exception](throws.<init>[Exception])`. What is this?
-          ()
-        else
-          tpd.allTermArguments(tree).foreach(checkAnnotArg)
-    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
-          || qual.symbol == defn.ClassTagModule // class tags are used as arguments to Array.apply
-          || qual.symbol == defn.SymbolModule // used in Akka
-          || qual.symbol == defn.JSSymbolModule // used in Scala.js
-          || isTupleModule(qual.symbol)
-        case Select(New(clazz), nme.CONSTRUCTOR) => clazz.symbol.isAnnotation
-        case Apply(fun, _) => isFunctionAllowed(fun)
-        case TypeApply(fun, _) => isFunctionAllowed(fun)
-        case _ => false
-
-    def valid(t: Tree): Boolean =
-      t.tpe.isEffectivelySingleton
-      || (
-        t match
-          case Literal(_) => true
-          // `_` is used as placeholder for unspecified arguments of Java
-          // annotations. Example: tests/run/java-ann-super-class
-          case Ident(nme.WILDCARD) => true
-          case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(valid)
-          case TypeApply(fun, args) => isFunctionAllowed(fun)
-          // Support for `x.isInstanceOf[T]`. Probably not needed.
-          //case TypeApply(meth @ Select(arg, _), _) if meth.symbol == defn.Any_asInstanceOf => valid(arg)
-          case SeqLiteral(elems, _) => elems.forall(valid)
-          case Typed(expr, _) => valid(expr)
-          case NamedArg(_, arg) => valid(arg)
-          case Splice(_) => true
-          case Hole(_, _, _, _) => true
-          case _ => false
-      )
-
-    if !valid(tree) then
-      report.error(
-        i"""Implementation restriction: not a valid annotation argument.
-           |  Argument: $tree
-           |  Type: ${tree.tpe}""",
-        tree.srcPos
-      )
 }
 
 trait Checking {
@@ -1463,7 +1385,13 @@ trait Checking {
       report.error(em"$what can only be used in an inline method", pos)
 
   def checkAnnot(tree: Tree)(using Context): Tree =
-    Checking.checkAnnotArgs(checkAnnotClass(tree))
+    tree match
+      case Ident(tpnme.BOUNDTYPE_ANNOT) => 
+        // `FirstTransform.toTypeTree` creates `Annotated` nodes whose `annot` are
+        // `Ident`s, not annotation instances. See `tests/pos/annot-boundtype.scala`.
+        tree
+      case _ => 
+        checkAnnotArgs(checkAnnotClass(tree))
 
   /** Check that the class corresponding to this tree is either a Scala or Java annotation.
    *
@@ -1481,6 +1409,73 @@ 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 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 _ =>
+        if cls.isRetainsLike then () // Do not check @retain annotations
+        else if cls == defn.ThrowsAnnot then
+          // Do not check @throws annotations.
+          // TODO(mbovel): in tests/run/t6380.scala, an annotation tree is
+          // `new throws[Exception](throws.<init>[Exception])`. What is this?
+          ()
+        else
+          tpd.allTermArguments(tree).foreach(checkAnnotArg)
+    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
+          || qual.symbol == defn.ClassTagModule // class tags are used as arguments to Array.apply
+          || qual.symbol == defn.SymbolModule // used in Akka
+          || qual.symbol == defn.JSSymbolModule // used in Scala.js
+          || isTupleModule(qual.symbol)
+        case Select(New(clazz), nme.CONSTRUCTOR) => clazz.symbol.isAnnotation
+        case Apply(fun, _) => isFunctionAllowed(fun)
+        case TypeApply(fun, _) => isFunctionAllowed(fun)
+        case _ => false
+
+    def valid(t: Tree): Boolean =
+      t.tpe.isEffectivelySingleton
+      || (
+        t match
+          case Literal(_) => true
+          // `_` is used as placeholder for unspecified arguments of Java
+          // annotations. Example: tests/run/java-ann-super-class
+          case Ident(nme.WILDCARD) => true
+          case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(valid)
+          case TypeApply(fun, args) => isFunctionAllowed(fun)
+          // Support for `x.isInstanceOf[T]`. Probably not needed.
+          //case TypeApply(meth @ Select(arg, _), _) if meth.symbol == defn.Any_asInstanceOf => valid(arg)
+          case SeqLiteral(elems, _) => elems.forall(valid)
+          case Typed(expr, _) => valid(expr)
+          case NamedArg(_, arg) => valid(arg)
+          case Splice(_) => true
+          case Hole(_, _, _, _) => true
+          case _ => false
+      )
+
+    if !valid(tree) then
+      report.error(
+        i"""Implementation restriction: not a valid annotation argument.
+           |  Argument: $tree
+           |  Type: ${tree.tpe}""",
+        tree.srcPos
+      )
 
   /** 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.

tests/neg/annot-invalid.check

@@ -0,0 +1,54 @@
+-- Error: tests/neg/annot-invalid.scala:7:21 ---------------------------------------------------------------------------
+7 |  val x1: Int @annot(n + 1) = 0 // error
+  |                     ^^^^^
+  |                     Implementation restriction: not a valid annotation argument.
+  |                       Argument: n.+(1)
+  |                       Type: Int
+-- Error: tests/neg/annot-invalid.scala:8:22 ---------------------------------------------------------------------------
+8 |  val x2: Int @annot(f(2)) = 0 // error
+  |                     ^^^^
+  |                     Implementation restriction: not a valid annotation argument.
+  |                       Argument: f(2)
+  |                       Type: Unit
+-- Error: tests/neg/annot-invalid.scala:9:21 ---------------------------------------------------------------------------
+9 |  val x3: Int @annot(throw new Error()) = 0 // error
+  |                     ^^^^^^^^^^^^^^^^^
+  |                     Implementation restriction: not a valid annotation argument.
+  |                       Argument: throw new Error()
+  |                       Type: Nothing
+-- Error: tests/neg/annot-invalid.scala:10:21 --------------------------------------------------------------------------
+10 |  val x4: Int @annot((x: Int) => x) = 0 // error
+   |                     ^^^^^^^^^^^^^
+   |                     Implementation restriction: not a valid annotation argument.
+   |                       Argument: {
+   |                       def $anonfun(x: Int): Int = x
+   |                       closure($anonfun)
+   |                     }
+   |                       Type: Int => Int
+-- Error: tests/neg/annot-invalid.scala:12:9 ---------------------------------------------------------------------------
+12 |  @annot(n + 1) val y1: Int = 0 // error
+   |         ^^^^^
+   |         Implementation restriction: not a valid annotation argument.
+   |           Argument: n.+(1)
+   |           Type: Int
+-- Error: tests/neg/annot-invalid.scala:13:10 --------------------------------------------------------------------------
+13 |  @annot(f(2)) val y2: Int = 0 // error
+   |         ^^^^
+   |         Implementation restriction: not a valid annotation argument.
+   |           Argument: f(2)
+   |           Type: Unit
+-- Error: tests/neg/annot-invalid.scala:14:9 ---------------------------------------------------------------------------
+14 |  @annot(throw new Error()) val y3: Int = 0 // error
+   |         ^^^^^^^^^^^^^^^^^
+   |         Implementation restriction: not a valid annotation argument.
+   |           Argument: throw new Error()
+   |           Type: Nothing
+-- Error: tests/neg/annot-invalid.scala:15:9 ---------------------------------------------------------------------------
+15 |  @annot((x: Int) => x) val y4: Int = 0  // error
+   |         ^^^^^^^^^^^^^
+   |         Implementation restriction: not a valid annotation argument.
+   |           Argument: {
+   |           def $anonfun(x: Int): Int = x
+   |           closure($anonfun)
+   |         }
+   |           Type: Int => Int

tests/neg/annot-invalid.scala

@@ -9,9 +9,9 @@ def main =
   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
+  @annot(n + 1) val y1: Int = 0 // error
+  @annot(f(2)) val y2: Int = 0 // error
+  @annot(throw new Error()) val y3: Int = 0 // error
+  @annot((x: Int) => x) val y4: Int = 0  // error
 
   ()

tests/pos/annot-boundtype.scala

@@ -0,0 +1,16 @@
+// `FirstTransform.toTypeTree` creates `Annotated` nodes whose `annot` are
+// `Ident`s, not annotation instances. This is relevant for `Checking.checkAnnot`.
+//
+// See also:
+// - tests/run/t2755.scala
+// - tests/neg/i13044.scala
+
+def f(a: Array[?]) =
+  a match
+    case x: Array[?] => ()
+
+def f2(t: Tuple) =
+  t match
+    case _: (t *: ts) => ()
+    case _ => ()
+