Make sure all arguments to Java annot constructors are NamedArg's.

The Java model of annotations is unordered and name-based. Even though we typecheck things from source with a particular ordering, semantically we must always use `NamedArg`s to match the Java model.
scala · Jun 10, 2024 · bd95888 · bd95888
1 parent 27a3f80
commit bd95888
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/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()
 

diff --git a/tests/run-macros/annot-arg-value-in-java.check b/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)
diff --git a/tests/run-macros/annot-java-tree/AnnoMacro.scala b/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