From f2779a015ca8002c0804a7a0b2e2796be5e4483a Mon Sep 17 00:00:00 2001
From: Matt Bovel <matthieu@bovel.net>
Date: Tue, 5 Nov 2024 15:45:40 +0100
Subject: [PATCH] Fix best-effort error

Throw away erroneous trees to avoid unpickling issues in best-effort mode.

Co-Authored-By: Kacper Korban <kacper.f.korban@gmail.com>
---
 .../src/dotty/tools/dotc/typer/Checking.scala | 68 ++++++++++---------
 tests/neg/annot-invalid.scala                 |  4 +-
 2 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala
index a6e7e01ea87d..67776dfba68e 100644
--- a/compiler/src/dotty/tools/dotc/typer/Checking.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala
@@ -1421,41 +1421,43 @@ trait Checking {
           case Literal(_) => // ok
           case _ =>
             report.error(em"@${cls.name} needs a string literal as argument", arg.srcPos)
+        tree
       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?
-          ()
+        if cls.isRetainsLike then tree
         else
-          tpd.allTermArguments(tree).foreach(checkAnnotArg)
-    tree
-
-  private def checkAnnotArg(tree: Tree)(using Context): Unit =
-    def valid(t: Tree): Boolean =
-      t match
-        case _ if t.tpe.isEffectivelySingleton => true
-        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) => valid(fun) && args.forall(valid)
-        case TypeApply(fun, args) => valid(fun)
-        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
-      )
-  
+          tpd.allTermArguments(tree).foldLeft(tree: Tree)((acc: Tree, arg: Tree) =>
+            if validAnnotArg(arg) then acc
+            else errorTree(
+              EmptyTree,
+              em"""Implementation restriction: not a valid annotation argument.
+                  |Argument: $tree
+                  |Type: ${tree.tpe}""",
+              arg.srcPos
+            )
+          )
+
+  private def validAnnotArg(t: Tree)(using Context): Boolean =
+    t match
+      case _ if t.tpe.isEffectivelySingleton => true
+      case Literal(_) => true
+      // `Ident(nme.WILDCARD)` 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) => validAnnotArg(fun) && args.forall(validAnnotArg)
+      case TypeApply(fun, args) => validAnnotArg(fun)
+      case SeqLiteral(elems, _) => elems.forall(validAnnotArg)
+      case Typed(expr, _) => validAnnotArg(expr)
+      // TODO(mbovel): should probably be handled by `tpd.allTermArguments` instead.
+      case NamedArg(_, arg) => validAnnotArg(arg)
+      // TODO(mbovel): we should probably not allow `Splice` and `Hole`.
+      // How to encode them as types?
+      // When removing those cases, tests/pos-macros/i7519b.scala and
+      // tests/pos-macros/i7052.scala fail.
+      case Splice(_) => true
+      case Hole(_, _, _, _) => true
+      case _ => false
+      
   /** 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.
diff --git a/tests/neg/annot-invalid.scala b/tests/neg/annot-invalid.scala
index e17cee100873..259a197213c8 100644
--- a/tests/neg/annot-invalid.scala
+++ b/tests/neg/annot-invalid.scala
@@ -3,10 +3,10 @@ class annot[T](arg: T) extends scala.annotation.Annotation
 def main =
   val x1: Int @annot(new Object {}) = 0 // error
   val x2: Int @annot({val x = 1}) = 0 // error
-  val x4: Int @annot((x: Int) => x) = 0 // error
+  val x3: Int @annot((x: Int) => x) = 0 // error
 
   @annot(new Object {}) val y1: Int = 0 // error
   @annot({val x = 1}) val y2: Int = 0 // error
-  @annot((x: Int) => x) val y4: Int = 0  // error
+  @annot((x: Int) => x) val y3: Int = 0  // error
 
   ()