Skip to content

Commit

Permalink
Require named arguments for java defined annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
hamzaremmal committed Aug 6, 2024
1 parent 6c4eace commit a36af49
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 22 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
28 changes: 28 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,34 @@ 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)

def hasOnePositionalParameter(params: List[untpd.Tree]): Boolean =
params.exists(!_.isInstanceOf[untpd.NamedArg])

def mapPositionalToNamed(param: untpd.Tree): untpd.Tree = param match
case tree: untpd.NamedArg => tree
case tree => untpd.cpy.NamedArg(param)(nme.value, param)
end mapPositionalToNamed

annot match
case untpd.Apply(fun, params) if hasOnePositionalParameter(params) && annotationHasValueField =>
untpd.cpy.Apply(annot)(fun, params.mapConserve(mapPositionalToNamed))
case untpd.Apply(_, params) if hasOnePositionalParameter(params) =>
for
param <- params
if !param.isInstanceOf[untpd.NamedArg]
do report.error(NonNamedArgumentInJavaAnnotation(), param)
annot
case _ => annot
end checkNamedArgumentForJavaAnnotation

}

trait Checking {
Expand Down
23 changes: 14 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 _ =>
}

Expand Down
42 changes: 42 additions & 0 deletions tests/neg/i20554-a.check
Original file line number Diff line number Diff line change
@@ -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)
|
---------------------------------------------------------------------------------------------------------------------
4 changes: 4 additions & 0 deletions tests/neg/i20554-a/Annotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public @interface Annotation {
int a() default 41;
int b() default 42;
}
4 changes: 4 additions & 0 deletions tests/neg/i20554-a/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
//> using options -explain

@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
class Test
21 changes: 21 additions & 0 deletions tests/neg/i20554-b.check
Original file line number Diff line number Diff line change
@@ -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)
|
---------------------------------------------------------------------------------------------------------------------
4 changes: 4 additions & 0 deletions tests/neg/i20554-b/SimpleAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

public @interface SimpleAnnotation {
int a() default 1;
}
4 changes: 4 additions & 0 deletions tests/neg/i20554-b/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
//> using options -explain

@SimpleAnnotation(1) // error: the parameters is not named 'value'
class Test
5 changes: 5 additions & 0 deletions tests/pos/i20554-a/Annotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
public @interface Annotation {
int a() default 41;
int b() default 42;
int c() default 43;
}
3 changes: 3 additions & 0 deletions tests/pos/i20554-a/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

@Annotation(a = 1, b = 2)
class Test
9 changes: 9 additions & 0 deletions tests/pos/i20554-b/SimpleAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

public @interface SimpleAnnotation {

int a() default 0;

int value() default 1;

int b() default 0;
}
3 changes: 3 additions & 0 deletions tests/pos/i20554-b/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

@SimpleAnnotation(1) // works because of the presence of a field called value
class Test
5 changes: 5 additions & 0 deletions tests/pos/i20554-c.scala
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a36af49

Please sign in to comment.