Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require named arguments for java defined annotations #21329

Merged
merged 2 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
20 changes: 20 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,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 {
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
2 changes: 1 addition & 1 deletion tests/pos-java-interop-separate/i6868/MyScala_2.scala
Original file line number Diff line number Diff line change
@@ -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")
Expand Down
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
2 changes: 1 addition & 1 deletion tests/pos/i6151/Test.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import Expect.*
@Outcome(ExpectVal)
@Outcome(enm = ExpectVal)
class SimpleTest
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = _)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Loading