Require named arguments for java defined annotations (#21329)

Closes #20554

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -214,6 +214,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case UnusedSymbolID // errorNumber: 198
   case TailrecNestedCallID //errorNumber: 199
   case FinalLocalDefID // errorNumber: 200
+  case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3288,3 +3288,25 @@ object UnusedSymbol {
     def privateMembers(using Context): UnusedSymbol = new UnusedSymbol(i"unused private member")
     def patVars(using Context): UnusedSymbol = new UnusedSymbol(i"unused pattern variable")
 }
+
+class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamedArgumentInJavaAnnotationID):
+
+  override protected def msg(using Context): String = 
+    "Named arguments are required for Java defined annotations"
+
+  override protected def explain(using Context): String = 
+    i"""Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
+        |Java defined annotations don't have an exact constructor representation 
+        |and we previously relied on the order of the fields to create one. 
+        |One possible issue with this representation is the reordering of the fields.
+        |Lets take the following example:
+        |
+        |  public @interface Annotation {
+        |    int a() default 41;
+        |    int b() default 42;
+        |  }
+        |
+        |Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
+        """
+
+end NonNamedArgumentInJavaAnnotation

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

@@ -883,6 +883,26 @@ object Checking {
     templ.parents.find(_.tpe.derivesFrom(defn.PolyFunctionClass)) match
       case Some(parent) => report.error(s"`PolyFunction` marker trait is reserved for compiler generated refinements", parent.srcPos)
       case None =>
+
+  /** check that parameters of a java defined annotations are all named arguments if we have more than one parameter */
+  def checkNamedArgumentForJavaAnnotation(annot: untpd.Tree, sym: ClassSymbol)(using Context): untpd.Tree =
+    assert(sym.is(JavaDefined))
+
+    def annotationHasValueField: Boolean =
+      sym.info.decls.exists(_.name == nme.value)
+
+    annot match
+      case untpd.Apply(fun, List(param)) if !param.isInstanceOf[untpd.NamedArg] && annotationHasValueField =>
+        untpd.cpy.Apply(annot)(fun, List(untpd.cpy.NamedArg(param)(nme.value, param)))
+      case untpd.Apply(_, params) =>
+        for
+          param <- params
+          if !param.isInstanceOf[untpd.NamedArg]
+        do report.error(NonNamedArgumentInJavaAnnotation(), param)
+        annot
+      case _ => annot
+  end checkNamedArgumentForJavaAnnotation
+
 }
 
 trait Checking {

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

@@ -868,15 +868,20 @@ class Namer { typer: Typer =>
     protected def addAnnotations(sym: Symbol): Unit = original match {
       case original: untpd.MemberDef =>
         lazy val annotCtx = annotContext(original, sym)
-        for (annotTree <- original.mods.annotations) {
-          val cls = typedAheadAnnotationClass(annotTree)(using annotCtx)
-          if (cls eq sym)
-            report.error(em"An annotation class cannot be annotated with iself", annotTree.srcPos)
-          else {
-            val ann = Annotation.deferred(cls)(typedAheadExpr(annotTree)(using annotCtx))
-            sym.addAnnotation(ann)
-          }
-        }
+        original.setMods: 
+          original.mods.withAnnotations :
+            original.mods.annotations.mapConserve: annotTree =>  
+              val cls = typedAheadAnnotationClass(annotTree)(using annotCtx)
+              if (cls eq sym)
+                report.error(em"An annotation class cannot be annotated with iself", annotTree.srcPos)
+                annotTree
+              else
+                val ann = 
+                  if cls.is(JavaDefined) then Checking.checkNamedArgumentForJavaAnnotation(annotTree, cls.asClass) 
+                  else annotTree
+                val ann1 = Annotation.deferred(cls)(typedAheadExpr(ann)(using annotCtx))
+                sym.addAnnotation(ann1)
+                ann
       case _ =>
     }
 

tests/neg/i20554-a.check

@@ -0,0 +1,42 @@
+-- [E201] Syntax Error: tests/neg/i20554-a/Test.scala:3:12 -------------------------------------------------------------
+3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
+  |            ^
+  |            Named arguments are required for Java defined annotations
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
+  | Java defined annotations don't have an exact constructor representation 
+  | and we previously relied on the order of the fields to create one. 
+  | One possible issue with this representation is the reordering of the fields.
+  | Lets take the following example:
+  |
+  |   public @interface Annotation {
+  |     int a() default 41;
+  |     int b() default 42;
+  |   }
+  |
+  | Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
+  |         
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E201] Syntax Error: tests/neg/i20554-a/Test.scala:3:15 -------------------------------------------------------------
+3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
+  |               ^
+  |               Named arguments are required for Java defined annotations
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
+  | Java defined annotations don't have an exact constructor representation 
+  | and we previously relied on the order of the fields to create one. 
+  | One possible issue with this representation is the reordering of the fields.
+  | Lets take the following example:
+  |
+  |   public @interface Annotation {
+  |     int a() default 41;
+  |     int b() default 42;
+  |   }
+  |
+  | Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
+  |         
+   ---------------------------------------------------------------------------------------------------------------------

tests/neg/i20554-a/Annotation.java

@@ -0,0 +1,4 @@
+public @interface Annotation {
+    int a() default 41;
+    int b() default 42;
+}

tests/neg/i20554-a/Test.scala

@@ -0,0 +1,4 @@
+//> using options -explain
+
+@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
+class Test

tests/neg/i20554-b.check

@@ -0,0 +1,21 @@
+-- [E201] Syntax Error: tests/neg/i20554-b/Test.scala:3:18 -------------------------------------------------------------
+3 |@SimpleAnnotation(1) // error: the parameters is not named 'value'
+  |                  ^
+  |                  Named arguments are required for Java defined annotations
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
+  | Java defined annotations don't have an exact constructor representation 
+  | and we previously relied on the order of the fields to create one. 
+  | One possible issue with this representation is the reordering of the fields.
+  | Lets take the following example:
+  |
+  |   public @interface Annotation {
+  |     int a() default 41;
+  |     int b() default 42;
+  |   }
+  |
+  | Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
+  |         
+   ---------------------------------------------------------------------------------------------------------------------

tests/neg/i20554-b/SimpleAnnotation.java

@@ -0,0 +1,4 @@
+
+public @interface SimpleAnnotation {
+    int a() default 1;
+}

tests/neg/i20554-b/Test.scala

@@ -0,0 +1,4 @@
+//> using options -explain
+
+@SimpleAnnotation(1) // error: the parameters is not named 'value'
+class Test

tests/pos-java-interop-separate/i6868/MyScala_2.scala

@@ -1,4 +1,4 @@
-@MyJava_1("MyScala1", typeA = MyJava_1.MyClassTypeA.B)
+@MyJava_1(value = "MyScala1", typeA = MyJava_1.MyClassTypeA.B)
 object MyScala {
   def a(mj: MyJava_1): Unit = {
     println("MyJava")

tests/pos/i20554-a/Annotation.java

@@ -0,0 +1,5 @@
+public @interface Annotation {
+    int a() default 41;
+    int b() default 42;
+    int c() default 43;
+}

tests/pos/i20554-a/Test.scala

@@ -0,0 +1,3 @@
+
+@Annotation(a = 1, b = 2)
+class Test

tests/pos/i20554-b/SimpleAnnotation.java

@@ -0,0 +1,9 @@
+
+public @interface SimpleAnnotation {
+
+    int a() default 0;
+
+    int value() default 1;
+
+    int b() default 0;
+}

tests/pos/i20554-b/Test.scala

@@ -0,0 +1,3 @@
+
+@SimpleAnnotation(1)      // works because of the presence of a field called value
+class Test

tests/pos/i20554-c.scala

@@ -0,0 +1,5 @@
+
+class MyAnnotation(a: Int, b: Int) extends annotation.StaticAnnotation
+
+@MyAnnotation(1, 2) // don't require named arguments as it is Scala Defined
+class Test

tests/pos/i6151/Test.scala

@@ -1,3 +1,3 @@
 import Expect.*
-@Outcome(ExpectVal)
+@Outcome(enm = ExpectVal)
 class SimpleTest

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

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

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

@@ -1,6 +1,5 @@
 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 = _)

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

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