Skip to content

Commit

Permalink
Add migration rewrite for non-named arguments in Java annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
WojciechMazur committed Aug 19, 2024
1 parent f1adc55 commit 7574b91
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 6 deletions.
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/MigrationVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ object MigrationVersion:
val WithOperator = MigrationVersion(`3.4`, future)
val FunctionUnderscore = MigrationVersion(`3.4`, future)

val NonNamedArgumentInJavaAnnotation = MigrationVersion(`3.6`, `3.6`)

val ImportWildcard = MigrationVersion(future, future)
val ImportRename = MigrationVersion(future, future)
val ParameterEnclosedByParenthesis = MigrationVersion(future, future)
Expand Down
10 changes: 6 additions & 4 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.SourcePosition
import scala.jdk.CollectionConverters.*
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.config.SourceVersion
import DidYouMean.*

/** Messages
Expand Down Expand Up @@ -3291,13 +3292,14 @@ object UnusedSymbol {

class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamedArgumentInJavaAnnotationID):

override protected def msg(using Context): String =
override protected def msg(using Context): String =
"Named arguments are required for Java defined annotations"
+ Message.rewriteNotice("This", version = SourceVersion.`3.6-migration`)

override protected def explain(using Context): String =
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.
|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:
|
Expand Down
16 changes: 14 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -891,14 +891,26 @@ object Checking {
def annotationHasValueField: Boolean =
sym.info.decls.exists(_.name == nme.value)

lazy val annotationFieldNamesByIdx: Map[Int, TermName] =
sym.info.decls.filter: decl =>
decl.is(Method) && decl.name != nme.CONSTRUCTOR
.map(_.name.toTermName)
.zipWithIndex
.map(_.swap)
.toMap

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
(param, paramIdx) <- params.zipWithIndex
if !param.isInstanceOf[untpd.NamedArg]
do report.error(NonNamedArgumentInJavaAnnotation(), param)
do
report.errorOrMigrationWarning(NonNamedArgumentInJavaAnnotation(), param, MigrationVersion.NonNamedArgumentInJavaAnnotation)
if MigrationVersion.NonNamedArgumentInJavaAnnotation.needsPatch then
annotationFieldNamesByIdx.get(paramIdx).foreach: paramName =>
patch(param.span, untpd.cpy.NamedArg(param)(paramName, param).show)
annot
case _ => annot
end checkNamedArgumentForJavaAnnotation
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class CompilationTests {
compileFile("tests/rewrites/i17187.scala", unindentOptions.and("-rewrite")),
compileFile("tests/rewrites/i17399.scala", unindentOptions.and("-rewrite")),
compileFile("tests/rewrites/i20002.scala", defaultOptions.and("-indent", "-rewrite")),
compileDir("tests/rewrites/annotation-named-pararamters", defaultOptions.and("-rewrite", "-source:3.6-migration")),
).checkRewrites()
}

Expand Down
8 changes: 8 additions & 0 deletions tests/rewrites/annotation-named-pararamters/MyAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import java.util.concurrent.TimeUnit;

public @interface MyAnnotation {
public TimeUnit D() default TimeUnit.DAYS;
TimeUnit C() default TimeUnit.DAYS;
String A() default "";
public String B() default "";
}
6 changes: 6 additions & 0 deletions tests/rewrites/annotation-named-pararamters/test.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.util.concurrent.TimeUnit
@MyAnnotation() class Test1
@MyAnnotation(D = TimeUnit.DAYS) class Test2
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS) class Test3
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS, A = "foo") class Test4
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS, A = "foo", B = "bar") class Test5
6 changes: 6 additions & 0 deletions tests/rewrites/annotation-named-pararamters/test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.util.concurrent.TimeUnit
@MyAnnotation() class Test1
@MyAnnotation(TimeUnit.DAYS) class Test2
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS) class Test3
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS, "foo") class Test4
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS, "foo", "bar") class Test5

0 comments on commit 7574b91

Please sign in to comment.