Fix #19951: Align TASTy with the Java annotation model.

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

@@ -466,7 +466,10 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
               }
             case _ =>
               if passesConditionForErroringBestEffortCode(tree.hasType) then
-                val sig = tree.tpe.signature
+                // #19951 The signature of a constructor of a Java annotation is irrelevant
+                val sig =
+                  if name == nme.CONSTRUCTOR && tree.symbol.exists && tree.symbol.owner.is(JavaAnnotation) then Signature.NotAMethod
+                  else tree.tpe.signature
                 var ename = tree.symbol.targetName
                 val selectFromQualifier =
                   name.isTypeName
@@ -507,7 +510,14 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
             writeByte(APPLY)
             withLength {
               pickleTree(fun)
-              args.foreach(pickleTree)
+              // #19951 Do not pickle default arguments to Java annotation constructors
+              if fun.symbol.isClassConstructor && fun.symbol.owner.is(JavaAnnotation) then
+                for arg <- args do
+                  arg match
+                    case NamedArg(_, Ident(nme.WILDCARD)) => ()
+                    case _                                => pickleTree(arg)
+              else
+                args.foreach(pickleTree)
             }
           }
         case TypeApply(fun, args) =>

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -1281,7 +1281,14 @@ class TreeUnpickler(reader: TastyReader,
           if unpicklingJava && name == tpnme.Object && qual.symbol == defn.JavaLangPackageVal then
             defn.FromJavaObjectSymbol.denot
           else
-            accessibleDenot(qual.tpe.widenIfUnstable, name, sig, target)
+            val qualType = qual.tpe.widenIfUnstable
+            if name == nme.CONSTRUCTOR && qualType.classSymbol.is(JavaAnnotation) then
+              // #19951 Disregard the signature (or the absence thereof) for constructors of Java annotations
+              // Note that Java annotations always have a single public constructor
+              // They may have a PrivateLocal constructor if compiled from source in mixed compilation
+              qualType.findMember(name, qualType, excluded = Private)
+            else
+              accessibleDenot(qualType, name, sig, target)
         makeSelect(qual, name, denot)
 
       def readQualId(): (untpd.Ident, TypeRef) =
@@ -1335,15 +1342,26 @@ class TreeUnpickler(reader: TastyReader,
           readPathTree()
       }
 
-      /** Adapt constructor calls where class has only using clauses from old to new scheme.
+      /** Adapt constructor calls for Java annot constructors and for the new scheme of `using` clauses.
+       *
+       *  #19951 If the `fn` is the constructor of a Java annotation, reorder and refill
+       *  arguments against the constructor signature. Only reorder if all the arguments
+       *  are `NamedArg`s, which is always the case if the TASTy was produced by 3.5+.
+       *  If some arguments are positional, only *add* missing arguments to the right
+       *  and hope for the best; this will at least fix #19951 after the fact if the new
+       *  annotation fields are added after all the existing ones.
+       *
+       *  Otherwise, adapt calls where class has only using clauses from old to new scheme.
        *  or class has mixed using clauses and other clauses.
        *  Old: leading (), new: nothing, or trailing () if all clauses are using clauses.
        *  This is neccessary so that we can read pre-3.2 Tasty correctly. There,
        *  constructor calls use the old scheme, but constructor definitions already
        *  use the new scheme, since they are reconstituted with normalizeIfConstructor.
        */
       def constructorApply(fn: Tree, args: List[Tree]): Tree =
-        if fn.tpe.widen.isContextualMethod && args.isEmpty then
+        if fn.symbol.owner.is(JavaAnnotation) then
+          tpd.Apply(fn, fixArgsToJavaAnnotConstructor(fn.tpe.widen, args))
+        else if fn.tpe.widen.isContextualMethod && args.isEmpty then
           fn.withAttachment(SuppressedApplyToNone, ())
         else
           val fn1 = fn match
@@ -1365,6 +1383,68 @@ class TreeUnpickler(reader: TastyReader,
                 res.withAttachment(SuppressedApplyToNone, ())
               else res
 
+      def fixArgsToJavaAnnotConstructor(methType: Type, args: List[Tree]): List[Tree] =
+        methType match
+          case methType: MethodType =>
+            val formalNames = methType.paramNames
+            val sizeCmp = args.sizeCompare(formalNames)
+
+            def makeDefault(name: TermName, tpe: Type): NamedArg =
+              NamedArg(name, Underscore(tpe))
+
+            def extendOnly(args: List[NamedArg]): List[NamedArg] =
+              if sizeCmp < 0 then
+                val argsSize = args.size
+                val additionalArgs: List[NamedArg] =
+                  formalNames.drop(argsSize).lazyZip(methType.paramInfos.drop(argsSize)).map(makeDefault(_, _))
+                args ::: additionalArgs
+              else
+                args // fast path
+
+            if formalNames.isEmpty then
+              // fast path
+              args
+            else if sizeCmp > 0 then
+              // Something's wrong anyway; don't touch anything
+              args
+            else if args.exists(!_.isInstanceOf[NamedArg]) then
+              // Pre 3.5 TASTy -- do our best, assuming that args match as a prefix of the formals
+              val prefixMatch = args.lazyZip(formalNames).forall {
+                case (NamedArg(actualName, _), formalName) => actualName == formalName
+                case _                                     => true
+              }
+              // If the prefix does not match, something's wrong; don't touch anything
+              if !prefixMatch then
+                args
+              else
+                // Turn non-named args to named and extend with defaults
+                extendOnly(args.lazyZip(formalNames).map {
+                  case (arg: NamedArg, _) => arg
+                  case (arg, formalName)  => NamedArg(formalName, arg)
+                })
+            else
+              // Good TASTy where all the arguments are named; reorder and extend if needed
+              val namedArgs = args.asInstanceOf[List[NamedArg]]
+              val prefixMatch = namedArgs.lazyZip(formalNames).forall((arg, formalName) => arg.name == formalName)
+              if prefixMatch then
+                // fast path, extend only
+                extendOnly(namedArgs)
+              else
+                // needs reordering, and possibly fill in holes for default arguments
+                val argsByName = mutable.AnyRefMap.from(namedArgs.map(arg => arg.name -> arg))
+                val reconstructedArgs = formalNames.lazyZip(methType.paramInfos).map { (name, tpe) =>
+                  argsByName.remove(name).getOrElse(makeDefault(name, tpe))
+                }
+                if argsByName.nonEmpty then
+                  // something's wrong; don't touch anything
+                  args
+                else
+                  reconstructedArgs
+
+          case _ =>
+            args
+      end fixArgsToJavaAnnotConstructor
+
       def quotedExpr(fn: Tree, args: List[Tree]): Tree =
         val TypeApply(_, targs) = fn: @unchecked
         untpd.Quote(args.head, Nil).withBodyType(targs.head.tpe)
@@ -1491,8 +1571,12 @@ class TreeUnpickler(reader: TastyReader,
                   NoDenotation
 
               val denot =
-                val d = ownerTpe.decl(name).atSignature(sig, target)
-                (if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
+                if owner.is(JavaAnnotation) && name == nme.CONSTRUCTOR then
+                  // #19951 Fix up to read TASTy produced before 3.5.0 -- ignore the signature
+                  ownerTpe.nonPrivateDecl(name).asSeenFrom(prefix)
+                else
+                  val d = ownerTpe.decl(name).atSignature(sig, target)
+                  (if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
 
               makeSelect(qual, name, denot)
             case REPEATED =>

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

@@ -945,12 +945,17 @@ trait Applications extends Compatibility {
       val app1 =
         if (!success || typedArgs.exists(_.tpe.isError)) app0.withType(UnspecifiedErrorType)
         else {
-          if !sameSeq(args, orderedArgs)
-             && !isJavaAnnotConstr(methRef.symbol)
-             && !typedArgs.forall(isSafeArg)
-          then
+          if isJavaAnnotConstr(methRef.symbol) then
+            // #19951 Make sure all arguments are NamedArgs for Java annotations
+            if typedArgs.exists(!_.isInstanceOf[NamedArg]) then
+              typedArgs = typedArgs.lazyZip(methType.asInstanceOf[MethodType].paramNames).map {
+                case (arg: NamedArg, _) => arg
+                case (arg, name)        => NamedArg(name, arg)
+              }
+          else if !sameSeq(args, orderedArgs) && !typedArgs.forall(isSafeArg) then
             // need to lift arguments to maintain evaluation order in the
             // presence of argument reorderings.
+            // (never do this for Java annotation constructors, hence the 'else if')
 
             liftFun()
 

sbt-test/scala3-compat/java-annotations-3.4/app/Main.scala

@@ -0,0 +1,21 @@
+object Test:
+  def main(args: Array[String]): Unit =
+    val actual = listAnnots("ScalaUser")
+    val expected = List(
+      "new JavaAnnot(a = 5, b = _, c = _)",
+      "new JavaAnnot(a = 5, b = _, c = _)",
+      "new JavaAnnot(a = 5, b = \"foo\", c = _)",
+      "new JavaAnnot(a = 5, b = \"foo\", c = 3)",
+      "new JavaAnnot(a = 5, b = _, c = 3)",
+      "new JavaAnnot(a = 5, b = \"foo\", c = 3)",
+      "new JavaAnnot(a = 5, b = \"foo\", c = 3)",
+      "new JavaAnnot(a = 5, b = \"foo\", c = _)",
+    )
+    if actual != expected then
+      println("Expected:")
+      expected.foreach(println(_))
+      println("Actual:")
+      actual.foreach(println(_))
+      throw new AssertionError("test failed")
+  end main
+end Test

sbt-test/scala3-compat/java-annotations-3.4/build.sbt

@@ -0,0 +1,7 @@
+lazy val lib = project.in(file("lib"))
+  .settings(
+    scalaVersion := "3.4.2"
+  )
+
+lazy val app = project.in(file("app"))
+  .dependsOn(lib)

sbt-test/scala3-compat/java-annotations-3.4/lib/AnnotMacro.scala

@@ -0,0 +1,7 @@
+import scala.quoted.*
+
+inline def listAnnots(inline c: String): List[String] = ${ listAnnotsImpl('c) }
+
+def listAnnotsImpl(c: Expr[String])(using Quotes): Expr[List[String]] =
+  import quotes.reflect.*
+  Expr(Symbol.requiredClass(c.valueOrError).declaredMethods.flatMap(_.annotations.map(_.show)))

sbt-test/scala3-compat/java-annotations-3.4/lib/JavaAnnot.java

@@ -0,0 +1,10 @@
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+@interface JavaAnnot {
+  int a();
+  String b() default "empty";
+  int c() default 5;
+}

sbt-test/scala3-compat/java-annotations-3.4/lib/ScalaUser.scala

@@ -0,0 +1,25 @@
+class ScalaUser {
+  @JavaAnnot(5)
+  def f1(): Int = 1
+
+  @JavaAnnot(a = 5)
+  def f2(): Int = 1
+
+  @JavaAnnot(5, "foo")
+  def f3(): Int = 1
+
+  @JavaAnnot(5, "foo", 3)
+  def f4(): Int = 1
+
+  @JavaAnnot(5, c = 3)
+  def f5(): Int = 1
+
+  @JavaAnnot(5, c = 3, b = "foo")
+  def f6(): Int = 1
+
+  @JavaAnnot(b = "foo", c = 3, a = 5)
+  def f7(): Int = 1
+
+  @JavaAnnot(b = "foo", a = 5)
+  def f8(): Int = 1
+}

sbt-test/scala3-compat/java-annotations-3.4/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion")
+  )
+}

sbt-test/scala3-compat/java-annotations-3.4/test

@@ -0,0 +1 @@
+> app/run

tests/run-macros/annot-arg-value-in-java.check

@@ -3,9 +3,9 @@ new java.lang.SuppressWarnings(value = "a")
 new java.lang.SuppressWarnings(value = "b")
 new java.lang.SuppressWarnings(value = _root_.scala.Array.apply[java.lang.String]("c", "d")(scala.reflect.ClassTag.apply[java.lang.String](classOf[java.lang.String])))
 JOtherTypes:
-new Annot(value = 1, _, _)
-new Annot(value = -2, _, _)
-new Annot(_, m = false, _)
-new Annot(_, m = true, _)
-new Annot(_, _, n = 1.1)
-new Annot(_, _, n = -2.1)
+new Annot(value = 1, m = _, n = _)
+new Annot(value = -2, m = _, n = _)
+new Annot(value = _, m = false, n = _)
+new Annot(value = _, m = true, n = _)
+new Annot(value = _, m = _, n = 1.1)
+new Annot(value = _, m = _, n = -2.1)

tests/run-macros/annot-java-tree/AnnoMacro.scala

@@ -8,12 +8,17 @@ def checkSuppressWarningsImpl[T: Type](using Quotes): Expr[Unit] =
   val sym = TypeRepr.of[T].typeSymbol
   // Imitate what wartremover does, so we can avoid unintentionally breaking it:
   // https://github.com/wartremover/wartremover/blob/fb18e6eafe9a47823e04960aaf4ec7a9293719ef/core/src/main/scala-3/org/wartremover/WartUniverse.scala#L63-L77
+  // We're intentionally breaking it in 3.5.x, though, with the addition of `NamedArg("value", ...)`
+  // The previous implementation would be broken for cases where the user explicitly write `value = ...` anyway.
   val actualArgs = sym
     .getAnnotation(SuppressWarningsSymbol)
     .collect {
       case Apply(
             Select(_, "<init>"),
-            Apply(Apply(_, Typed(Repeated(values, _), _) :: Nil), Apply(_, _ :: Nil) :: Nil) :: Nil
+            NamedArg(
+              "value",
+              Apply(Apply(_, Typed(Repeated(values, _), _) :: Nil), Apply(_, _ :: Nil) :: Nil)
+            ) :: Nil
           ) =>
         // "-Yexplicit-nulls"
         // https://github.com/wartremover/wartremover/issues/660

tests/run-macros/i19951-java-annotations-tasty-compat-2/AnnotMacro_1.scala

@@ -0,0 +1,11 @@
+import scala.quoted.*
+
+inline def showAnnots(inline c: String): Unit = ${ showAnnotsImpl('c) }
+
+def showAnnotsImpl(c: Expr[String])(using Quotes): Expr[Unit] =
+  import quotes.reflect.*
+  val al = Expr(Symbol.requiredClass(c.valueOrError).declaredMethods.flatMap(_.annotations.map(_.show)))
+  '{
+    println($c + ":")
+    $al.foreach(println)
+  }

tests/run-macros/i19951-java-annotations-tasty-compat-2/JavaAnnot_1.java

@@ -0,0 +1,10 @@
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+@interface JavaAnnot {
+  int a();
+  String b() default "empty";
+  int c() default 5;
+}

tests/run-macros/i19951-java-annotations-tasty-compat-2/JavaAnnot_2.java

@@ -0,0 +1,11 @@
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+@interface JavaAnnot {
+  int c() default 5;
+  int a();
+  int d() default 42;
+  String b() default "empty";
+}

tests/run-macros/i19951-java-annotations-tasty-compat-2/ScalaUser_1.scala

@@ -0,0 +1,25 @@
+class ScalaUser {
+  @JavaAnnot(5)
+  def f1(): Int = 1
+
+  @JavaAnnot(a = 5)
+  def f2(): Int = 1
+
+  @JavaAnnot(5, "foo")
+  def f3(): Int = 1
+
+  @JavaAnnot(5, "foo", 3)
+  def f4(): Int = 1
+
+  @JavaAnnot(5, c = 3)
+  def f5(): Int = 1
+
+  @JavaAnnot(5, c = 3, b = "foo")
+  def f6(): Int = 1
+
+  @JavaAnnot(b = "foo", c = 3, a = 5)
+  def f7(): Int = 1
+
+  @JavaAnnot(b = "foo", a = 5)
+  def f8(): Int = 1
+}

tests/run-macros/i19951-java-annotations-tasty-compat-2/Test_2.scala

@@ -0,0 +1,4 @@
+object Test {
+  def main(args: Array[String]): Unit =
+    showAnnots("ScalaUser")
+}

tests/run-macros/i19951-java-annotations-tasty-compat.check

@@ -0,0 +1,9 @@
+ScalaUser:
+new JavaAnnot(c = _, a = 5, d = _, b = _)
+new JavaAnnot(c = _, a = 5, d = _, b = _)
+new JavaAnnot(c = _, a = 5, d = _, b = "foo")
+new JavaAnnot(c = 3, a = 5, d = _, b = "foo")
+new JavaAnnot(c = 3, a = 5, d = _, b = _)
+new JavaAnnot(c = 3, a = 5, d = _, b = "foo")
+new JavaAnnot(c = 3, a = 5, d = _, b = "foo")
+new JavaAnnot(c = _, a = 5, d = _, b = "foo")

tests/run-macros/i19951-java-annotations-tasty-compat/AnnotMacro_2.scala

@@ -0,0 +1,11 @@
+import scala.quoted.*
+
+inline def showAnnots(inline c: String): Unit = ${ showAnnotsImpl('c) }
+
+def showAnnotsImpl(c: Expr[String])(using Quotes): Expr[Unit] =
+  import quotes.reflect.*
+  val al = Expr(Symbol.requiredClass(c.valueOrError).declaredMethods.flatMap(_.annotations.map(_.show)))
+  '{
+    println($c + ":")
+    $al.foreach(println)
+  }