Skip to content

Commit c0b6b0f

Browse files
hamzaremmalhamzaremmal
committed
Require named arguments for java defined annotations
1 parent fd45847 commit c0b6b0f

File tree

15 files changed

+161
-9
lines changed

15 files changed

+161
-9
lines changed

Diff for: ‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

+1
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
214214
case UnusedSymbolID // errorNumber: 198
215215
case TailrecNestedCallID //errorNumber: 199
216216
case FinalLocalDefID // errorNumber: 200
217+
case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
217218

218219
def errorNumber = ordinal - 1
219220

Diff for: ‎compiler/src/dotty/tools/dotc/reporting/messages.scala

+22
Original file line numberDiff line numberDiff line change
@@ -3288,3 +3288,25 @@ object UnusedSymbol {
32883288
def privateMembers(using Context): UnusedSymbol = new UnusedSymbol(i"unused private member")
32893289
def patVars(using Context): UnusedSymbol = new UnusedSymbol(i"unused pattern variable")
32903290
}
3291+
3292+
class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamedArgumentInJavaAnnotationID):
3293+
3294+
override protected def msg(using Context): String =
3295+
"Named arguments are required for Java defined annotations"
3296+
3297+
override protected def explain(using Context): String =
3298+
i"""Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
3299+
|Java defined annotations don't have an exact constructor representation
3300+
|and we previously relied on the order of the fields to create one.
3301+
|One possible issue with this representation is the reordering of the fields.
3302+
|Lets take the following example:
3303+
|
3304+
| public @interface Annotation {
3305+
| int a() default 41;
3306+
| int b() default 42;
3307+
| }
3308+
|
3309+
|Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
3310+
"""
3311+
3312+
end NonNamedArgumentInJavaAnnotation

Diff for: ‎compiler/src/dotty/tools/dotc/typer/Checking.scala

+20
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,26 @@ object Checking {
883883
templ.parents.find(_.tpe.derivesFrom(defn.PolyFunctionClass)) match
884884
case Some(parent) => report.error(s"`PolyFunction` marker trait is reserved for compiler generated refinements", parent.srcPos)
885885
case None =>
886+
887+
/** check that parameters of a java defined annotations are all named arguments if we have more than one parameter */
888+
def checkNamedArgumentForJavaAnnotation(annot: untpd.Tree, sym: ClassSymbol)(using Context): untpd.Tree =
889+
assert(sym.is(JavaDefined))
890+
891+
def annotationHasValueField: Boolean =
892+
sym.info.decls.exists(_.name == nme.value)
893+
894+
annot match
895+
case untpd.Apply(fun, List(param)) if !param.isInstanceOf[untpd.NamedArg] && annotationHasValueField =>
896+
untpd.cpy.Apply(annot)(fun, List(untpd.cpy.NamedArg(param)(nme.value, param)))
897+
case untpd.Apply(_, params) =>
898+
for
899+
param <- params
900+
if !param.isInstanceOf[untpd.NamedArg]
901+
do report.error(NonNamedArgumentInJavaAnnotation(), param)
902+
annot
903+
case _ => annot
904+
end checkNamedArgumentForJavaAnnotation
905+
886906
}
887907

888908
trait Checking {

Diff for: ‎compiler/src/dotty/tools/dotc/typer/Namer.scala

+14-9
Original file line numberDiff line numberDiff line change
@@ -868,15 +868,20 @@ class Namer { typer: Typer =>
868868
protected def addAnnotations(sym: Symbol): Unit = original match {
869869
case original: untpd.MemberDef =>
870870
lazy val annotCtx = annotContext(original, sym)
871-
for (annotTree <- original.mods.annotations) {
872-
val cls = typedAheadAnnotationClass(annotTree)(using annotCtx)
873-
if (cls eq sym)
874-
report.error(em"An annotation class cannot be annotated with iself", annotTree.srcPos)
875-
else {
876-
val ann = Annotation.deferred(cls)(typedAheadExpr(annotTree)(using annotCtx))
877-
sym.addAnnotation(ann)
878-
}
879-
}
871+
original.setMods:
872+
original.mods.withAnnotations :
873+
original.mods.annotations.mapConserve: annotTree =>
874+
val cls = typedAheadAnnotationClass(annotTree)(using annotCtx)
875+
if (cls eq sym)
876+
report.error(em"An annotation class cannot be annotated with iself", annotTree.srcPos)
877+
annotTree
878+
else
879+
val ann =
880+
if cls.is(JavaDefined) then Checking.checkNamedArgumentForJavaAnnotation(annotTree, cls.asClass)
881+
else annotTree
882+
val ann1 = Annotation.deferred(cls)(typedAheadExpr(ann)(using annotCtx))
883+
sym.addAnnotation(ann1)
884+
ann
880885
case _ =>
881886
}
882887

Diff for: ‎tests/neg/i20554-a.check

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
-- [E201] Syntax Error: tests/neg/i20554-a/Test.scala:3:12 -------------------------------------------------------------
2+
3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
3+
| ^
4+
| Named arguments are required for Java defined annotations
5+
|---------------------------------------------------------------------------------------------------------------------
6+
| Explanation (enabled by `-explain`)
7+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
8+
| Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
9+
| Java defined annotations don't have an exact constructor representation
10+
| and we previously relied on the order of the fields to create one.
11+
| One possible issue with this representation is the reordering of the fields.
12+
| Lets take the following example:
13+
|
14+
| public @interface Annotation {
15+
| int a() default 41;
16+
| int b() default 42;
17+
| }
18+
|
19+
| Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
20+
|
21+
---------------------------------------------------------------------------------------------------------------------
22+
-- [E201] Syntax Error: tests/neg/i20554-a/Test.scala:3:15 -------------------------------------------------------------
23+
3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
24+
| ^
25+
| Named arguments are required for Java defined annotations
26+
|---------------------------------------------------------------------------------------------------------------------
27+
| Explanation (enabled by `-explain`)
28+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
29+
| Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
30+
| Java defined annotations don't have an exact constructor representation
31+
| and we previously relied on the order of the fields to create one.
32+
| One possible issue with this representation is the reordering of the fields.
33+
| Lets take the following example:
34+
|
35+
| public @interface Annotation {
36+
| int a() default 41;
37+
| int b() default 42;
38+
| }
39+
|
40+
| Reordering the fields is binary-compatible but it might affect the meaning of @Annotation(1)
41+
|
42+
---------------------------------------------------------------------------------------------------------------------

Diff for: ‎tests/neg/i20554-a/Annotation.java

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
public @interface Annotation {
2+
int a() default 41;
3+
int b() default 42;
4+
}

Diff for: ‎tests/neg/i20554-a/Test.scala

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

Diff for: ‎tests/neg/i20554-b.check

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

Diff for: ‎tests/neg/i20554-b/SimpleAnnotation.java

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
public @interface SimpleAnnotation {
3+
int a() default 1;
4+
}

Diff for: ‎tests/neg/i20554-b/Test.scala

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
//> using options -explain
2+
3+
@SimpleAnnotation(1) // error: the parameters is not named 'value'
4+
class Test

Diff for: ‎tests/pos/i20554-a/Annotation.java

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
public @interface Annotation {
2+
int a() default 41;
3+
int b() default 42;
4+
int c() default 43;
5+
}

Diff for: ‎tests/pos/i20554-a/Test.scala

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
@Annotation(a = 1, b = 2)
3+
class Test

Diff for: ‎tests/pos/i20554-b/SimpleAnnotation.java

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
public @interface SimpleAnnotation {
3+
4+
int a() default 0;
5+
6+
int value() default 1;
7+
8+
int b() default 0;
9+
}

Diff for: ‎tests/pos/i20554-b/Test.scala

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
@SimpleAnnotation(1) // works because of the presence of a field called value
3+
class Test

Diff for: ‎tests/pos/i20554-c.scala

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

0 commit comments

Comments
 (0)