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 5, 2024
1 parent 6c4eace commit f915648
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 0 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
11 changes: 11 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,17 @@ trait Checking {
// TODO: Add more checks here
}

/** check that parameters of a java defined annotations are all named arguments if we have more than one parameter */
def checkNamedArgumentForJavaAnnotation(annot: untpd.Tree)(using Context): Unit =
assert(annot.symbol.is(JavaDefined))
annot match
case untpd.Apply(_, params) if params.length > 1 =>
for param <- params
if !param.isInstanceOf[untpd.NamedArg]
do report.error(NonNamedArgumentInJavaAnnotation(), param)
case _ =>
end checkNamedArgumentForJavaAnnotation

/** Check that symbol's external name does not clash with symbols defined in the same scope */
def checkNoTargetNameConflict(stats: List[Tree])(using Context): Unit =
var seen = Set[Name]()
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2737,6 +2737,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
lazy val annotCtx = annotContext(mdef, sym)
// necessary in order to mark the typed ahead annotations as definitely typed:
for (annot <- mdef.mods.annotations)
if (annot.symbol.is(JavaDefined)) then
checkNamedArgumentForJavaAnnotation(annot)
val annot1 = typedAnnotation(annot)(using annotCtx)
checkAnnotApplicable(annot1, sym)
if Annotations.annotClass(annot1) == defn.NowarnAnnot then
Expand Down
42 changes: 42 additions & 0 deletions tests/neg/i20554.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
-- [E201] Syntax Error: tests/neg/i20554/Test.scala:3:12 ---------------------------------------------------------------
3 |@Annotation(3, 4) // error // error
| ^
| 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/Test.scala:3:15 ---------------------------------------------------------------
3 |@Annotation(3, 4) // error // error
| ^
| 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)
|
---------------------------------------------------------------------------------------------------------------------
5 changes: 5 additions & 0 deletions tests/neg/i20554/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;
}
4 changes: 4 additions & 0 deletions tests/neg/i20554/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
//> using options -explain

@Annotation(3, 4) // error // error
class Test
5 changes: 5 additions & 0 deletions tests/pos/i20554/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/Container.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Container {
SimpleAnnotation[] value();
}
6 changes: 6 additions & 0 deletions tests/pos/i20554/SimpleAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.lang.annotation.Repeatable;

@Repeatable(Container.class)
public @interface SimpleAnnotation {
int a() default 1;
}
4 changes: 4 additions & 0 deletions tests/pos/i20554/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

@Container(Array(new SimpleAnnotation(), new SimpleAnnotation(1), new SimpleAnnotation(a = 1)))
@Annotation(a = 1, b = 2)
class Test

0 comments on commit f915648

Please sign in to comment.